[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

@@ -136,13 +136,19 @@ export class ComponentNode<T extends typeof Component = typeof Component>
   }
 
   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);

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 });
     }

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(\`<abc><block-text-0/></abc>\`);
+  let block1 = createBlock(\`<abc><block-text-0/><block-text-1/></abc>\`);
   
   return function template(ctx, node, key = \\"\\") {
     let d1 = ctx['props'].val;
-    return block1([d1]);
+    let d2 = ctx['mounted'];
+    return block1([d1, d2]);
   }
 }"
 `;

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(\`<span><block-text-0/></span>\`);
+  
+  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(\`<span><block-text-0/></span>\`);
+  
+  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(\`<span><block-text-0/></span>\`);
+  
+  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
 ) {

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`<abc><t t-esc="props.val"/></abc>`;
+    static template = xml`<abc><t t-esc="props.val"/><t t-esc="mounted" /></abc>`;
+    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("<div><abc>Panel1Mounted</abc></div>");
+
   parent.state.panel = "Panel2";
   await nextTick();
   expect(fixture.innerHTML).toBe("<div><abc>Panel2</abc></div>");
@@ -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("<div><abc>Panel2Mounted</abc></div>");
+  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("<div></div>");
+  expect(["Parent:willPatch", "Parent:patched"]).toBeLogged();
 });
 
 test("parent and child rendered at exact same time", async () => {

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`<span t-esc="patched"/>`;
+      patched: any;
+      setup() {
+        useLogLifecycle();
+        onMounted(() => {
+          this.patched = "Patched";
+          this.render();
+        });
+      }
+    }
+
+    await mount(Parent, fixture);
+    expect(fixture.innerHTML).toBe("<span></span>");
+    expect([
+      "Parent:setup",
+      "Parent:willStart",
+      "Parent:willRender",
+      "Parent:rendered",
+      "Parent:mounted",
+      "Parent:willRender",
+      "Parent:rendered",
+    ]).toBeLogged();
+
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<span>Patched</span>");
+    expect(["Parent:willPatch", "Parent:patched"]).toBeLogged();
+  });
+
+  test("render in patched", async () => {
+    class Parent extends Component {
+      static template = xml`<span t-esc="patched"/>`;
+      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("<span></span>");
+    expect([
+      "Parent:setup",
+      "Parent:willStart",
+      "Parent:willRender",
+      "Parent:rendered",
+      "Parent:mounted",
+    ]).toBeLogged();
+
+    parent.render();
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<span></span>");
+    expect([
+      "Parent:willRender",
+      "Parent:rendered",
+      "Parent:willPatch",
+      "Parent:patched",
+      "Parent:willRender",
+      "Parent:rendered",
+    ]).toBeLogged();
+
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<span>Patched</span>");
+    expect(["Parent:willPatch", "Parent:patched"]).toBeLogged();
+  });
+
+  test("render in willPatch", async () => {
+    class Parent extends Component {
+      static template = xml`<span t-esc="patched"/>`;
+      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("<span></span>");
+    expect([
+      "Parent:setup",
+      "Parent:willStart",
+      "Parent:willRender",
+      "Parent:rendered",
+      "Parent:mounted",
+    ]).toBeLogged();
+
+    parent.render();
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<span></span>");
+
+    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("<span>Patched</span>");
+  });
 });