[FIX] issue with delayed renders being left pending forever

src/component/fibers.ts

@@ -8,10 +8,6 @@ export function makeChildFiber(node: ComponentNode, parent: Fiber): Fiber {
   if (current) {
     cancelFibers(current.children);
     current.root = null;
-    if (current instanceof RootFiber && current.delayedRenders.length) {
-      let root = parent.root!;
-      root.delayedRenders = root.delayedRenders.concat(current.delayedRenders);
-    }
   }
   return new Fiber(node, parent);
 }
@@ -92,17 +88,18 @@ export class Fiber {
   render() {
     // if some parent has a fiber => register in followup
     let prev = this.root!.node;
+    let scheduler = prev.app.scheduler;
     let current = prev.parent;
     while (current) {
       if (current.fiber) {
         let root = current.fiber.root!;
         if (root.counter) {
-          root.delayedRenders.push(this);
+          scheduler.delayedRenders.push(this);
           return;
         } else {
           if (!root.reachedChildren.has(prev)) {
-            // is dead
-            this.node.app.scheduler.shouldClear = true;
+            // is dead. but we keep the render around just in case
+            scheduler.delayedRenders.push(this);
             return;
           }
           current = root.node;
@@ -141,7 +138,6 @@ export class RootFiber extends Fiber {
   // i.e.: render triggered in onWillUnmount or in willPatch will be delayed
   locked: boolean = false;
 
-  delayedRenders: Fiber[] = [];
   reachedChildren: WeakSet<ComponentNode> = new WeakSet();
 
   complete() {
@@ -198,14 +194,6 @@ export class RootFiber extends Fiber {
   setCounter(newValue: number) {
     this.counter = newValue;
     if (newValue === 0) {
-      if (this.delayedRenders.length) {
-        for (let f of this.delayedRenders) {
-          if (f.root) {
-            f.render();
-          }
-        }
-        this.delayedRenders = [];
-      }
       this.node.app.scheduler.flush();
     }
   }

src/component/scheduler.ts

@@ -13,7 +13,7 @@ export class Scheduler {
   tasks: Set<RootFiber> = new Set();
   requestAnimationFrame: Window["requestAnimationFrame"];
   frame: number = 0;
-  shouldClear: boolean = false;
+  delayedRenders: Fiber[] = [];
 
   constructor() {
     this.requestAnimationFrame = Scheduler.requestAnimationFrame;
@@ -28,16 +28,23 @@ export class Scheduler {
    * Other tasks are left unchanged.
    */
   flush() {
+    if (this.delayedRenders.length) {
+      let renders = this.delayedRenders;
+      this.delayedRenders = [];
+      for (let f of renders) {
+        if (f.root) {
+          f.render();
+        }
+      }
+    }
+
     if (this.frame === 0) {
       this.frame = this.requestAnimationFrame(() => {
         this.frame = 0;
         this.tasks.forEach((fiber) => this.processFiber(fiber));
-        if (this.shouldClear) {
-          this.shouldClear = false;
-          for (let task of this.tasks) {
-            if (task.node.status === STATUS.DESTROYED) {
-              this.tasks.delete(task);
-            }
+        for (let task of this.tasks) {
+          if (task.node.status === STATUS.DESTROYED) {
+            this.tasks.delete(task);
           }
         }
       });

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

@@ -52,6 +52,50 @@ exports[`Cascading renders after microtaskTick 3`] = `
 }"
 `;
 
+exports[`another scenario with delayed rendering 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    b2 = text(\`A\`);
+    if (ctx['state'].value<15) {
+      b3 = component(\`B\`, {value: ctx['state'].value}, key + \`__1\`, node, ctx);
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`another scenario with delayed rendering 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(ctx['props'].value);
+    const b3 = component(\`C\`, {}, key + \`__1\`, node, ctx);
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`another scenario with delayed rendering 3`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<button block-handler-0=\\"click\\"><block-text-1/></button>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let hdlr1 = [ctx['increment'], ctx];
+    let txt1 = ctx['state'].val;
+    return block1([hdlr1, txt1]);
+  }
+}"
+`;
+
 exports[`async rendering destroying a widget before start is over 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/components/concurrency.test.ts

@@ -3591,6 +3591,105 @@ test("delayed rendering, reusing fiber then component is destroyed and  stuff",
   ]).toBeLogged();
 });
 
+test("another scenario with delayed rendering", async () => {
+  let prom1 = makeDeferred();
+  let onSecondRenderA = makeDeferred();
+
+  class C extends Component {
+    static template = xml`<button t-on-click="increment"><t t-esc="state.val"/></button>`;
+    state = useState({ val: 1 });
+    setup() {
+      useLogLifecycle();
+    }
+    increment() {
+      this.state.val++;
+    }
+  }
+
+  class B extends Component {
+    static template = xml`<t t-esc="props.value"/><C />`;
+    static components = { C };
+    setup() {
+      useLogLifecycle();
+      onWillUpdateProps(() => prom1);
+    }
+  }
+
+  class A extends Component {
+    static template = xml`A<t t-if="state.value lt 15"><B value="state.value"/></t>`;
+    static components = { B };
+    state = useState({ value: 3 });
+    setup() {
+      useLogLifecycle();
+      let n = 0;
+      onRendered(() => {
+        n++;
+        if (n === 2) {
+          onSecondRenderA.resolve();
+        }
+      });
+    }
+  }
+
+  const parent = await mount(A, fixture);
+  expect(fixture.innerHTML).toBe("A3<button>1</button>");
+  expect([
+    "A:setup",
+    "A:willStart",
+    "A:willRender",
+    "B:setup",
+    "B:willStart",
+    "A:rendered",
+    "B:willRender",
+    "C:setup",
+    "C:willStart",
+    "B:rendered",
+    "C:willRender",
+    "C:rendered",
+    "C:mounted",
+    "B:mounted",
+    "A:mounted",
+  ]).toBeLogged();
+
+  // initiate a render in A, but is blocked in B
+  parent.state.value = 5;
+  await nextTick();
+  expect(["A:willRender", "B:willUpdateProps", "A:rendered"]).toBeLogged();
+
+  // initiate a render in C (will be delayed because of render in A)
+  fixture.querySelector("button")!.click();
+  await nextTick();
+  expect([]).toBeLogged();
+
+  // initiate a render in A, that will destroy B
+  parent.state.value = 23;
+  await onSecondRenderA;
+  await nextMicroTick();
+  expect(["A:willRender", "A:rendered"]).toBeLogged();
+
+  // rerender A, but without destroying B
+  parent.state.value = 7;
+  await nextTick();
+  expect(["A:willRender", "B:willUpdateProps", "A:rendered"]).toBeLogged();
+
+  prom1.resolve();
+  await nextTick();
+  expect(fixture.innerHTML).toBe("A7<button>2</button>");
+
+  expect([
+    "B:willRender",
+    "B:rendered",
+    "C:willRender",
+    "C:rendered",
+    "A:willPatch",
+    "B:willPatch",
+    "B:patched",
+    "A:patched",
+    "C:willPatch",
+    "C:patched",
+  ]).toBeLogged();
+});
+
 //   test.skip("components with shouldUpdate=false", async () => {
 //     const state = { p: 1, cc: 10 };