fix(runtime): always call component lifecycle hooks (#6167)

* chore: init. wip * chore: tests * chore: * chore: prettier * chore: revert hasLifecycle * chore: remove all `componentDidUnload` related stuff * chore: build nonsense --------- Co-authored-by: John Jenkins <[email protected]>
ionic-team · Feb 22, 2025 · 260ced2 · 260ced2
1 parent 57e7e58
commit 260ced2
Show file tree

Hide file tree

Showing 14 changed files with 163 additions and 94 deletions.
diff --git a/src/app-data/index.ts b/src/app-data/index.ts
@@ -24,19 +24,9 @@ import type { BuildConditionals } from '@stencil/core/internal';
  */
 export const BUILD: BuildConditionals = {
   allRenderFn: false,
-  cmpDidLoad: true,
-  cmpDidUnload: false,
-  cmpDidUpdate: true,
-  cmpDidRender: true,
-  cmpWillLoad: true,
-  cmpWillUpdate: true,
-  cmpWillRender: true,
-  connectedCallback: true,
-  disconnectedCallback: true,
   element: true,
   event: true,
   hasRenderFn: true,
-  lifecycle: true,
   hostListener: true,
   hostListenerTargetWindow: true,
   hostListenerTargetDocument: true,
@@ -101,7 +91,6 @@ export const BUILD: BuildConditionals = {
   propNumber: true,
   propString: true,
   constructableCSS: true,
-  cmpShouldUpdate: true,
   devTools: false,
   shadowDelegatesFocus: true,
   initializeNextTick: false,

diff --git a/src/compiler/app-core/app-data.ts b/src/compiler/app-core/app-data.ts
@@ -24,23 +24,13 @@ export const getBuildFeatures = (cmps: ComponentCompilerMeta[]): BuildFeatures =
   const slotRelocation = cmps.some((c) => c.encapsulation !== 'shadow' && c.htmlTagNames.includes('slot'));
   const f: BuildFeatures = {
     allRenderFn: cmps.every((c) => c.hasRenderFn),
-    cmpDidLoad: cmps.some((c) => c.hasComponentDidLoadFn),
-    cmpShouldUpdate: cmps.some((c) => c.hasComponentShouldUpdateFn),
-    cmpDidUnload: cmps.some((c) => c.hasComponentDidUnloadFn),
-    cmpDidUpdate: cmps.some((c) => c.hasComponentDidUpdateFn),
-    cmpDidRender: cmps.some((c) => c.hasComponentDidRenderFn),
-    cmpWillLoad: cmps.some((c) => c.hasComponentWillLoadFn),
-    cmpWillUpdate: cmps.some((c) => c.hasComponentWillUpdateFn),
-    cmpWillRender: cmps.some((c) => c.hasComponentWillRenderFn),
     formAssociated: cmps.some((c) => c.formAssociated),
 
-    connectedCallback: cmps.some((c) => c.hasConnectedCallbackFn),
-    disconnectedCallback: cmps.some((c) => c.hasDisconnectedCallbackFn),
     element: cmps.some((c) => c.hasElement),
     event: cmps.some((c) => c.hasEvent),
     hasRenderFn: cmps.some((c) => c.hasRenderFn),
     lifecycle: cmps.some((c) => c.hasLifecycle),
-    asyncLoading: false,
+    asyncLoading: true,
     hostListener: cmps.some((c) => c.hasListener),
     hostListenerTargetWindow: cmps.some((c) => c.hasListenerTargetWindow),
     hostListenerTargetDocument: cmps.some((c) => c.hasListenerTargetDocument),
@@ -81,7 +71,6 @@ export const getBuildFeatures = (cmps: ComponentCompilerMeta[]): BuildFeatures =
     watchCallback: cmps.some((c) => c.hasWatchCallback),
     taskQueue: true,
   };
-  f.asyncLoading = f.cmpWillUpdate || f.cmpWillLoad || f.cmpWillRender;
   f.vdomAttribute = f.vdomAttribute || f.reflect;
   f.vdomPropOrAttr = f.vdomPropOrAttr || f.reflect;
 

diff --git a/src/compiler/transformers/static-to-meta/class-methods.ts b/src/compiler/transformers/static-to-meta/class-methods.ts
@@ -25,7 +25,6 @@ export const parseClassMethods = (cmpNode: ts.ClassDeclaration, cmpMeta: d.Compo
   cmpMeta.hasComponentDidLoadFn = classMethods.some((m) => isMethod(m, 'componentDidLoad'));
   cmpMeta.hasComponentShouldUpdateFn = classMethods.some((m) => isMethod(m, 'componentShouldUpdate'));
   cmpMeta.hasComponentDidUpdateFn = classMethods.some((m) => isMethod(m, 'componentDidUpdate'));
-  cmpMeta.hasComponentDidUnloadFn = classMethods.some((m) => isMethod(m, 'componentDidUnload'));
   cmpMeta.hasLifecycle =
     cmpMeta.hasComponentWillLoadFn ||
     cmpMeta.hasComponentDidLoadFn ||

diff --git a/src/compiler/transformers/static-to-meta/component.ts b/src/compiler/transformers/static-to-meta/component.ts
@@ -106,7 +106,6 @@ export const parseStaticComponentMeta = (
     hasComponentDidUpdateFn: false,
     hasComponentWillRenderFn: false,
     hasComponentDidRenderFn: false,
-    hasComponentDidUnloadFn: false,
     hasConnectedCallbackFn: false,
     hasDisconnectedCallbackFn: false,
     hasElement: false,

diff --git a/src/compiler/types/tests/ComponentCompilerMeta.stub.ts b/src/compiler/types/tests/ComponentCompilerMeta.stub.ts
@@ -27,7 +27,6 @@ export const stubComponentCompilerMeta = (
   hasAttributeChangedCallbackFn: false,
   hasComponentDidLoadFn: false,
   hasComponentDidRenderFn: false,
-  hasComponentDidUnloadFn: false,
   hasComponentDidUpdateFn: false,
   hasComponentShouldUpdateFn: false,
   hasComponentWillLoadFn: false,

diff --git a/src/declarations/stencil-private.ts b/src/declarations/stencil-private.ts
@@ -145,16 +145,6 @@ export interface BuildFeatures {
 
   // lifecycle events
   lifecycle: boolean;
-  cmpDidLoad: boolean;
-  cmpShouldUpdate: boolean;
-  cmpWillLoad: boolean;
-  cmpDidUpdate: boolean;
-  cmpWillUpdate: boolean;
-  cmpWillRender: boolean;
-  cmpDidRender: boolean;
-  cmpDidUnload: boolean;
-  connectedCallback: boolean;
-  disconnectedCallback: boolean;
   asyncLoading: boolean;
 
   // attr
@@ -547,7 +537,6 @@ export interface ComponentCompilerFeatures {
   hasComponentDidUpdateFn: boolean;
   hasComponentWillRenderFn: boolean;
   hasComponentDidRenderFn: boolean;
-  hasComponentDidUnloadFn: boolean;
   hasConnectedCallbackFn: boolean;
   hasDisconnectedCallbackFn: boolean;
   hasElement: boolean;

diff --git a/src/runtime/bootstrap-custom-element.ts b/src/runtime/bootstrap-custom-element.ts
@@ -84,13 +84,13 @@ export const proxyCustomElement = (Cstr: any, compactMeta: d.ComponentRuntimeMet
       }
 
       connectedCallback(this);
-      if (BUILD.connectedCallback && originalConnectedCallback) {
+      if (originalConnectedCallback) {
         originalConnectedCallback.call(this);
       }
     },
     disconnectedCallback() {
       disconnectedCallback(this);
-      if (BUILD.disconnectedCallback && originalDisconnectedCallback) {
+      if (originalDisconnectedCallback) {
         originalDisconnectedCallback.call(this);
       }
     },

diff --git a/src/runtime/disconnected-callback.ts b/src/runtime/disconnected-callback.ts
@@ -7,12 +7,9 @@ import { rootAppliedStyles } from './styles';
 import { safeCall } from './update-component';
 
 const disconnectInstance = (instance: any, elm?: d.HostElement) => {
-  if (BUILD.lazyLoad && BUILD.disconnectedCallback) {
+  if (BUILD.lazyLoad) {
     safeCall(instance, 'disconnectedCallback', undefined, elm || instance);
   }
-  if (BUILD.cmpDidUnload) {
-    safeCall(instance, 'componentDidUnload', undefined, elm || instance);
-  }
 };
 
 export const disconnectedCallback = async (elm: d.HostElement) => {

diff --git a/src/runtime/initialize-component.ts b/src/runtime/initialize-component.ts
@@ -184,7 +184,7 @@ export const initializeComponent = async (
 };
 
 export const fireConnectedCallback = (instance: any, elm?: HTMLElement) => {
-  if (BUILD.lazyLoad && BUILD.connectedCallback) {
+  if (BUILD.lazyLoad) {
     safeCall(instance, 'connectedCallback', undefined, elm);
   }
 };
diff --git a/src/runtime/set-value.ts b/src/runtime/set-value.ts
@@ -92,7 +92,7 @@ export const setValue = (ref: d.RuntimeRef, propName: string, newVal: any, cmpMe
         BUILD.updatable &&
         (flags & (HOST_FLAGS.hasRendered | HOST_FLAGS.isQueuedForUpdate)) === HOST_FLAGS.hasRendered
       ) {
-        if (BUILD.cmpShouldUpdate && instance.componentShouldUpdate) {
+        if (instance.componentShouldUpdate) {
           if (instance.componentShouldUpdate(newVal, oldVal, propName) === false) {
             return;
           }

diff --git a/src/runtime/update-component.ts b/src/runtime/update-component.ts
@@ -97,31 +97,25 @@ const dispatchHooks = (hostRef: d.HostRef, isInitialLoad: boolean): Promise<void
       }
     }
     emitLifecycleEvent(elm, 'componentWillLoad');
-    if (BUILD.cmpWillLoad) {
-      // If `componentWillLoad` returns a `Promise` then we want to wait on
-      // whatever's going on in that `Promise` before we launch into
-      // rendering the component, doing other lifecycle stuff, etc. So
-      // in that case we assign the returned promise to the variable we
-      // declared above to hold a possible 'queueing' Promise
-      maybePromise = safeCall(instance, 'componentWillLoad', undefined, elm);
-    }
+    // If `componentWillLoad` returns a `Promise` then we want to wait on
+    // whatever's going on in that `Promise` before we launch into
+    // rendering the component, doing other lifecycle stuff, etc. So
+    // in that case we assign the returned promise to the variable we
+    // declared above to hold a possible 'queueing' Promise
+    maybePromise = safeCall(instance, 'componentWillLoad', undefined, elm);
   } else {
     emitLifecycleEvent(elm, 'componentWillUpdate');
 
-    if (BUILD.cmpWillUpdate) {
-      // Like `componentWillLoad` above, we allow Stencil component
-      // authors to return a `Promise` from this lifecycle callback, and
-      // we specify that our runtime will wait for that `Promise` to
-      // resolve before the component re-renders. So if the method
-      // returns a `Promise` we need to keep it around!
-      maybePromise = safeCall(instance, 'componentWillUpdate', undefined, elm);
-    }
+    // Like `componentWillLoad` above, we allow Stencil component
+    // authors to return a `Promise` from this lifecycle callback, and
+    // we specify that our runtime will wait for that `Promise` to
+    // resolve before the component re-renders. So if the method
+    // returns a `Promise` we need to keep it around!
+    maybePromise = safeCall(instance, 'componentWillUpdate', undefined, elm);
   }
 
   emitLifecycleEvent(elm, 'componentWillRender');
-  if (BUILD.cmpWillRender) {
-    maybePromise = enqueue(maybePromise, () => safeCall(instance, 'componentWillRender', undefined, elm));
-  }
+  maybePromise = enqueue(maybePromise, () => safeCall(instance, 'componentWillRender', undefined, elm));
 
   endSchedule();
 
@@ -322,14 +316,12 @@ export const postUpdateComponent = (hostRef: d.HostRef) => {
   const instance = BUILD.lazyLoad ? hostRef.$lazyInstance$ : (elm as any);
   const ancestorComponent = hostRef.$ancestorComponent$;
 
-  if (BUILD.cmpDidRender) {
-    if (BUILD.isDev) {
-      hostRef.$flags$ |= HOST_FLAGS.devOnRender;
-    }
-    safeCall(instance, 'componentDidRender', undefined, elm);
-    if (BUILD.isDev) {
-      hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
-    }
+  if (BUILD.isDev) {
+    hostRef.$flags$ |= HOST_FLAGS.devOnRender;
+  }
+  safeCall(instance, 'componentDidRender', undefined, elm);
+  if (BUILD.isDev) {
+    hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
   }
   emitLifecycleEvent(elm, 'componentDidRender');
 
@@ -341,14 +333,12 @@ export const postUpdateComponent = (hostRef: d.HostRef) => {
       addHydratedFlag(elm);
     }
 
-    if (BUILD.cmpDidLoad) {
-      if (BUILD.isDev) {
-        hostRef.$flags$ |= HOST_FLAGS.devOnDidLoad;
-      }
-      safeCall(instance, 'componentDidLoad', undefined, elm);
-      if (BUILD.isDev) {
-        hostRef.$flags$ &= ~HOST_FLAGS.devOnDidLoad;
-      }
+    if (BUILD.isDev) {
+      hostRef.$flags$ |= HOST_FLAGS.devOnDidLoad;
+    }
+    safeCall(instance, 'componentDidLoad', undefined, elm);
+    if (BUILD.isDev) {
+      hostRef.$flags$ &= ~HOST_FLAGS.devOnDidLoad;
     }
 
     emitLifecycleEvent(elm, 'componentDidLoad');
@@ -361,18 +351,16 @@ export const postUpdateComponent = (hostRef: d.HostRef) => {
       }
     }
   } else {
-    if (BUILD.cmpDidUpdate) {
-      // we've already loaded this component
-      // fire off the user's componentDidUpdate method (if one was provided)
-      // componentDidUpdate runs AFTER render() has been called
-      // and all child components have finished updating
-      if (BUILD.isDev) {
-        hostRef.$flags$ |= HOST_FLAGS.devOnRender;
-      }
-      safeCall(instance, 'componentDidUpdate', undefined, elm);
-      if (BUILD.isDev) {
-        hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
-      }
+    // we've already loaded this component
+    // fire off the user's componentDidUpdate method (if one was provided)
+    // componentDidUpdate runs AFTER render() has been called
+    // and all child components have finished updating
+    if (BUILD.isDev) {
+      hostRef.$flags$ |= HOST_FLAGS.devOnRender;
+    }
+    safeCall(instance, 'componentDidUpdate', undefined, elm);
+    if (BUILD.isDev) {
+      hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
     }
     emitLifecycleEvent(elm, 'componentDidUpdate');
     endPostUpdate();

diff --git a/test/wdio/ts-target-props/cmp.test.tsx b/test/wdio/ts-target-props/cmp.test.tsx
@@ -4,6 +4,10 @@ import { $, browser } from '@wdio/globals';
 
 import { setupIFrameTest } from '../util.js';
 
+/**
+ * Smoke tests for `tsconfig.json` > `"target": "es2022"` `dist` and `dist-custom-elements` outputs.
+ */
+
 // @ts-ignore may not be existing when project hasn't been built
 type HydrateModule = typeof import('../../hydrate');
 
@@ -123,6 +127,25 @@ const testSuites = async (root: HTMLTsTargetPropsElement) => {
       expect(await getTxtHtml(html, 'basicState')).toBe('basicState');
       expect(await getTxtHtml(html, 'decoratedState')).toBe('10');
     },
+    dynamicLifecycleMethods: async () => {
+      root.basicProp = 'basicProp via prop';
+      await browser.pause(100);
+      const buttons = root.querySelectorAll('button');
+      buttons[0].click();
+      await browser.pause(100);
+      root.remove();
+      await browser.pause(100);
+
+      expect(window.lifecycleCalls).toContain('componentWillLoad');
+      expect(window.lifecycleCalls).toContain('componentWillRender');
+      expect(window.lifecycleCalls).toContain('componentDidLoad');
+      expect(window.lifecycleCalls).toContain('componentDidRender');
+      expect(window.lifecycleCalls).toContain('connectedCallback');
+      expect(window.lifecycleCalls).toContain('disconnectedCallback');
+      expect(window.lifecycleCalls).toContain('componentShouldUpdate');
+      expect(window.lifecycleCalls).toContain('componentWillUpdate');
+      expect(window.lifecycleCalls).toContain('componentDidUpdate');
+    },
   };
 };
 
@@ -160,6 +183,12 @@ describe('Checks class properties and runtime decorators of different es targets
       const mod = await import('/hydrate/index.mjs');
       await (await testSuites(document.querySelector('ts-target-props'))).ssrViaProps(mod);
     });
+
+    it('adds dynamic lifecycle hooks', async () => {
+      render({ template: () => <ts-target-props /> });
+      await (await $('ts-target-props')).waitForStable();
+      await (await testSuites(document.querySelector('ts-target-props'))).dynamicLifecycleMethods();
+    });
   });
 
   describe('es2022 dist output', () => {
@@ -196,6 +225,11 @@ describe('Checks class properties and runtime decorators of different es targets
       const mod = await import('/test-ts-target-output/hydrate/index.mjs');
       await (await testSuites(document.querySelector('ts-target-props'))).ssrViaProps(mod);
     });
+
+    it('adds dynamic lifecycle hooks', async () => {
+      const { dynamicLifecycleMethods } = await testSuites(frameContent.querySelector('ts-target-props'));
+      await dynamicLifecycleMethods();
+    });
   });
 
   describe('es2022 dist-custom-elements output', () => {
@@ -227,5 +261,10 @@ describe('Checks class properties and runtime decorators of different es targets
       const { reflectsStateChanges } = await testSuites(frameContent.querySelector('ts-target-props'));
       await reflectsStateChanges();
     });
+
+    it('adds dynamic lifecycle hooks', async () => {
+      const { dynamicLifecycleMethods } = await testSuites(frameContent.querySelector('ts-target-props'));
+      await dynamicLifecycleMethods();
+    });
   });
 });