[FIX] components: cascading render after microtaskTick (makeChildFiber)

Co-authored-by: Samuel Degueldre <sad@odoo.com>
Co-authored-by: François Georis <fge@odoo.com>

src/component/component_node.ts

@@ -121,7 +121,7 @@ export class ComponentNode<T extends typeof Component = typeof Component>
   async initiateRender(fiber: Fiber | MountFiber) {
     this.fiber = fiber;
     if (this.mounted.length) {
-      fiber.root.mounted.push(fiber);
+      fiber.root!.mounted.push(fiber);
     }
     const component = this.component;
     try {
@@ -137,7 +137,7 @@ export class ComponentNode<T extends typeof Component = typeof Component>
 
   async render() {
     let current = this.fiber;
-    if (current && current.root.locked) {
+    if (current && current.root!.locked) {
       await Promise.resolve();
       // situation may have changed after the microtask tick
       current = this.fiber;
@@ -167,7 +167,7 @@ export class ComponentNode<T extends typeof Component = typeof Component>
     //   a root fiber to a child fiber in the previous microtick, because it was
     //   embedded in a rendering coming from above, so the fiber will be rendered
     //   in the next microtick anyway, so we should not render it again.
-    if (this.fiber && (current || !fiber.parent)) {
+    if (this.fiber === fiber && (current || !fiber.parent)) {
       this._render(fiber);
     }
   }
@@ -175,7 +175,7 @@ export class ComponentNode<T extends typeof Component = typeof Component>
   _render(fiber: Fiber | RootFiber) {
     try {
       fiber.bdom = this.renderFn();
-      fiber.root.counter--;
+      fiber.root!.counter--;
     } catch (e) {
       handleError({ node: this, error: e });
     }
@@ -218,7 +218,7 @@ export class ComponentNode<T extends typeof Component = typeof Component>
     }
     component.props = props;
     this._render(fiber);
-    const parentRoot = parentFiber.root;
+    const parentRoot = parentFiber.root!;
     if (this.willPatch.length) {
       parentRoot.willPatch.push(fiber);
     }

src/component/error_handling.ts

@@ -30,7 +30,7 @@ function _handleError(node: ComponentNode | null, error: any, isFirstRound = fal
 
     if (stopped) {
       if (isFirstRound && fiber) {
-        fiber.root.counter--;
+        fiber.root!.counter--;
       }
       return true;
     }
@@ -52,7 +52,7 @@ export function handleError(params: ErrorParams) {
     current = current.parent;
   } while (current);
 
-  fibersInError.set(fiber.root, error);
+  fibersInError.set(fiber.root!, error);
 
   const handled = _handleError(node, error, true);
   if (!handled) {

src/component/fibers.ts

@@ -3,46 +3,12 @@ import type { ComponentNode } from "./component_node";
 import { fibersInError, handleError } from "./error_handling";
 import { STATUS } from "./status";
 
-/**
- * Cleans on the root fiber the patch and willPatch fiber lists
- * It is typically needed when the same root fiber needs to recycle on
- * of its children or grandchildren's fiber.
- */
-function cleanPatchableFiber(child: Fiber, root: RootFiber) {
-  const { willPatch, patched } = root;
-  let i = willPatch.indexOf(child);
-  if (i > -1) {
-    willPatch.splice(i, 1);
-  }
-  i = patched.indexOf(child);
-  if (i > -1) {
-    patched.splice(i, 1);
-  }
-}
-
 export function makeChildFiber(node: ComponentNode, parent: Fiber): Fiber {
   let current = node.fiber;
   if (current) {
-    // current is necessarily a rootfiber here
     let root = parent.root;
-    const isSameRoot = current.root === root;
     cancelFibers(root, current.children);
-    current.children = [];
-    current.parent = parent;
-    // only increment our rendering if we were not
-    // already accounted for, or that we have been rendered
-    // already (in which case our fiber was removed from the root rendering)
-    if (!isSameRoot || current.bdom) {
-      root.counter++;
-    }
-
-    if (isSameRoot) {
-      cleanPatchableFiber(current, root);
-    }
-
-    current.bdom = null;
-    current.root = root;
-    return current;
+    current.root = null;
   }
   return new Fiber(node, parent);
 }
@@ -50,7 +16,7 @@ export function makeChildFiber(node: ComponentNode, parent: Fiber): Fiber {
 export function makeRootFiber(node: ComponentNode): Fiber {
   let current = node.fiber;
   if (current) {
-    let root = current.root;
+    let root = current.root!;
     root.counter -= cancelFibers(root, current.children);
     current.children = [];
     root.counter++;
@@ -92,7 +58,7 @@ function cancelFibers(root: any, fibers: Fiber[]): number {
 export class Fiber {
   node: ComponentNode;
   bdom: BDom | null = null;
-  root: RootFiber;
+  root: RootFiber | null; // A Fiber that has been replaced by another has no root
   parent: Fiber | null;
   children: Fiber[] = [];
   appliedToDom = false;
@@ -101,7 +67,7 @@ export class Fiber {
     this.node = node;
     this.parent = parent;
     if (parent) {
-      const root = parent.root;
+      const root = parent.root!;
       root.counter++;
       this.root = root;
       parent.children.push(this);

src/component/scheduler.ts

@@ -25,7 +25,7 @@ export class Scheduler {
   }
 
   addFiber(fiber: Fiber) {
-    this.tasks.add(fiber.root);
+    this.tasks.add(fiber.root!);
     if (!this.isRunning) {
       this.start();
     }

tests/components/__snapshots__/concurrency.test.ts.snap

@@ -1,5 +1,57 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Cascading renders after microtaskTick 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { prepareList, withKey } = helpers;
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2 = component(\`Child\`, {}, key + \`__1\`, node, ctx);
+    let b3 = text(\` _ \`);
+    ctx = Object.create(ctx);
+    const [k_block4, v_block4, l_block4, c_block4] = prepareList(ctx['state']);
+    for (let i1 = 0; i1 < l_block4; i1++) {
+      ctx[\`elem\`] = v_block4[i1];
+      let key1 = ctx['elem'].id;
+      c_block4[i1] = withKey(text(ctx['elem'].id), key1);
+    }
+    let b4 = list(c_block4);
+    return multi([b2, b3, b4]);
+  }
+}"
+`;
+
+exports[`Cascading renders after microtaskTick 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { prepareList, withKey } = helpers;
+  
+  return function template(ctx, node, key = \\"\\") {
+    ctx = Object.create(ctx);
+    const [k_block1, v_block1, l_block1, c_block1] = prepareList(ctx['state']);
+    for (let i1 = 0; i1 < l_block1; i1++) {
+      ctx[\`elem\`] = v_block1[i1];
+      let key1 = ctx['elem'].id;
+      c_block1[i1] = withKey(component(\`Element\`, {id: ctx['elem'].id}, key + \`__1__\${key1}\`, node, ctx), key1);
+    }
+    return list(c_block1);
+  }
+}"
+`;
+
+exports[`Cascading renders after microtaskTick 3`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(ctx['props'].id);
+  }
+}"
+`;
+
 exports[`async rendering destroying a widget before start is over 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/components/concurrency.test.ts

@@ -3082,6 +3082,53 @@ test("t-foreach with dynamic async component", async () => {
     "Child (3):mounted",
   ]).toBeLogged();
 });
+
+test("Cascading renders after microtaskTick", async () => {
+  const state = [{ id: 0 }, { id: 1 }];
+  let child: any;
+  let parent: any;
+
+  class Element extends Component {
+    static template = xml`<t t-esc="props.id" />`;
+  }
+
+  class Child extends Component {
+    static components = { Element };
+    static template = xml`
+      <t t-foreach="state" t-as="elem" t-key="elem.id">
+        <Element id="elem.id"/>
+      </t>`;
+    state = state;
+    setup() {
+      child = this;
+    }
+  }
+
+  class Parent extends Component {
+    static components = { Child };
+    static template = xml`<Child /> _ <t t-foreach="state" t-as="elem" t-key="elem.id" t-esc="elem.id"/>`;
+    state = state;
+    setup() {
+      parent = this;
+    }
+  }
+
+  await mount(Parent, fixture);
+  expect(fixture.innerHTML).toBe("01 _ 01");
+
+  state.push({ id: 2 });
+  parent.render();
+  child.render();
+
+  await Promise.resolve();
+  expect(fixture.innerHTML).toBe("01 _ 01");
+  state.push({ id: 3 });
+  parent.render();
+  child.render();
+
+  await nextTick();
+  expect(fixture.innerHTML).toBe("0123 _ 0123");
+});
 //   test.skip("components with shouldUpdate=false", async () => {
 //     const state = { p: 1, cc: 10 };