From e04582644f91ce4a240dd3e7d362b0e3eda2408b Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 17 Oct 2023 13:36:33 +0900 Subject: [PATCH] refactor(tiny-react): remove redundant `BNode.parent` (#167) --- packages/tiny-react/src/compat/index.ts | 4 +-- packages/tiny-react/src/helper/hyperscript.ts | 4 +-- packages/tiny-react/src/index.test.ts | 10 ------ packages/tiny-react/src/reconciler.ts | 28 +++++++-------- packages/tiny-react/src/virtual-dom.ts | 35 +++++++++---------- 5 files changed, 34 insertions(+), 47 deletions(-) diff --git a/packages/tiny-react/src/compat/index.ts b/packages/tiny-react/src/compat/index.ts index 7182c2f2..61f0db99 100644 --- a/packages/tiny-react/src/compat/index.ts +++ b/packages/tiny-react/src/compat/index.ts @@ -1,6 +1,6 @@ import { useEffect, useRef, useState } from "../hooks"; import { render } from "../reconciler"; -import { type BNode, EMPTY_VNODE, type FC, type VNode } from "../virtual-dom"; +import { type BNode, EMPTY_NODE, type FC, type VNode } from "../virtual-dom"; // non comprehensive compatibility features @@ -33,7 +33,7 @@ export function createRoot(container: Element) { bnode = render(vnode, container, bnode); }, unmount() { - render(EMPTY_VNODE, container, bnode); + render(EMPTY_NODE, container, bnode); }, }; } diff --git a/packages/tiny-react/src/helper/hyperscript.ts b/packages/tiny-react/src/helper/hyperscript.ts index e06820e0..a11cb8da 100644 --- a/packages/tiny-react/src/helper/hyperscript.ts +++ b/packages/tiny-react/src/helper/hyperscript.ts @@ -1,5 +1,5 @@ import { - EMPTY_VNODE, + EMPTY_NODE, NODE_TYPE_CUSTOM, NODE_TYPE_FRAGMENT, NODE_TYPE_TAG, @@ -80,7 +80,7 @@ function normalizeComponentChild(child: ComponentChild): VNode { typeof child === "undefined" || typeof child === "boolean" ) { - return EMPTY_VNODE; + return EMPTY_NODE; } if (typeof child === "string" || typeof child === "number") { return { diff --git a/packages/tiny-react/src/index.test.ts b/packages/tiny-react/src/index.test.ts index bdd9adc7..00fcc131 100644 --- a/packages/tiny-react/src/index.test.ts +++ b/packages/tiny-react/src/index.test.ts @@ -43,7 +43,6 @@ describe(render, () => { "children": [ { "hnode": hello, - "parent": [Circular], "type": "text", "vnode": { "data": "hello", @@ -53,7 +52,6 @@ describe(render, () => { { "child": { "hnode": world, - "parent": [Circular], "type": "text", "vnode": { "data": "world", @@ -66,7 +64,6 @@ describe(render, () => { world , "listeners": Map {}, - "parent": [Circular], "type": "tag", "vnode": { "child": { @@ -124,7 +121,6 @@ describe(render, () => { , "listeners": Map {}, - "parent": undefined, "type": "tag", "vnode": { "child": { @@ -174,7 +170,6 @@ describe(render, () => { { "child": { "hnode": reconcile, - "parent": [Circular], "type": "text", "vnode": { "data": "reconcile", @@ -187,7 +182,6 @@ describe(render, () => { reconcile , "listeners": Map {}, - "parent": undefined, "type": "tag", "vnode": { "child": { @@ -232,7 +226,6 @@ describe(render, () => { { "child": { "hnode": hello, - "parent": [Circular], "type": "text", "vnode": { "data": "hello", @@ -243,7 +236,6 @@ describe(render, () => { hello , "listeners": Map {}, - "parent": [Circular], "type": "tag", "vnode": { "child": { @@ -259,7 +251,6 @@ describe(render, () => { }, { "hnode": world, - "parent": [Circular], "type": "text", "vnode": { "data": "world", @@ -298,7 +289,6 @@ describe(render, () => { world , "listeners": Map {}, - "parent": [Circular], "type": "tag", "vnode": { "child": { diff --git a/packages/tiny-react/src/reconciler.ts b/packages/tiny-react/src/reconciler.ts index 8ca5e76f..fdf61571 100644 --- a/packages/tiny-react/src/reconciler.ts +++ b/packages/tiny-react/src/reconciler.ts @@ -6,6 +6,7 @@ import { type BNode, type BTag, type BText, + EMPTY_NODE, type HNode, NODE_TYPE_CUSTOM, NODE_TYPE_EMPTY, @@ -16,17 +17,18 @@ import { type Props, type VCustom, type VNode, - emptyBNode, getBNodeKey, + getBNodeParent, getBNodeSlot, getVNodeKey, + setBNodeParent, } from "./virtual-dom"; export function render(vnode: VNode, parent: HNode, bnode?: BNode) { const effectManager = new EffectManager(); const newBnode = reconcileNode( vnode, - bnode ?? emptyBNode(), + bnode ?? EMPTY_NODE, parent, undefined, effectManager @@ -47,7 +49,7 @@ function reconcileNode( if (bnode.type === NODE_TYPE_EMPTY) { } else { unmount(bnode); - bnode = emptyBNode(); + bnode = EMPTY_NODE; } } else if (vnode.type === NODE_TYPE_TAG) { if ( @@ -71,7 +73,7 @@ function reconcileNode( const hnode = document.createElement(vnode.name); const child = reconcileNode( vnode.child, - emptyBNode(), + EMPTY_NODE, hnode, undefined, effectManager @@ -82,13 +84,12 @@ function reconcileNode( child, hnode, listeners: new Map(), - parent: undefined, } satisfies BTag; reconcileTagProps(bnode, vnode.props, {}); placeChild(bnode.hnode, hparent, preSlot, true); effectManager.refNodes.push(bnode); } - bnode.child.parent = bnode; + setBNodeParent(bnode.child, bnode); } else if (vnode.type === NODE_TYPE_TEXT) { if (bnode.type === NODE_TYPE_TEXT) { if (bnode.vnode.data !== vnode.data) { @@ -103,7 +104,6 @@ function reconcileNode( type: vnode.type, vnode, hnode, - parent: undefined, } satisfies BText; placeChild(bnode.hnode, hparent, preSlot, true); } @@ -142,7 +142,7 @@ function reconcileNode( for (let i = 0; i < vnode.children.length; i++) { const bchild = reconcileNode( vnode.children[i], - bchildren[i] ?? emptyBNode(), + bchildren[i] ?? EMPTY_NODE, hparent, preSlot, effectManager @@ -150,7 +150,7 @@ function reconcileNode( preSlot = getBNodeSlot(bchild) ?? preSlot; bnode.slot = getBNodeSlot(bchild) ?? bnode.slot; bnode.children[i] = bchild; - bchild.parent = bnode; + setBNodeParent(bchild, bnode); } } else if (vnode.type === NODE_TYPE_CUSTOM) { if ( @@ -174,7 +174,7 @@ function reconcileNode( const vchild = hookContext.wrap(() => vnode.render(vnode.props)); const child = reconcileNode( vchild, - emptyBNode(), + EMPTY_NODE, hparent, preSlot, effectManager @@ -190,7 +190,7 @@ function reconcileNode( } satisfies BCustom; } bnode.hparent = hparent; - bnode.child.parent = bnode; + setBNodeParent(bnode.child, bnode); bnode.slot = getBNodeSlot(bnode.child); effectManager.effectNodes.push(bnode); @@ -256,7 +256,7 @@ export function updateCustomNode(vnode: VCustom, bnode: BCustom) { } function findPreviousSlot(child: BNode): HNode | undefined { - let parent = child.parent; + let parent = getBNodeParent(child); while (parent) { if (parent.type === NODE_TYPE_TAG) { // no slot i.e. new node will be the first child within parent tag @@ -287,7 +287,7 @@ function findPreviousSlot(child: BNode): HNode | undefined { } function updateParentSlot(child: BNode) { - let parent = child.parent; + let parent = getBNodeParent(child); while (parent) { if (parent.type === NODE_TYPE_TAG) { return; @@ -326,7 +326,7 @@ function alignChildrenByKey( return [bnodes, []]; } - const newBnodes: BNode[] = vnodes.map(() => emptyBNode()); + const newBnodes: BNode[] = vnodes.map(() => EMPTY_NODE); const oldBnodes: BNode[] = []; for (const bnode of bnodes) { diff --git a/packages/tiny-react/src/virtual-dom.ts b/packages/tiny-react/src/virtual-dom.ts index bfac8668..94e5f57d 100644 --- a/packages/tiny-react/src/virtual-dom.ts +++ b/packages/tiny-react/src/virtual-dom.ts @@ -70,17 +70,11 @@ export type BNode = BEmpty | BTag | BText | BCustom | BFragment; export type BNodeParent = BTag | BCustom | BFragment; -export type BEmpty = { - type: typeof NODE_TYPE_EMPTY; - // not needed since only we need to traverse up only from BCustom? - // but for now, make it easier by having uniform `BNode.parent` type - parent?: BNodeParent; -}; +export type BEmpty = VEmpty; export type BTag = { type: typeof NODE_TYPE_TAG; vnode: VTag; - parent?: BNodeParent; child: BNode; hnode: HTag; listeners: Map void>; @@ -89,7 +83,6 @@ export type BTag = { export type BText = { type: typeof NODE_TYPE_TEXT; vnode: VText; - parent?: BNodeParent; hnode: HText; }; @@ -111,17 +104,7 @@ export type BFragment = { slot?: HNode; }; -export function emptyBNode(): BEmpty { - return { - type: NODE_TYPE_EMPTY, - parent: undefined, - }; -} - -// TODO: identical empty vnode? -// for now, this would be critical to not break `memo(Component)` shallow equal with empty children. -// ideally, we could VNode to accomodate `null | string | number` primitives... -export const EMPTY_VNODE: VEmpty = { +export const EMPTY_NODE: VEmpty = { type: NODE_TYPE_EMPTY, }; @@ -157,3 +140,17 @@ export function getBNodeSlot(node: BNode): HNode | undefined { } return node.slot; } + +// 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) { + return node.parent; + } + return; +} + +export function setBNodeParent(node: BNode, parent: BNodeParent) { + if (node.type === NODE_TYPE_CUSTOM || node.type === NODE_TYPE_FRAGMENT) { + node.parent = parent; + } +}