From ddc358f48a59952a512df97fdf95bfba8697b051 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Tue, 14 Dec 2021 09:31:25 +0100 Subject: [PATCH] [FIX] fiber, lifecycle: trigger a render during the fiber.complete Have a component which does a render in its onWillPatch, onPatched, onMounted hooks. Before this commit, the result was incorrect: the second rendering was not taken into account. After this commit, those renderings are correctly applied at the price of a delayed render when the fiber is in a critical state. --- src/component/component_node.ts | 8 +- src/component/fibers.ts | 18 ++- .../__snapshots__/concurrency.test.ts.snap | 5 +- .../__snapshots__/lifecycle.test.ts.snap | 42 +++++++ tests/components/concurrency.test.ts | 42 ++++++- tests/components/lifecycle.test.ts | 117 ++++++++++++++++++ 6 files changed, 224 insertions(+), 8 deletions(-) diff --git a/src/component/component_node.ts b/src/component/component_node.ts index fc563640..e34f5d2e 100644 --- a/src/component/component_node.ts +++ b/src/component/component_node.ts @@ -136,13 +136,19 @@ export class ComponentNode } async render() { - const current = this.fiber; + let current = this.fiber; + if (current && current.root.locked) { + await Promise.resolve(); + // situation may have changed after the microtask tick + current = this.fiber; + } if (current && !current.bdom && !fibersInError.has(current)) { return; } if (!this.bdom && !current) { return; } + const fiber = makeRootFiber(this); this.fiber = fiber; this.app.scheduler.addFiber(fiber); diff --git a/src/component/fibers.ts b/src/component/fibers.ts index 24b20193..8037d4ae 100644 --- a/src/component/fibers.ts +++ b/src/component/fibers.ts @@ -118,9 +118,13 @@ export class RootFiber extends Fiber { willPatch: Fiber[] = []; patched: Fiber[] = []; mounted: Fiber[] = []; + // A fiber is typically locked when it is completing and the patch has not, or is being applied. + // i.e.: render triggered in onWillUnmount or in willPatch will be delayed + locked: boolean = false; complete() { const node = this.node; + this.locked = true; let current: Fiber | undefined = undefined; try { // Step 1: calling all willPatch lifecycle hooks @@ -142,6 +146,11 @@ export class RootFiber extends Fiber { node.bdom!.patch(this.bdom!, Object.keys(node.children).length > 0); this.appliedToDom = true; + this.locked = false; + // unregistering the fiber before mounted since it can do another render + // and that the current rendering is obviously completed + node.fiber = null; + // Step 4: calling all mounted lifecycle hooks let mountedFibers = this.mounted; while ((current = mountedFibers.pop())) { @@ -163,9 +172,8 @@ export class RootFiber extends Fiber { } } } - // unregistering the fiber - node.fiber = null; } catch (e) { + this.locked = false; handleError({ fiber: current || this, error: e }); } } @@ -205,6 +213,11 @@ export class MountFiber extends RootFiber { mount(node.bdom!, this.target, firstChild); } } + + // unregistering the fiber before mounted since it can do another render + // and that the current rendering is obviously completed + node.fiber = null; + node.status = STATUS.MOUNTED; this.appliedToDom = true; let mountedFibers = this.mounted; @@ -215,7 +228,6 @@ export class MountFiber extends RootFiber { } } } - node.fiber = null; } catch (e) { handleError({ fiber: current as Fiber, error: e }); } diff --git a/tests/components/__snapshots__/concurrency.test.ts.snap b/tests/components/__snapshots__/concurrency.test.ts.snap index 83e0b110..09920084 100644 --- a/tests/components/__snapshots__/concurrency.test.ts.snap +++ b/tests/components/__snapshots__/concurrency.test.ts.snap @@ -1148,11 +1148,12 @@ exports[`two renderings initiated between willPatch and patched 1`] = ` ) { let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; - let block1 = createBlock(\`\`); + let block1 = createBlock(\`\`); return function template(ctx, node, key = \\"\\") { let d1 = ctx['props'].val; - return block1([d1]); + let d2 = ctx['mounted']; + return block1([d1, d2]); } }" `; diff --git a/tests/components/__snapshots__/lifecycle.test.ts.snap b/tests/components/__snapshots__/lifecycle.test.ts.snap index 27a15929..1e3c2adb 100644 --- a/tests/components/__snapshots__/lifecycle.test.ts.snap +++ b/tests/components/__snapshots__/lifecycle.test.ts.snap @@ -562,6 +562,48 @@ exports[`lifecycle hooks patched hook is called after updating State 1`] = ` }" `; +exports[`lifecycle hooks render in mounted 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`\`); + + return function template(ctx, node, key = \\"\\") { + let d1 = ctx['patched']; + return block1([d1]); + } +}" +`; + +exports[`lifecycle hooks render in patched 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`\`); + + return function template(ctx, node, key = \\"\\") { + let d1 = ctx['patched']; + return block1([d1]); + } +}" +`; + +exports[`lifecycle hooks render in willPatch 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`\`); + + return function template(ctx, node, key = \\"\\") { + let d1 = ctx['patched']; + return block1([d1]); + } +}" +`; + exports[`lifecycle hooks sub widget (inside sub node): hooks are correctly called 1`] = ` "function anonymous(bdom, helpers ) { diff --git a/tests/components/concurrency.test.ts b/tests/components/concurrency.test.ts index 783a258d..3a791462 100644 --- a/tests/components/concurrency.test.ts +++ b/tests/components/concurrency.test.ts @@ -2466,10 +2466,14 @@ test("two renderings initiated between willPatch and patched", async () => { let parent: any = null; class Panel extends Component { - static template = xml``; + static template = xml``; + mounted: any; setup() { useLogLifecycle(); - onMounted(() => parent.render()); + onMounted(() => { + this.mounted = "Mounted"; + parent.render(); + }); onWillUnmount(() => parent.render()); } } @@ -2498,8 +2502,22 @@ test("two renderings initiated between willPatch and patched", async () => { "Panel:rendered", "Panel:mounted", "Parent:mounted", + "Parent:willRender", + "Panel:willUpdateProps", + "Parent:rendered", ]).toBeLogged(); + await nextTick(); + expect([ + "Panel:willRender", + "Panel:rendered", + "Parent:willPatch", + "Panel:willPatch", + "Panel:patched", + "Parent:patched", + ]).toBeLogged(); + expect(fixture.innerHTML).toBe("
Panel1Mounted
"); + parent.state.panel = "Panel2"; await nextTick(); expect(fixture.innerHTML).toBe("
Panel2
"); @@ -2515,6 +2533,20 @@ test("two renderings initiated between willPatch and patched", async () => { "Panel:willDestroy", "Panel:mounted", "Parent:patched", + "Parent:willRender", + "Panel:willUpdateProps", + "Parent:rendered", + ]).toBeLogged(); + + await nextTick(); + expect(fixture.innerHTML).toBe("
Panel2Mounted
"); + expect([ + "Panel:willRender", + "Panel:rendered", + "Parent:willPatch", + "Panel:willPatch", + "Panel:patched", + "Parent:patched", ]).toBeLogged(); parent.state.flag = false; @@ -2527,7 +2559,13 @@ test("two renderings initiated between willPatch and patched", async () => { "Panel:willUnmount", "Panel:willDestroy", "Parent:patched", + "Parent:willRender", + "Parent:rendered", ]).toBeLogged(); + + await nextTick(); + expect(fixture.innerHTML).toBe("
"); + expect(["Parent:willPatch", "Parent:patched"]).toBeLogged(); }); test("parent and child rendered at exact same time", async () => { diff --git a/tests/components/lifecycle.test.ts b/tests/components/lifecycle.test.ts index d39a869c..0617b6e6 100644 --- a/tests/components/lifecycle.test.ts +++ b/tests/components/lifecycle.test.ts @@ -1079,4 +1079,121 @@ describe("lifecycle hooks", () => { "Parent:patched", ]).toBeLogged(); }); + + test("render in mounted", async () => { + class Parent extends Component { + static template = xml``; + patched: any; + setup() { + useLogLifecycle(); + onMounted(() => { + this.patched = "Patched"; + this.render(); + }); + } + } + + await mount(Parent, fixture); + expect(fixture.innerHTML).toBe(""); + expect([ + "Parent:setup", + "Parent:willStart", + "Parent:willRender", + "Parent:rendered", + "Parent:mounted", + "Parent:willRender", + "Parent:rendered", + ]).toBeLogged(); + + await nextTick(); + expect(fixture.innerHTML).toBe("Patched"); + expect(["Parent:willPatch", "Parent:patched"]).toBeLogged(); + }); + + test("render in patched", async () => { + class Parent extends Component { + static template = xml``; + patched: any; + setup() { + useLogLifecycle(); + onPatched(() => { + if (this.patched === "Patched") { + return; + } + this.patched = "Patched"; + this.render(); + }); + } + } + + const parent = await mount(Parent, fixture); + expect(fixture.innerHTML).toBe(""); + expect([ + "Parent:setup", + "Parent:willStart", + "Parent:willRender", + "Parent:rendered", + "Parent:mounted", + ]).toBeLogged(); + + parent.render(); + await nextTick(); + expect(fixture.innerHTML).toBe(""); + expect([ + "Parent:willRender", + "Parent:rendered", + "Parent:willPatch", + "Parent:patched", + "Parent:willRender", + "Parent:rendered", + ]).toBeLogged(); + + await nextTick(); + expect(fixture.innerHTML).toBe("Patched"); + expect(["Parent:willPatch", "Parent:patched"]).toBeLogged(); + }); + + test("render in willPatch", async () => { + class Parent extends Component { + static template = xml``; + patched: any; + setup() { + useLogLifecycle(); + onWillPatch(() => { + if (this.patched === "Patched") { + return; + } + this.patched = "Patched"; + this.render(); + }); + } + } + + const parent = await mount(Parent, fixture); + expect(fixture.innerHTML).toBe(""); + expect([ + "Parent:setup", + "Parent:willStart", + "Parent:willRender", + "Parent:rendered", + "Parent:mounted", + ]).toBeLogged(); + + parent.render(); + await nextTick(); + expect(fixture.innerHTML).toBe(""); + + expect([ + "Parent:willRender", + "Parent:rendered", + "Parent:willPatch", + "Parent:patched", + "Parent:willRender", + "Parent:rendered", + ]).toBeLogged(); + + await nextTick(); + expect(["Parent:willPatch", "Parent:patched"]).toBeLogged(); + expect(fixture.innerHTML).toBe("Patched"); + }); });