diff --git a/src/component/fibers.ts b/src/component/fibers.ts index c25296f6..744baa60 100644 --- a/src/component/fibers.ts +++ b/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(); } diff --git a/src/component/scheduler.ts b/src/component/scheduler.ts index 87095702..aa6f8b94 100644 --- a/src/component/scheduler.ts +++ b/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(); } } diff --git a/tests/components/__snapshots__/concurrency.test.ts.snap b/tests/components/__snapshots__/concurrency.test.ts.snap index b470bddd..55d79c02 100644 --- a/tests/components/__snapshots__/concurrency.test.ts.snap +++ b/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 ) { diff --git a/tests/components/concurrency.test.ts b/tests/components/concurrency.test.ts index c7e75c0a..1a152e2b 100644 --- a/tests/components/concurrency.test.ts +++ b/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`; + static components = { D }; + setup() { + useLogLifecycle("", true); + c = this; + } + } + let c: C; + + class B extends Component { + static template = xml`B`; + static components = { C }; + setup() { + useLogLifecycle("", true); + } + } + + class A extends Component { + static template = xml`A`; + 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; diff --git a/tests/helpers.ts b/tests/helpers.ts index 44574c00..e5c5b973 100644 --- a/tests/helpers.ts +++ b/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(() => { - expect(name + ": " + status(component)).toBe(name + ": " + "new"); - logStep(`${name}:willStart`); - }); + if (!skipAsyncHooks) { + onWillStart(() => { + expect(name + ": " + status(component)).toBe(name + ": " + "new"); + logStep(`${name}:willStart`); + }); + } onMounted(() => { expect(name + ": " + status(component)).toBe(name + ": " + "mounted"); logStep(`${name}:mounted`); }); - onWillUpdateProps(() => { - expect(name + ": " + status(component)).toBe(name + ": " + "mounted"); - logStep(`${name}:willUpdateProps`); - }); + if (!skipAsyncHooks) { + onWillUpdateProps(() => { + expect(name + ": " + status(component)).toBe(name + ": " + "mounted"); + logStep(`${name}:willUpdateProps`); + }); + } onWillRender(() => { logStep(`${name}:willRender`);