From 68684a9a45c78b6bfe01bebccb4c66016be35a7a Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Thu, 13 Jan 2022 18:46:27 +0100 Subject: [PATCH] [FIX] component, fiber: subchildren should also patch and destroy their children Have a GrandParent which controls whether one of its GrandChildren is displayed or not. First, the GrandChild is displayed. Then, change the state of the GrandParent in order to kill the GrandChild. Before this commit the GrandChild is only removed from the DOM, as bdom correctly works. But it is not destroyed. After this commit, the GrandChild is correctly destroyed. --- src/component/component_node.ts | 14 +-- src/component/fibers.ts | 8 +- .../__snapshots__/basics.test.ts.snap | 93 ++++++++----------- tests/components/basics.test.ts | 37 ++++++++ 4 files changed, 85 insertions(+), 67 deletions(-) diff --git a/src/component/component_node.ts b/src/component/component_node.ts index 24c1fce5..a2a89977 100644 --- a/src/component/component_node.ts +++ b/src/component/component_node.ts @@ -275,8 +275,11 @@ export class ComponentNode } patch() { - this.bdom!.patch(this!.fiber!.bdom!, false); - this.cleanOutdatedChildren(); + const hasChildren = Object.keys(this.children).length > 0; + this.bdom!.patch(this!.fiber!.bdom!, hasChildren); + if (hasChildren) { + this.cleanOutdatedChildren(); + } this.fiber!.appliedToDom = true; this.fiber = null; } @@ -290,12 +293,9 @@ export class ComponentNode } cleanOutdatedChildren() { - const childrenEntries = Object.entries(this.children); - if (!childrenEntries.length) { - return; - } const children = this.children; - for (const [key, node] of childrenEntries) { + for (const key in children) { + const node = children[key]; const status = node.status; if (status !== STATUS.MOUNTED) { delete children[key]; diff --git a/src/component/fibers.ts b/src/component/fibers.ts index ed647368..b90f3d3e 100644 --- a/src/component/fibers.ts +++ b/src/component/fibers.ts @@ -143,14 +143,8 @@ export class RootFiber extends Fiber { current = undefined; // Step 2: patching the dom - node.bdom!.patch(this.bdom!, Object.keys(node.children).length > 0); - node.cleanOutdatedChildren(); - this.appliedToDom = true; - + node.patch(); 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; diff --git a/tests/components/__snapshots__/basics.test.ts.snap b/tests/components/__snapshots__/basics.test.ts.snap index 6e341a6d..b85efa8b 100644 --- a/tests/components/__snapshots__/basics.test.ts.snap +++ b/tests/components/__snapshots__/basics.test.ts.snap @@ -1,5 +1,45 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`basics GrandChild display is controlled by its GrandParent 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + return function template(ctx, node, key = \\"\\") { + let Comp1 = ctx['myComp']; + return toggler(Comp1, component(Comp1, {displayGrandChild: ctx['displayGrandChild']}, key + \`__1\`, node, ctx)); + } +}" +`; + +exports[`basics GrandChild display is controlled by its GrandParent 2`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + return function template(ctx, node, key = \\"\\") { + let b2; + if (ctx['props'].displayGrandChild) { + b2 = component(\`GrandChild\`, {}, key + \`__1\`, node, ctx); + } + return multi([b2]); + } +}" +`; + +exports[`basics GrandChild display is controlled by its GrandParent 3`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + exports[`basics Multi root component 1`] = ` "function anonymous(bdom, helpers ) { @@ -1177,59 +1217,6 @@ exports[`t-out in components can render list of t-out 1`] = ` }" `; -exports[`t-out in components component children doesn't leak (if case) 1`] = ` -"function anonymous(bdom, helpers -) { - let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; - - return function template(ctx, node, key = \\"\\") { - let b2; - if (ctx['ifVar']) { - b2 = component(\`Child\`, {}, key + \`__1\`, node, ctx); - } - return multi([b2]); - } -}" -`; - -exports[`t-out in components component children doesn't leak (if case) 2`] = ` -"function anonymous(bdom, helpers -) { - let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; - - let block1 = createBlock(\`
\`); - - return function template(ctx, node, key = \\"\\") { - return block1(); - } -}" -`; - -exports[`t-out in components component children doesn't leak (t-key case) 1`] = ` -"function anonymous(bdom, helpers -) { - let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; - - return function template(ctx, node, key = \\"\\") { - const tKey_1 = ctx['keyVar']; - return toggler(tKey_1, component(\`Child\`, {}, tKey_1 + key + \`__1\`, node, ctx)); - } -}" -`; - -exports[`t-out in components component children doesn't leak (t-key case) 2`] = ` -"function anonymous(bdom, helpers -) { - let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; - - let block1 = createBlock(\`
\`); - - return function template(ctx, node, key = \\"\\") { - return block1(); - } -}" -`; - exports[`t-out in components update properly on state changes 1`] = ` "function anonymous(bdom, helpers ) { diff --git a/tests/components/basics.test.ts b/tests/components/basics.test.ts index 2bd67aaf..2e12d33f 100644 --- a/tests/components/basics.test.ts +++ b/tests/components/basics.test.ts @@ -863,6 +863,43 @@ describe("basics", () => { "Child:mounted", ]).toBeLogged(); }); + + test("GrandChild display is controlled by its GrandParent", async () => { + class GrandChild extends Component { + static template = xml`
`; + setup() { + useLogLifecycle(); + } + } + + class Child extends Component { + static components = { GrandChild }; + static template = xml``; + } + + class Parent extends Component { + static template = xml``; + myComp = Child; + displayGrandChild = true; + } + + const parent = await mount(Parent, fixture); + expect(fixture.innerHTML).toBe("
"); + expect([ + "GrandChild:setup", + "GrandChild:willStart", + "GrandChild:willRender", + "GrandChild:rendered", + "GrandChild:mounted", + ]).toBeLogged(); + + parent.displayGrandChild = false; + parent.render(); + await nextTick(); + expect(fixture.innerHTML).toBe(""); + + expect(["GrandChild:willUnmount", "GrandChild:willDestroy"]).toBeLogged(); + }); }); describe("mount targets", () => {