[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

@@ -275,8 +275,11 @@ export class ComponentNode<T extends typeof Component = typeof Component>
   }
 
   patch() {
-    this.bdom!.patch(this!.fiber!.bdom!, false);
+    const hasChildren = Object.keys(this.children).length > 0;
-    this.cleanOutdatedChildren();
+    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<T extends typeof Component = typeof Component>
   }
 
   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];

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.patch();
-      node.cleanOutdatedChildren();
-      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;

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(\`<div/>\`);
+  
+  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(\`<div/>\`);
-  
-  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(\`<div/>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    return block1();
-  }
-}"
-`;
-
 exports[`t-out in components update properly on state changes 1`] = `
 "function anonymous(bdom, helpers
 ) {

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`<div />`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Child extends Component {
+      static components = { GrandChild };
+      static template = xml`<GrandChild t-if="props.displayGrandChild" />`;
+    }
+
+    class Parent extends Component {
+      static template = xml`<t t-component="myComp" displayGrandChild="displayGrandChild"/>`;
+      myComp = Child;
+      displayGrandChild = true;
+    }
+
+    const parent = await mount(Parent, fixture);
+    expect(fixture.innerHTML).toBe("<div></div>");
+    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", () => {