From df5faddcc2ad27be5700823d4f5367e5e9ae4620 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 10 Feb 2020 23:38:24 +0000 Subject: [PATCH] Refactor commitPlacement to recursively insert nodes (#17996) --- .../src/client/ReactDOMHostConfig.js | 2 +- .../src/ReactFiberCommitWork.js | 94 ++++++++++++------- 2 files changed, 62 insertions(+), 34 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 393f9f033f72a..2e90978b31c03 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -88,7 +88,7 @@ export type EventTargetChildElement = { }, ... }; -export type Container = Element | Document; +export type Container = DOMContainer; export type Instance = Element; export type TextInstance = Text; export type SuspenseInstance = Comment & {_reactRetry?: () => void, ...}; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index f4282ac4fa5b5..99a95dde732b8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -1099,44 +1099,72 @@ function commitPlacement(finishedWork: Fiber): void { const before = getHostSibling(finishedWork); // We only have the top Fiber that was inserted but we need to recurse down its // children to find all the terminal nodes. - let node: Fiber = finishedWork; - while (true) { - const isHost = node.tag === HostComponent || node.tag === HostText; - if (isHost || (enableFundamentalAPI && node.tag === FundamentalComponent)) { - const stateNode = isHost ? node.stateNode : node.stateNode.instance; - if (before) { - if (isContainer) { - insertInContainerBefore(parent, stateNode, before); - } else { - insertBefore(parent, stateNode, before); - } - } else { - if (isContainer) { - appendChildToContainer(parent, stateNode); - } else { - appendChild(parent, stateNode); - } + if (isContainer) { + insertOrAppendPlacementNodeIntoContainer(finishedWork, before, parent); + } else { + insertOrAppendPlacementNode(finishedWork, before, parent); + } +} + +function insertOrAppendPlacementNodeIntoContainer( + node: Fiber, + before: ?Instance, + parent: Container, +): void { + const {tag} = node; + const isHost = tag === HostComponent || tag === HostText; + if (isHost || (enableFundamentalAPI && tag === FundamentalComponent)) { + const stateNode = isHost ? node.stateNode : node.stateNode.instance; + if (before) { + insertInContainerBefore(parent, stateNode, before); + } else { + appendChildToContainer(parent, stateNode); + } + } else if (tag === HostPortal) { + // If the insertion itself is a portal, then we don't want to traverse + // down its children. Instead, we'll get insertions from each child in + // the portal directly. + } else { + const child = node.child; + if (child !== null) { + insertOrAppendPlacementNodeIntoContainer(child, before, parent); + let sibling = child.sibling; + while (sibling !== null) { + insertOrAppendPlacementNodeIntoContainer(sibling, before, parent); + sibling = sibling.sibling; } - } else if (node.tag === HostPortal) { - // If the insertion itself is a portal, then we don't want to traverse - // down its children. Instead, we'll get insertions from each child in - // the portal directly. - } else if (node.child !== null) { - node.child.return = node; - node = node.child; - continue; } - if (node === finishedWork) { - return; + } +} + +function insertOrAppendPlacementNode( + node: Fiber, + before: ?Instance, + parent: Instance, +): void { + const {tag} = node; + const isHost = tag === HostComponent || tag === HostText; + if (isHost || (enableFundamentalAPI && tag === FundamentalComponent)) { + const stateNode = isHost ? node.stateNode : node.stateNode.instance; + if (before) { + insertBefore(parent, stateNode, before); + } else { + appendChild(parent, stateNode); } - while (node.sibling === null) { - if (node.return === null || node.return === finishedWork) { - return; + } else if (tag === HostPortal) { + // If the insertion itself is a portal, then we don't want to traverse + // down its children. Instead, we'll get insertions from each child in + // the portal directly. + } else { + const child = node.child; + if (child !== null) { + insertOrAppendPlacementNode(child, before, parent); + let sibling = child.sibling; + while (sibling !== null) { + insertOrAppendPlacementNode(sibling, before, parent); + sibling = sibling.sibling; } - node = node.return; } - node.sibling.return = node.return; - node = node.sibling; } }