[FIX] concurrency: do not render delayed fibers when cancelled

Previously, if a fiber was delayed because one of its ancestors was
rendering, and that fiber was not a root fiber, it would be rendered
after all its ancestors had finished rendering even if one of those
ancestor renderings cancelled it.

This commit fixes that by simply checking that a delayed fiber is still
its component node's current fiber before rendering it.

src/component/fibers.ts

@@ -42,6 +42,10 @@ export function makeRootFiber(node: ComponentNode): Fiber {
   return fiber;
 }
 
+function throwOnRender() {
+  throw new Error("Attempted to render cancelled fiber");
+}
+
 /**
  * @returns number of not-yet rendered fibers cancelled
  */
@@ -49,6 +53,7 @@ function cancelFibers(fibers: Fiber[]): number {
   let result = 0;
   for (let fiber of fibers) {
     let node = fiber.node;
+    fiber.render = throwOnRender;
     if (node.status === STATUS.NEW) {
       node.destroy();
     }

src/component/scheduler.ts

@@ -32,7 +32,7 @@ export class Scheduler {
       let renders = this.delayedRenders;
       this.delayedRenders = [];
       for (let f of renders) {
-        if (f.root && f.node.status !== STATUS.DESTROYED) {
+        if (f.root && f.node.status !== STATUS.DESTROYED && f.node.fiber === f) {
           f.render();
         }
       }

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

@@ -1111,6 +1111,56 @@ exports[`delay willUpdateProps with rendering grandchild 4`] = `
 }"
 `;
 
+exports[`delayed fiber does not get rendered if it was cancelled 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`A\`);
+    const b3 = component(\`B\`, {}, key + \`__1\`, node, ctx);
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`delayed fiber does not get rendered if it was cancelled 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`B\`);
+    const b3 = component(\`C\`, {}, key + \`__1\`, node, ctx);
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`delayed fiber does not get rendered if it was cancelled 3`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`C\`);
+    const b3 = component(\`D\`, {}, key + \`__1\`, node, ctx);
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`delayed fiber does not get rendered if it was cancelled 4`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`D\`);
+  }
+}"
+`;
+
 exports[`delayed rendering, but then initial rendering is cancelled by yet another render 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/components/concurrency.test.ts

@@ -3696,6 +3696,89 @@ test("another scenario with delayed rendering", async () => {
   ]).toBeLogged();
 });
 
+test("delayed fiber does not get rendered if it was cancelled", async () => {
+  class D extends Component {
+    static template = xml`D`;
+    setup() {
+      useLogLifecycle("", true);
+    }
+  }
+
+  class C extends Component {
+    static template = xml`C<D/>`;
+    static components = { D };
+    setup() {
+      useLogLifecycle("", true);
+      c = this;
+    }
+  }
+  let c: C;
+
+  class B extends Component {
+    static template = xml`B<C/>`;
+    static components = { C };
+    setup() {
+      useLogLifecycle("", true);
+    }
+  }
+
+  class A extends Component {
+    static template = xml`A<B/>`;
+    static components = { B };
+    setup() {
+      useLogLifecycle("", true);
+    }
+  }
+
+  const a = await mount(A, fixture);
+  expect(fixture.innerHTML).toBe("ABCD");
+  expect([
+    "A:setup",
+    "A:willRender",
+    "B:setup",
+    "A:rendered",
+    "B:willRender",
+    "C:setup",
+    "B:rendered",
+    "C:willRender",
+    "D:setup",
+    "C:rendered",
+    "D:willRender",
+    "D:rendered",
+    "D:mounted",
+    "C:mounted",
+    "B:mounted",
+    "A:mounted",
+  ]).toBeLogged();
+  // Start a render in C
+  c!.render(true);
+  await nextMicroTick();
+  expect(["C:willRender", "C:rendered"]).toBeLogged();
+  // Start a render in A such that C is already rendered, but D will be delayed
+  // (because A is rendering) then cancelled (when the render from A reaches C)
+  a.render(true);
+  // Make sure the render can go to completion (Cancelled fibers will throw when rendered)
+  await nextTick();
+  expect([
+    "A:willRender",
+    "A:rendered",
+    "B:willRender",
+    "B:rendered",
+    "C:willRender",
+    "C:rendered",
+    "D:willRender",
+    "D:rendered",
+    "A:willPatch",
+    "B:willPatch",
+    "C:willPatch",
+    "D:willPatch",
+    "D:patched",
+    "C:patched",
+    "B:patched",
+    "A:patched",
+  ]).toBeLogged();
+});
+
 test("destroyed component causes other soon to be destroyed component to rerender, weird stuff happens", async () => {
   let def = makeDeferred();
   let c: any = null;

tests/helpers.ts

@@ -138,7 +138,7 @@ const steps: string[] = [];
 export function logStep(step: string) {
   steps.push(step);
 }
-export function useLogLifecycle(key?: string) {
+export function useLogLifecycle(key?: string, skipAsyncHooks: boolean = false) {
   const component = useComponent();
   let name = component.constructor.name;
   if (key) {
@@ -147,20 +147,24 @@ export function useLogLifecycle(key?: string) {
   logStep(`${name}:setup`);
   expect(name + ": " + status(component)).toBe(name + ": " + "new");
 
-  onWillStart(() => {
+  if (!skipAsyncHooks) {
-    expect(name + ": " + status(component)).toBe(name + ": " + "new");
+    onWillStart(() => {
-    logStep(`${name}:willStart`);
+      expect(name + ": " + status(component)).toBe(name + ": " + "new");
-  });
+      logStep(`${name}:willStart`);
+    });
+  }
 
   onMounted(() => {
     expect(name + ": " + status(component)).toBe(name + ": " + "mounted");
     logStep(`${name}:mounted`);
   });
 
-  onWillUpdateProps(() => {
+  if (!skipAsyncHooks) {
-    expect(name + ": " + status(component)).toBe(name + ": " + "mounted");
+    onWillUpdateProps(() => {
-    logStep(`${name}:willUpdateProps`);
+      expect(name + ": " + status(component)).toBe(name + ": " + "mounted");
-  });
+      logStep(`${name}:willUpdateProps`);
+    });
+  }
 
   onWillRender(() => {
     logStep(`${name}:willRender`);