Skip to content

Commit 260ced2

Browse files
johnjenkinsJohn Jenkins
and
John Jenkins
authored
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 <john.jenkins@nanoporetech.com>
1 parent 57e7e58 commit 260ced2

14 files changed

+163
-94
lines changed

src/app-data/index.ts

-11
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,9 @@ import type { BuildConditionals } from '@stencil/core/internal';
2424
*/
2525
export const BUILD: BuildConditionals = {
2626
allRenderFn: false,
27-
cmpDidLoad: true,
28-
cmpDidUnload: false,
29-
cmpDidUpdate: true,
30-
cmpDidRender: true,
31-
cmpWillLoad: true,
32-
cmpWillUpdate: true,
33-
cmpWillRender: true,
34-
connectedCallback: true,
35-
disconnectedCallback: true,
3627
element: true,
3728
event: true,
3829
hasRenderFn: true,
39-
lifecycle: true,
4030
hostListener: true,
4131
hostListenerTargetWindow: true,
4232
hostListenerTargetDocument: true,
@@ -101,7 +91,6 @@ export const BUILD: BuildConditionals = {
10191
propNumber: true,
10292
propString: true,
10393
constructableCSS: true,
104-
cmpShouldUpdate: true,
10594
devTools: false,
10695
shadowDelegatesFocus: true,
10796
initializeNextTick: false,

src/compiler/app-core/app-data.ts

+1-12
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,13 @@ export const getBuildFeatures = (cmps: ComponentCompilerMeta[]): BuildFeatures =
2424
const slotRelocation = cmps.some((c) => c.encapsulation !== 'shadow' && c.htmlTagNames.includes('slot'));
2525
const f: BuildFeatures = {
2626
allRenderFn: cmps.every((c) => c.hasRenderFn),
27-
cmpDidLoad: cmps.some((c) => c.hasComponentDidLoadFn),
28-
cmpShouldUpdate: cmps.some((c) => c.hasComponentShouldUpdateFn),
29-
cmpDidUnload: cmps.some((c) => c.hasComponentDidUnloadFn),
30-
cmpDidUpdate: cmps.some((c) => c.hasComponentDidUpdateFn),
31-
cmpDidRender: cmps.some((c) => c.hasComponentDidRenderFn),
32-
cmpWillLoad: cmps.some((c) => c.hasComponentWillLoadFn),
33-
cmpWillUpdate: cmps.some((c) => c.hasComponentWillUpdateFn),
34-
cmpWillRender: cmps.some((c) => c.hasComponentWillRenderFn),
3527
formAssociated: cmps.some((c) => c.formAssociated),
3628

37-
connectedCallback: cmps.some((c) => c.hasConnectedCallbackFn),
38-
disconnectedCallback: cmps.some((c) => c.hasDisconnectedCallbackFn),
3929
element: cmps.some((c) => c.hasElement),
4030
event: cmps.some((c) => c.hasEvent),
4131
hasRenderFn: cmps.some((c) => c.hasRenderFn),
4232
lifecycle: cmps.some((c) => c.hasLifecycle),
43-
asyncLoading: false,
33+
asyncLoading: true,
4434
hostListener: cmps.some((c) => c.hasListener),
4535
hostListenerTargetWindow: cmps.some((c) => c.hasListenerTargetWindow),
4636
hostListenerTargetDocument: cmps.some((c) => c.hasListenerTargetDocument),
@@ -81,7 +71,6 @@ export const getBuildFeatures = (cmps: ComponentCompilerMeta[]): BuildFeatures =
8171
watchCallback: cmps.some((c) => c.hasWatchCallback),
8272
taskQueue: true,
8373
};
84-
f.asyncLoading = f.cmpWillUpdate || f.cmpWillLoad || f.cmpWillRender;
8574
f.vdomAttribute = f.vdomAttribute || f.reflect;
8675
f.vdomPropOrAttr = f.vdomPropOrAttr || f.reflect;
8776

src/compiler/transformers/static-to-meta/class-methods.ts

-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ export const parseClassMethods = (cmpNode: ts.ClassDeclaration, cmpMeta: d.Compo
2525
cmpMeta.hasComponentDidLoadFn = classMethods.some((m) => isMethod(m, 'componentDidLoad'));
2626
cmpMeta.hasComponentShouldUpdateFn = classMethods.some((m) => isMethod(m, 'componentShouldUpdate'));
2727
cmpMeta.hasComponentDidUpdateFn = classMethods.some((m) => isMethod(m, 'componentDidUpdate'));
28-
cmpMeta.hasComponentDidUnloadFn = classMethods.some((m) => isMethod(m, 'componentDidUnload'));
2928
cmpMeta.hasLifecycle =
3029
cmpMeta.hasComponentWillLoadFn ||
3130
cmpMeta.hasComponentDidLoadFn ||

src/compiler/transformers/static-to-meta/component.ts

-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ export const parseStaticComponentMeta = (
106106
hasComponentDidUpdateFn: false,
107107
hasComponentWillRenderFn: false,
108108
hasComponentDidRenderFn: false,
109-
hasComponentDidUnloadFn: false,
110109
hasConnectedCallbackFn: false,
111110
hasDisconnectedCallbackFn: false,
112111
hasElement: false,

src/compiler/types/tests/ComponentCompilerMeta.stub.ts

-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ export const stubComponentCompilerMeta = (
2727
hasAttributeChangedCallbackFn: false,
2828
hasComponentDidLoadFn: false,
2929
hasComponentDidRenderFn: false,
30-
hasComponentDidUnloadFn: false,
3130
hasComponentDidUpdateFn: false,
3231
hasComponentShouldUpdateFn: false,
3332
hasComponentWillLoadFn: false,

src/declarations/stencil-private.ts

-11
Original file line numberDiff line numberDiff line change
@@ -145,16 +145,6 @@ export interface BuildFeatures {
145145

146146
// lifecycle events
147147
lifecycle: boolean;
148-
cmpDidLoad: boolean;
149-
cmpShouldUpdate: boolean;
150-
cmpWillLoad: boolean;
151-
cmpDidUpdate: boolean;
152-
cmpWillUpdate: boolean;
153-
cmpWillRender: boolean;
154-
cmpDidRender: boolean;
155-
cmpDidUnload: boolean;
156-
connectedCallback: boolean;
157-
disconnectedCallback: boolean;
158148
asyncLoading: boolean;
159149

160150
// attr
@@ -547,7 +537,6 @@ export interface ComponentCompilerFeatures {
547537
hasComponentDidUpdateFn: boolean;
548538
hasComponentWillRenderFn: boolean;
549539
hasComponentDidRenderFn: boolean;
550-
hasComponentDidUnloadFn: boolean;
551540
hasConnectedCallbackFn: boolean;
552541
hasDisconnectedCallbackFn: boolean;
553542
hasElement: boolean;

src/runtime/bootstrap-custom-element.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,13 @@ export const proxyCustomElement = (Cstr: any, compactMeta: d.ComponentRuntimeMet
8484
}
8585

8686
connectedCallback(this);
87-
if (BUILD.connectedCallback && originalConnectedCallback) {
87+
if (originalConnectedCallback) {
8888
originalConnectedCallback.call(this);
8989
}
9090
},
9191
disconnectedCallback() {
9292
disconnectedCallback(this);
93-
if (BUILD.disconnectedCallback && originalDisconnectedCallback) {
93+
if (originalDisconnectedCallback) {
9494
originalDisconnectedCallback.call(this);
9595
}
9696
},

src/runtime/disconnected-callback.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,9 @@ import { rootAppliedStyles } from './styles';
77
import { safeCall } from './update-component';
88

99
const disconnectInstance = (instance: any, elm?: d.HostElement) => {
10-
if (BUILD.lazyLoad && BUILD.disconnectedCallback) {
10+
if (BUILD.lazyLoad) {
1111
safeCall(instance, 'disconnectedCallback', undefined, elm || instance);
1212
}
13-
if (BUILD.cmpDidUnload) {
14-
safeCall(instance, 'componentDidUnload', undefined, elm || instance);
15-
}
1613
};
1714

1815
export const disconnectedCallback = async (elm: d.HostElement) => {

src/runtime/initialize-component.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ export const initializeComponent = async (
184184
};
185185

186186
export const fireConnectedCallback = (instance: any, elm?: HTMLElement) => {
187-
if (BUILD.lazyLoad && BUILD.connectedCallback) {
187+
if (BUILD.lazyLoad) {
188188
safeCall(instance, 'connectedCallback', undefined, elm);
189189
}
190190
};

src/runtime/set-value.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export const setValue = (ref: d.RuntimeRef, propName: string, newVal: any, cmpMe
9292
BUILD.updatable &&
9393
(flags & (HOST_FLAGS.hasRendered | HOST_FLAGS.isQueuedForUpdate)) === HOST_FLAGS.hasRendered
9494
) {
95-
if (BUILD.cmpShouldUpdate && instance.componentShouldUpdate) {
95+
if (instance.componentShouldUpdate) {
9696
if (instance.componentShouldUpdate(newVal, oldVal, propName) === false) {
9797
return;
9898
}

src/runtime/update-component.ts

+35-47
Original file line numberDiff line numberDiff line change
@@ -97,31 +97,25 @@ const dispatchHooks = (hostRef: d.HostRef, isInitialLoad: boolean): Promise<void
9797
}
9898
}
9999
emitLifecycleEvent(elm, 'componentWillLoad');
100-
if (BUILD.cmpWillLoad) {
101-
// If `componentWillLoad` returns a `Promise` then we want to wait on
102-
// whatever's going on in that `Promise` before we launch into
103-
// rendering the component, doing other lifecycle stuff, etc. So
104-
// in that case we assign the returned promise to the variable we
105-
// declared above to hold a possible 'queueing' Promise
106-
maybePromise = safeCall(instance, 'componentWillLoad', undefined, elm);
107-
}
100+
// If `componentWillLoad` returns a `Promise` then we want to wait on
101+
// whatever's going on in that `Promise` before we launch into
102+
// rendering the component, doing other lifecycle stuff, etc. So
103+
// in that case we assign the returned promise to the variable we
104+
// declared above to hold a possible 'queueing' Promise
105+
maybePromise = safeCall(instance, 'componentWillLoad', undefined, elm);
108106
} else {
109107
emitLifecycleEvent(elm, 'componentWillUpdate');
110108

111-
if (BUILD.cmpWillUpdate) {
112-
// Like `componentWillLoad` above, we allow Stencil component
113-
// authors to return a `Promise` from this lifecycle callback, and
114-
// we specify that our runtime will wait for that `Promise` to
115-
// resolve before the component re-renders. So if the method
116-
// returns a `Promise` we need to keep it around!
117-
maybePromise = safeCall(instance, 'componentWillUpdate', undefined, elm);
118-
}
109+
// Like `componentWillLoad` above, we allow Stencil component
110+
// authors to return a `Promise` from this lifecycle callback, and
111+
// we specify that our runtime will wait for that `Promise` to
112+
// resolve before the component re-renders. So if the method
113+
// returns a `Promise` we need to keep it around!
114+
maybePromise = safeCall(instance, 'componentWillUpdate', undefined, elm);
119115
}
120116

121117
emitLifecycleEvent(elm, 'componentWillRender');
122-
if (BUILD.cmpWillRender) {
123-
maybePromise = enqueue(maybePromise, () => safeCall(instance, 'componentWillRender', undefined, elm));
124-
}
118+
maybePromise = enqueue(maybePromise, () => safeCall(instance, 'componentWillRender', undefined, elm));
125119

126120
endSchedule();
127121

@@ -322,14 +316,12 @@ export const postUpdateComponent = (hostRef: d.HostRef) => {
322316
const instance = BUILD.lazyLoad ? hostRef.$lazyInstance$ : (elm as any);
323317
const ancestorComponent = hostRef.$ancestorComponent$;
324318

325-
if (BUILD.cmpDidRender) {
326-
if (BUILD.isDev) {
327-
hostRef.$flags$ |= HOST_FLAGS.devOnRender;
328-
}
329-
safeCall(instance, 'componentDidRender', undefined, elm);
330-
if (BUILD.isDev) {
331-
hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
332-
}
319+
if (BUILD.isDev) {
320+
hostRef.$flags$ |= HOST_FLAGS.devOnRender;
321+
}
322+
safeCall(instance, 'componentDidRender', undefined, elm);
323+
if (BUILD.isDev) {
324+
hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
333325
}
334326
emitLifecycleEvent(elm, 'componentDidRender');
335327

@@ -341,14 +333,12 @@ export const postUpdateComponent = (hostRef: d.HostRef) => {
341333
addHydratedFlag(elm);
342334
}
343335

344-
if (BUILD.cmpDidLoad) {
345-
if (BUILD.isDev) {
346-
hostRef.$flags$ |= HOST_FLAGS.devOnDidLoad;
347-
}
348-
safeCall(instance, 'componentDidLoad', undefined, elm);
349-
if (BUILD.isDev) {
350-
hostRef.$flags$ &= ~HOST_FLAGS.devOnDidLoad;
351-
}
336+
if (BUILD.isDev) {
337+
hostRef.$flags$ |= HOST_FLAGS.devOnDidLoad;
338+
}
339+
safeCall(instance, 'componentDidLoad', undefined, elm);
340+
if (BUILD.isDev) {
341+
hostRef.$flags$ &= ~HOST_FLAGS.devOnDidLoad;
352342
}
353343

354344
emitLifecycleEvent(elm, 'componentDidLoad');
@@ -361,18 +351,16 @@ export const postUpdateComponent = (hostRef: d.HostRef) => {
361351
}
362352
}
363353
} else {
364-
if (BUILD.cmpDidUpdate) {
365-
// we've already loaded this component
366-
// fire off the user's componentDidUpdate method (if one was provided)
367-
// componentDidUpdate runs AFTER render() has been called
368-
// and all child components have finished updating
369-
if (BUILD.isDev) {
370-
hostRef.$flags$ |= HOST_FLAGS.devOnRender;
371-
}
372-
safeCall(instance, 'componentDidUpdate', undefined, elm);
373-
if (BUILD.isDev) {
374-
hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
375-
}
354+
// we've already loaded this component
355+
// fire off the user's componentDidUpdate method (if one was provided)
356+
// componentDidUpdate runs AFTER render() has been called
357+
// and all child components have finished updating
358+
if (BUILD.isDev) {
359+
hostRef.$flags$ |= HOST_FLAGS.devOnRender;
360+
}
361+
safeCall(instance, 'componentDidUpdate', undefined, elm);
362+
if (BUILD.isDev) {
363+
hostRef.$flags$ &= ~HOST_FLAGS.devOnRender;
376364
}
377365
emitLifecycleEvent(elm, 'componentDidUpdate');
378366
endPostUpdate();

test/wdio/ts-target-props/cmp.test.tsx

+39
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import { $, browser } from '@wdio/globals';
44

55
import { setupIFrameTest } from '../util.js';
66

7+
/**
8+
* Smoke tests for `tsconfig.json` > `"target": "es2022"` `dist` and `dist-custom-elements` outputs.
9+
*/
10+
711
// @ts-ignore may not be existing when project hasn't been built
812
type HydrateModule = typeof import('../../hydrate');
913

@@ -123,6 +127,25 @@ const testSuites = async (root: HTMLTsTargetPropsElement) => {
123127
expect(await getTxtHtml(html, 'basicState')).toBe('basicState');
124128
expect(await getTxtHtml(html, 'decoratedState')).toBe('10');
125129
},
130+
dynamicLifecycleMethods: async () => {
131+
root.basicProp = 'basicProp via prop';
132+
await browser.pause(100);
133+
const buttons = root.querySelectorAll('button');
134+
buttons[0].click();
135+
await browser.pause(100);
136+
root.remove();
137+
await browser.pause(100);
138+
139+
expect(window.lifecycleCalls).toContain('componentWillLoad');
140+
expect(window.lifecycleCalls).toContain('componentWillRender');
141+
expect(window.lifecycleCalls).toContain('componentDidLoad');
142+
expect(window.lifecycleCalls).toContain('componentDidRender');
143+
expect(window.lifecycleCalls).toContain('connectedCallback');
144+
expect(window.lifecycleCalls).toContain('disconnectedCallback');
145+
expect(window.lifecycleCalls).toContain('componentShouldUpdate');
146+
expect(window.lifecycleCalls).toContain('componentWillUpdate');
147+
expect(window.lifecycleCalls).toContain('componentDidUpdate');
148+
},
126149
};
127150
};
128151

@@ -160,6 +183,12 @@ describe('Checks class properties and runtime decorators of different es targets
160183
const mod = await import('/hydrate/index.mjs');
161184
await (await testSuites(document.querySelector('ts-target-props'))).ssrViaProps(mod);
162185
});
186+
187+
it('adds dynamic lifecycle hooks', async () => {
188+
render({ template: () => <ts-target-props /> });
189+
await (await $('ts-target-props')).waitForStable();
190+
await (await testSuites(document.querySelector('ts-target-props'))).dynamicLifecycleMethods();
191+
});
163192
});
164193

165194
describe('es2022 dist output', () => {
@@ -196,6 +225,11 @@ describe('Checks class properties and runtime decorators of different es targets
196225
const mod = await import('/test-ts-target-output/hydrate/index.mjs');
197226
await (await testSuites(document.querySelector('ts-target-props'))).ssrViaProps(mod);
198227
});
228+
229+
it('adds dynamic lifecycle hooks', async () => {
230+
const { dynamicLifecycleMethods } = await testSuites(frameContent.querySelector('ts-target-props'));
231+
await dynamicLifecycleMethods();
232+
});
199233
});
200234

201235
describe('es2022 dist-custom-elements output', () => {
@@ -227,5 +261,10 @@ describe('Checks class properties and runtime decorators of different es targets
227261
const { reflectsStateChanges } = await testSuites(frameContent.querySelector('ts-target-props'));
228262
await reflectsStateChanges();
229263
});
264+
265+
it('adds dynamic lifecycle hooks', async () => {
266+
const { dynamicLifecycleMethods } = await testSuites(frameContent.querySelector('ts-target-props'));
267+
await dynamicLifecycleMethods();
268+
});
230269
});
231270
});

0 commit comments

Comments
 (0)