Refactor commitPlacement to recursively insert nodes

packages/react-dom/src/client/ReactDOM.js

@@ -113,8 +113,14 @@ setBatchingImplementation(
   batchedEventUpdates,
 );
 
+export type DOMInstance = Element & {_reactRootContainer: ?RootType, ...};
+export type DOMHTMLInstance = HTMLElement & {
+  _reactRootContainer: ?RootType,
+  ...
+};
+
 export type DOMContainer =
-  | (Element & {_reactRootContainer: ?RootType, ...})
+  | DOMInstance
   | (Document & {_reactRootContainer: ?RootType, ...});
 
 function createPortal(

packages/react-dom/src/client/ReactDOMHostConfig.js

@@ -45,7 +45,7 @@ import {
 } from '../shared/HTMLNodeType';
 import dangerousStyleValue from '../shared/dangerousStyleValue';
 
-import type {DOMContainer} from './ReactDOM';
+import type {DOMContainer, DOMInstance, DOMHTMLInstance} from './ReactDOM';
 import type {
   ReactDOMEventResponder,
   ReactDOMEventResponderInstance,
@@ -88,8 +88,8 @@ export type EventTargetChildElement = {
   },
   ...
 };
-export type Container = Element | Document;
-export type Instance = Element;
+export type Container = DOMContainer;
+export type Instance = DOMInstance;
 export type TextInstance = Text;
 export type SuspenseInstance = Comment & {_reactRetry?: () => void, ...};
 export type HydratableInstance = Instance | TextInstance | SuspenseInstance;
@@ -252,15 +252,15 @@ export function createInstance(
   } else {
     parentNamespace = ((hostContext: any): HostContextProd);
   }
-  const domElement: Instance = createElement(
+  const domElement = createElement(
     type,
     props,
     rootContainerInstance,
     parentNamespace,
   );
   precacheFiberNode(internalInstanceHandle, domElement);
   updateFiberProps(domElement, props);
-  return domElement;
+  return ((domElement: any): DOMInstance);
 }
 
 export function appendInitialChild(
@@ -398,7 +398,7 @@ export function commitUpdate(
   updateProperties(domElement, updatePayload, type, oldProps, newProps);
 }
 
-export function resetTextContent(domElement: Instance): void {
+export function resetTextContent(domElement: DOMInstance): void {
   setTextContent(domElement, '');
 }
 
@@ -411,15 +411,15 @@ export function commitTextUpdate(
 }
 
 export function appendChild(
-  parentInstance: Instance,
-  child: Instance | TextInstance,
+  parentInstance: DOMInstance,
+  child: DOMInstance | TextInstance,
 ): void {
   parentInstance.appendChild(child);
 }
 
 export function appendChildToContainer(
-  container: DOMContainer,
-  child: Instance | TextInstance,
+  container: DOMInstance,
+  child: DOMInstance | TextInstance,
 ): void {
   let parentNode;
   if (container.nodeType === COMMENT_NODE) {
@@ -448,17 +448,17 @@ export function appendChildToContainer(
 }
 
 export function insertBefore(
-  parentInstance: Instance,
-  child: Instance | TextInstance,
-  beforeChild: Instance | TextInstance | SuspenseInstance,
+  parentInstance: DOMContainer,
+  child: DOMInstance | TextInstance,
+  beforeChild: DOMInstance | TextInstance | SuspenseInstance,
 ): void {
   parentInstance.insertBefore(child, beforeChild);
 }
 
 export function insertInContainerBefore(
-  container: Container,
-  child: Instance | TextInstance,
-  beforeChild: Instance | TextInstance | SuspenseInstance,
+  container: DOMContainer,
+  child: DOMInstance | TextInstance,
+  beforeChild: DOMInstance | TextInstance | SuspenseInstance,
 ): void {
   if (container.nodeType === COMMENT_NODE) {
     (container.parentNode: any).insertBefore(child, beforeChild);
@@ -587,7 +587,7 @@ export function clearSuspenseBoundaryFromContainer(
 export function hideInstance(instance: Instance): void {
   // TODO: Does this work for all element types? What about MathML? Should we
   // pass host context to this method?
-  instance = ((instance: any): HTMLElement);
+  instance = ((instance: any): DOMHTMLInstance);
   const style = instance.style;
   if (typeof style.setProperty === 'function') {
     style.setProperty('display', 'none', 'important');
@@ -601,7 +601,7 @@ export function hideTextInstance(textInstance: TextInstance): void {
 }
 
 export function unhideInstance(instance: Instance, props: Props): void {
-  instance = ((instance: any): HTMLElement);
+  instance = ((instance: any): DOMHTMLInstance);
   const styleProp = props[STYLE];
   const display =
     styleProp !== undefined &&

packages/react-native-renderer/src/ReactNativeHostConfig.js

@@ -289,12 +289,12 @@ export function appendChild(
 }
 
 export function appendChildToContainer(
-  parentInstance: Container,
+  parentInstance: Instance | Container,
   child: Instance | TextInstance,
 ): void {
   const childTag = typeof child === 'number' ? child : child._nativeTag;
   UIManager.setChildren(
-    parentInstance, // containerTag
+    ((parentInstance: any): Container), // containerTag
     [childTag], // reactTags
   );
 }
@@ -386,7 +386,7 @@ export function insertBefore(
 }
 
 export function insertInContainerBefore(
-  parentInstance: Container,
+  parentInstance: Instance | Container,
   child: Instance | TextInstance,
   beforeChild: Instance | TextInstance,
 ): void {

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: Instance,
+  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;
   }
 }