Refactor commitPlacement to recursively insert nodes

facebook · Feb 7, 2020 · 5dd6086 · 5dd6086
1 parent 256d78d
commit 5dd6086
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 37 deletions.
diff --git 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, ...};
@@ -411,7 +411,7 @@ export function commitTextUpdate(
 }
 
 export function appendChild(
-  parentInstance: Instance,
+  parentInstance: DOMContainer,
   child: Instance | TextInstance,
 ): void {
   parentInstance.appendChild(child);
@@ -448,15 +448,15 @@ export function appendChildToContainer(
 }
 
 export function insertBefore(
-  parentInstance: Instance,
+  parentInstance: DOMContainer,
   child: Instance | TextInstance,
   beforeChild: Instance | TextInstance | SuspenseInstance,
 ): void {
   parentInstance.insertBefore(child, beforeChild);
 }
 
 export function insertInContainerBefore(
-  container: Container,
+  container: DOMContainer,
   child: Instance | TextInstance,
   beforeChild: Instance | TextInstance | SuspenseInstance,
 ): void {

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -1099,44 +1099,45 @@ 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);
-        }
+  insertOrAppendPlacementNode(finishedWork, before, parent, isContainer);
+}
+
+function insertOrAppendPlacementNode(
+  node: Fiber,
+  before: ?Instance,
+  parent: Container,
+  isContainer: boolean,
+): void {
+  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 {
-        if (isContainer) {
-          appendChildToContainer(parent, stateNode);
-        } else {
-          appendChild(parent, stateNode);
-        }
+        insertBefore(parent, stateNode, before);
+      }
+    } else {
+      if (isContainer) {
+        appendChildToContainer(parent, stateNode);
+      } else {
+        appendChild(parent, stateNode);
       }
-    } 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;
     }
-    while (node.sibling === null) {
-      if (node.return === null || node.return === finishedWork) {
-        return;
+  } 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 {
+    const child = node.child;
+    if (child !== null) {
+      insertOrAppendPlacementNode(child, before, parent, isContainer);
+      let sibling = child.sibling;
+      while (sibling !== null) {
+        insertOrAppendPlacementNode(sibling, before, parent, isContainer);
+        sibling = sibling.sibling;
       }
-      node = node.return;
     }
-    node.sibling.return = node.return;
-    node = node.sibling;
   }
 }