Repository: facebook/react
Pull request: Move traverseFragmentInstanceChildren to internal ReactFiberTreeReflection

This is a nit but a Config should not have to know anything about the internals of Fibers. Ideally it shouldn't even access them but we have some cases where we need pointers back in like for this fragment.

The way we've typically abstracted this is using the `ReactFiberTreeReflection` helper that's in the `react-reconciler`. Such as in the event system. https://github.com/facebook/react/blob/f3c956006a90dc68210bd3e19497d10fb9b028d3/packages/react-dom-bindings/src/events/ReactDOMEventListener.js#L22-L26

We sometimes cheat but we really should clean this up such that a `Fiber` is actually an opaque type to the Configs and it can never dot into it without using a helper.

So this just moves `traverseFragmentInstanceChildren` to ReactFiberTreeReflection so that the ConfigDOM doesn't ever dot into its fields itself. It just passes the Fiber through back into the react-reconciler. I had to add a wrapper to read the `.child` to avoid that being assumed too. I also noticed that FragmentInstanceType is not actually passed through so that argument is unnecessary.

Changed files:
filename: packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
patch: @@ -34,7 +34,6 @@ import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostCo
 import hasOwnProperty from 'shared/hasOwnProperty';
 import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
 import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
-import {OffscreenComponent} from 'react-reconciler/src/ReactWorkTags';
 
 export {
   setCurrentUpdatePriority,
@@ -54,6 +53,8 @@ import {
   markNodeAsHoistable,
   isOwnedInstance,
 } from './ReactDOMComponentTree';
+import {traverseFragmentInstance} from 'react-reconciler/src/ReactFiberTreeReflection';
+
 export {detachDeletedInstance};
 import {hasRole} from './DOMAccessibilityRoles';
 import {
@@ -2221,9 +2222,8 @@ FragmentInstance.prototype.addEventListener = function (
     indexOfEventListener(listeners, type, listener, optionsOrUseCapture) === -1;
   if (isNewEventListener) {
     listeners.push({type, listener, optionsOrUseCapture});
-    traverseFragmentInstanceChildren(
-      this,
-      this._fragmentFiber.child,
+    traverseFragmentInstance(
+      this._fragmentFiber,
       addEventListenerToChild,
       type,
       listener,
@@ -2253,9 +2253,8 @@ FragmentInstance.prototype.removeEventListener = function (
     return;
   }
   if (typeof listeners !== 'undefined' && listeners.length > 0) {
-    traverseFragmentInstanceChildren(
-      this,
-      this._fragmentFiber.child,
+    traverseFragmentInstance(
+      this._fragmentFiber,
       removeEventListenerFromChild,
       type,
       listener,
@@ -2283,45 +2282,9 @@ function removeEventListenerFromChild(
 }
 // $FlowFixMe[prop-missing]
 FragmentInstance.prototype.focus = function (this: FragmentInstanceType) {
-  traverseFragmentInstanceChildren(
-    this,
-    this._fragmentFiber.child,
-    setFocusIfFocusable,
-  );
+  traverseFragmentInstance(this._fragmentFiber, setFocusIfFocusable);
 };
 
-function traverseFragmentInstanceChildren<A, B, C>(
-  fragmentInstance: FragmentInstanceType,
-  child: Fiber | null,
-  fn: (Instance, A, B, C) => boolean,
-  a: A,
-  b: B,
-  c: C,
-): void {
-  while (child !== null) {
-    if (child.tag === HostComponent) {
-      if (fn(child.stateNode, a, b, c)) {
-        return;
-      }
-    } else if (
-      child.tag === OffscreenComponent &&
-      child.memoizedState !== null
-    ) {
-      // Skip hidden subtrees
-    } else {
-      traverseFragmentInstanceChildren(
-        fragmentInstance,
-        child.child,
-        fn,
-        a,
-        b,
-        c,
-      );
-    }
-    child = child.sibling;
-  }
-}
-
 function normalizeListenerOptions(
   opts: ?EventListenerOptionsOrUseCapture,
 ): string {

filename: packages/react-reconciler/src/ReactFiberTreeReflection.js
patch: @@ -8,7 +8,7 @@
  */
 
 import type {Fiber} from './ReactInternalTypes';
-import type {Container, SuspenseInstance} from './ReactFiberConfig';
+import type {Container, SuspenseInstance, Instance} from './ReactFiberConfig';
 import type {SuspenseState} from './ReactFiberSuspenseComponent';
 
 import {
@@ -19,6 +19,7 @@ import {
   HostPortal,
   HostText,
   SuspenseComponent,
+  OffscreenComponent,
 } from './ReactWorkTags';
 import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
 
@@ -317,3 +318,37 @@ export function doesFiberContain(
   }
   return false;
 }
+
+export function traverseFragmentInstance<A, B, C>(
+  fragmentFiber: Fiber,
+  fn: (Instance, A, B, C) => boolean,
+  a: A,
+  b: B,
+  c: C,
+): void {
+  return traverseFragmentInstanceChildren(fragmentFiber.child, fn, a, b, c);
+}
+
+function traverseFragmentInstanceChildren<A, B, C>(
+  child: Fiber | null,
+  fn: (Instance, A, B, C) => boolean,
+  a: A,
+  b: B,
+  c: C,
+): void {
+  while (child !== null) {
+    if (child.tag === HostComponent) {
+      if (fn(child.stateNode, a, b, c)) {
+        return;
+      }
+    } else if (
+      child.tag === OffscreenComponent &&
+      child.memoizedState !== null
+    ) {
+      // Skip hidden subtrees
+    } else {
+      traverseFragmentInstanceChildren(child.child, fn, a, b, c);
+    }
+    child = child.sibling;
+  }
+}