From 374dbb2fd9302e2c72f4ce86356aaf1e086f8e46 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Tue, 18 Jan 2022 16:46:36 +0100 Subject: [PATCH] [FIX] components: cascading render after microtaskTick (makeChildFiber) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Samuel Degueldre Co-authored-by: François Georis --- src/component/component_node.ts | 10 ++-- src/component/error_handling.ts | 4 +- src/component/fibers.ts | 42 ++------------- src/component/scheduler.ts | 2 +- .../__snapshots__/concurrency.test.ts.snap | 52 +++++++++++++++++++ tests/components/concurrency.test.ts | 47 +++++++++++++++++ 6 files changed, 111 insertions(+), 46 deletions(-) diff --git a/src/component/component_node.ts b/src/component/component_node.ts index a2a89977..b0c97864 100644 --- a/src/component/component_node.ts +++ b/src/component/component_node.ts @@ -121,7 +121,7 @@ export class ComponentNode 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 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 // 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 _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 } component.props = props; this._render(fiber); - const parentRoot = parentFiber.root; + const parentRoot = parentFiber.root!; if (this.willPatch.length) { parentRoot.willPatch.push(fiber); } diff --git a/src/component/error_handling.ts b/src/component/error_handling.ts index d61a6d8c..f625b54e 100644 --- a/src/component/error_handling.ts +++ b/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) { diff --git a/src/component/fibers.ts b/src/component/fibers.ts index b90f3d3e..1da13453 100644 --- a/src/component/fibers.ts +++ b/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); diff --git a/src/component/scheduler.ts b/src/component/scheduler.ts index c1a9a274..2b868d08 100644 --- a/src/component/scheduler.ts +++ b/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(); } diff --git a/tests/components/__snapshots__/concurrency.test.ts.snap b/tests/components/__snapshots__/concurrency.test.ts.snap index b7fc5a0c..92623c8a 100644 --- a/tests/components/__snapshots__/concurrency.test.ts.snap +++ b/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 ) { diff --git a/tests/components/concurrency.test.ts b/tests/components/concurrency.test.ts index d95470c8..02ee1912 100644 --- a/tests/components/concurrency.test.ts +++ b/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``; + } + + class Child extends Component { + static components = { Element }; + static template = xml` + + + `; + state = state; + setup() { + child = this; + } + } + + class Parent extends Component { + static components = { Child }; + static template = xml` _ `; + 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 };