From 49a5e3b18a985d08db44af75c42a26fb90ca12a4 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 19 Oct 2023 12:41:03 +0900 Subject: [PATCH] refactor(tiny-react): simplify placeChild --- packages/tiny-react/src/hooks.ts | 5 +- packages/tiny-react/src/index.test.ts | 28 ++++++++ packages/tiny-react/src/reconciler.ts | 91 +++++++++++++++++++++++++- packages/tiny-react/src/utils.ts | 3 + packages/tiny-react/src/virtual-dom.ts | 13 ++++ 5 files changed, 133 insertions(+), 7 deletions(-) create mode 100644 packages/tiny-react/src/utils.ts diff --git a/packages/tiny-react/src/hooks.ts b/packages/tiny-react/src/hooks.ts index 6d6d8c15..60f4915a 100644 --- a/packages/tiny-react/src/hooks.ts +++ b/packages/tiny-react/src/hooks.ts @@ -1,4 +1,5 @@ import { tinyassert } from "@hiogawa/utils"; +import { isEqualShallow } from "./utils"; // not particularly intentional but this hook module doesn't depend on any of reconciler/virtual-dom logic, // which tells that the hook idea itself is a general concept applicable to functional api...? @@ -216,7 +217,3 @@ function defineHook(implement: (ctx: HookContext) => T): T { }, }) as T; } - -function isEqualShallow(xs: unknown[], ys: unknown[]) { - return xs.length === ys.length && xs.every((x, i) => x === ys[i]); -} diff --git a/packages/tiny-react/src/index.test.ts b/packages/tiny-react/src/index.test.ts index 136e47e8..39cb531c 100644 --- a/packages/tiny-react/src/index.test.ts +++ b/packages/tiny-react/src/index.test.ts @@ -80,6 +80,14 @@ describe(render, () => { }, }, ], + "hrange": [ + hello, + + world + , + ], "parent": [Circular], "slot": hello, "type": "fragment", @@ -254,6 +262,12 @@ describe(render, () => { }, }, ], + "hrange": [ + + hello + , + world, + ], "parent": [Circular], "slot": hello @@ -332,6 +346,20 @@ describe(render, () => { world , + "hrange": [ +
+ + hello + + world +
, +
+ + hello + + world +
, + ], "parent": undefined, "slot":
diff --git a/packages/tiny-react/src/reconciler.ts b/packages/tiny-react/src/reconciler.ts index d19cda07..46a05b4b 100644 --- a/packages/tiny-react/src/reconciler.ts +++ b/packages/tiny-react/src/reconciler.ts @@ -1,5 +1,6 @@ import { tinyassert } from "@hiogawa/utils"; import { HookContext } from "./hooks"; +import { isEqualShallow } from "./utils"; import { type BCustom, type BFragment, @@ -19,6 +20,7 @@ import { type VNode, getBNodeKey, getBNodeParent, + getBNodeRange, getBNodeSlot, getVNodeKey, setBNodeParent, @@ -142,12 +144,16 @@ function reconcileNode( // https://github.com/yewstack/yew/blob/30e2d548ef57a4b738fb285251b986467ef7eb95/packages/yew/src/dom_bundle/blist.rs#L416 // https://github.com/snabbdom/snabbdom/blob/420fa78abe98440d24e2c5af2f683e040409e0a6/src/init.ts#L289 // https://github.com/WebReflection/udomdiff/blob/8923d4fac63a40c72006a46eb0af7bfb5fdef282/index.js + let isReordering = false; if (bnode.type === NODE_TYPE_FRAGMENT && bnode.vnode.key === vnode.key) { const [newChildren, oldChildren] = alignChildrenByKey( vnode.children, bnode.children ); - bnode.children = newChildren; + if (!isEqualShallow(bnode.children, newChildren)) { + isReordering = true; + bnode.children = newChildren; + } for (const bnode of oldChildren) { unmount(bnode); } @@ -169,6 +175,7 @@ function reconcileNode( } // reconcile vnode.children bnode.slot = undefined; + bnode.hrange = undefined; for (let i = vnode.children.length - 1; i >= 0; i--) { const bchild = reconcileNode( vnode.children[i], @@ -178,6 +185,20 @@ function reconcileNode( effectManager, isHydrate ); + const hrange = getBNodeRange(bchild); + if (hrange) { + if (!bnode.hrange) { + bnode.hrange = [...hrange]; + } else { + bnode.hrange[0] = hrange[0]; + } + if (isReordering) { + // TODO: this should replace each `placeChild` when mutating VTag, VText + // TODO: slot-fixup in updateCustomNode also has to be adjusted + // placeBNode(bchild, hparent, hnextSibling); + placeBNode; + } + } hnextSibling = getBNodeSlot(bchild) ?? hnextSibling; bnode.slot = getBNodeSlot(bchild) ?? bnode.slot; bnode.children[i] = bchild; @@ -225,6 +246,7 @@ function reconcileNode( bnode.hparent = hparent; setBNodeParent(bnode.child, bnode); bnode.slot = getBNodeSlot(bnode.child); + bnode.hrange = getBNodeRange(bnode.child); effectManager.effectNodes.push(bnode); // expose self re-rendering for hooks @@ -287,14 +309,40 @@ function hydrateNode( return vnode satisfies never; } +// TODO: should use it only for "mounting" case function placeChild( hparent: HNode, hnode: HNode, hnextSibling: HNode | null, - init: boolean + isMount: boolean +) { + if (isMount || hnode.nextSibling !== hnextSibling) { + hparent.insertBefore(hnode, hnextSibling); + } +} + +function placeChildrenRange( + first: HNode, + last: HNode, + hparent: HNode, + hnextSibling: HNode | null ) { - if (init || hnode.nextSibling !== hnextSibling) { + let hnode = last; + while (true) { hparent.insertBefore(hnode, hnextSibling); + if (first === hnode) { + break; + } + hnextSibling = hnode; + tinyassert(hnode.previousSibling); + hnode = hnode.previousSibling; + } +} + +function placeBNode(bnode: BNode, hparent: HNode, hnextSibling: HNode | null) { + const hrange = getBNodeRange(bnode); + if (hrange) { + placeChildrenRange(hrange[0], hrange[1], hparent, hnextSibling); } } @@ -311,6 +359,7 @@ export function updateCustomNode(vnode: VCustom, bnode: BCustom) { } const oldSlot = getBNodeSlot(bnode); + const oldRange = getBNodeRange(bnode); // traverse ancestors to find "slot" const hnextSibling = findNextSibling(bnode); @@ -327,6 +376,15 @@ export function updateCustomNode(vnode: VCustom, bnode: BCustom) { ); tinyassert(newBnode === bnode); // reconciled over itself without unmount (i.e. should be same `key` and `render`) + // fix up ancestors range + const newRange = getBNodeRange(bnode); + if ( + oldRange !== newRange || + (oldRange && newRange && !isEqualShallow(oldRange, newRange)) + ) { + updateParentRange(bnode); + } + // fix up ancestors slot const newSlot = getBNodeSlot(bnode); if (oldSlot !== newSlot) { @@ -390,6 +448,33 @@ function updateParentSlot(child: BNode) { } } +function updateParentRange(child: BNode) { + let parent = getBNodeParent(child); + while (parent) { + if (parent.type === NODE_TYPE_TAG) { + return; + } + if (parent.type === NODE_TYPE_CUSTOM) { + parent.hrange = getBNodeRange(child); + } + if (parent.type === NODE_TYPE_FRAGMENT) { + parent.hrange = undefined; + for (const c of parent.children) { + const hrange = getBNodeRange(c); + if (hrange) { + if (!parent.hrange) { + parent.hrange = [...hrange]; + } else { + parent.hrange[0] = hrange[0]; + } + } + } + } + child = parent; + parent = child.parent; + } +} + function alignChildrenByKey( vnodes: VNode[], bnodes: BNode[] diff --git a/packages/tiny-react/src/utils.ts b/packages/tiny-react/src/utils.ts new file mode 100644 index 00000000..66e05a7b --- /dev/null +++ b/packages/tiny-react/src/utils.ts @@ -0,0 +1,3 @@ +export function isEqualShallow(xs: unknown[], ys: unknown[]) { + return xs.length === ys.length && xs.every((x, i) => x === ys[i]); +} diff --git a/packages/tiny-react/src/virtual-dom.ts b/packages/tiny-react/src/virtual-dom.ts index 3e563d09..e2dba01f 100644 --- a/packages/tiny-react/src/virtual-dom.ts +++ b/packages/tiny-react/src/virtual-dom.ts @@ -91,7 +91,9 @@ export type BCustom = { vnode: VCustom; parent?: BNodeParent; child: BNode; + // TOD: replace `slot` with `hrange` slot?: HNode; + hrange?: [HNode, HNode]; hparent?: HNode; // undefined after unmounted (this flag seems necessary to skip already scheduled re-rendering after unmount) hookContext: HookContext; }; @@ -102,6 +104,7 @@ export type BFragment = { parent?: BNodeParent; children: BNode[]; slot?: HNode; + hrange?: [HNode, HNode]; }; export const EMPTY_NODE: VEmpty = { @@ -141,6 +144,16 @@ export function getBNodeSlot(node: BNode): HNode | undefined { return node.slot; } +export function getBNodeRange(node: BNode): [HNode, HNode] | undefined { + if (node.type === NODE_TYPE_EMPTY) { + return; + } + if (node.type === NODE_TYPE_TAG || node.type === NODE_TYPE_TEXT) { + return [node.hnode, node.hnode]; + } + return node.hrange; +} + // bnode parent traversal is only for BCustom and BFragment export function getBNodeParent(node: BNode): BNodeParent | undefined { if (node.type === NODE_TYPE_CUSTOM || node.type === NODE_TYPE_FRAGMENT) {