Address feedback

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

@@ -113,14 +113,8 @@ setBatchingImplementation(
   batchedEventUpdates,
 );
 
-export type DOMInstance = Element & {_reactRootContainer: ?RootType, ...};
-export type DOMHTMLInstance = HTMLElement & {
-  _reactRootContainer: ?RootType,
-  ...
-};
-
 export type DOMContainer =
-  | DOMInstance
+  | (Element & {_reactRootContainer: ?RootType, ...})
   | (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, DOMInstance, DOMHTMLInstance} from './ReactDOM';
+import type {DOMContainer} from './ReactDOM';
 import type {
   ReactDOMEventResponder,
   ReactDOMEventResponderInstance,
@@ -89,7 +89,7 @@ export type EventTargetChildElement = {
   ...
 };
 export type Container = DOMContainer;
-export type Instance = DOMInstance;
+export type Instance = Element;
 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 = createElement(
+  const domElement: Instance = createElement(
     type,
     props,
     rootContainerInstance,
     parentNamespace,
   );
   precacheFiberNode(internalInstanceHandle, domElement);
   updateFiberProps(domElement, props);
-  return ((domElement: any): DOMInstance);
+  return domElement;
 }
 
 export function appendInitialChild(
@@ -398,7 +398,7 @@ export function commitUpdate(
   updateProperties(domElement, updatePayload, type, oldProps, newProps);
 }
 
-export function resetTextContent(domElement: DOMInstance): void {
+export function resetTextContent(domElement: Instance): void {
   setTextContent(domElement, '');
 }
 
@@ -411,15 +411,15 @@ export function commitTextUpdate(
 }
 
 export function appendChild(
-  parentInstance: DOMInstance,
-  child: DOMInstance | TextInstance,
+  parentInstance: Instance,
+  child: Instance | TextInstance,
 ): void {
   parentInstance.appendChild(child);
 }
 
 export function appendChildToContainer(
-  container: DOMInstance,
-  child: DOMInstance | TextInstance,
+  container: DOMContainer,
+  child: Instance | TextInstance,
 ): void {
   let parentNode;
   if (container.nodeType === COMMENT_NODE) {
@@ -448,17 +448,17 @@ export function appendChildToContainer(
 }
 
 export function insertBefore(
-  parentInstance: DOMContainer,
-  child: DOMInstance | TextInstance,
-  beforeChild: DOMInstance | TextInstance | SuspenseInstance,
+  parentInstance: Instance,
+  child: Instance | TextInstance,
+  beforeChild: Instance | TextInstance | SuspenseInstance,
 ): void {
   parentInstance.insertBefore(child, beforeChild);
 }
 
 export function insertInContainerBefore(
-  container: DOMContainer,
-  child: DOMInstance | TextInstance,
-  beforeChild: DOMInstance | TextInstance | SuspenseInstance,
+  container: Container,
+  child: Instance | TextInstance,
+  beforeChild: Instance | 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): DOMHTMLInstance);
+  instance = ((instance: any): HTMLElement);
   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): DOMHTMLInstance);
+  instance = ((instance: any): HTMLElement);
   const styleProp = props[STYLE];
   const display =
     styleProp !== undefined &&

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -1099,42 +1099,69 @@ 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.
-  insertOrAppendPlacementNode(finishedWork, before, parent, isContainer);
+  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;
+      }
+    }
+  }
 }
 
 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 {tag} = node;
+  const isHost = tag === HostComponent || tag === HostText;
+  if (isHost || (enableFundamentalAPI && tag === FundamentalComponent)) {
     const stateNode = isHost ? node.stateNode : node.stateNode.instance;
     if (before) {
-      if (isContainer) {
-        insertInContainerBefore(parent, stateNode, before);
-      } else {
-        insertBefore(parent, stateNode, before);
-      }
+      insertBefore(parent, stateNode, before);
     } else {
-      if (isContainer) {
-        appendChildToContainer(parent, stateNode);
-      } else {
-        appendChild(parent, stateNode);
-      }
+      appendChild(parent, stateNode);
     }
-  } else if (node.tag === HostPortal) {
+  } 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, isContainer);
+      insertOrAppendPlacementNode(child, before, parent);
       let sibling = child.sibling;
       while (sibling !== null) {
-        insertOrAppendPlacementNode(sibling, before, parent, isContainer);
+        insertOrAppendPlacementNode(sibling, before, parent);
         sibling = sibling.sibling;
       }
     }