New children notify fragment instances in Fabric (facebook#33093)

When a new child of a fragment instance is inserted, we need to notify
the instance to keep any relevant tracking up to date. For example, we
automatically observe the new child with any active
IntersectionObserver.

For mutable renderers (DOM), we reuse the existing traversal in
`commitPlacement` that does the insertions for HostComponents. Immutable
renderers (Fabric) exit this path before the traversal though, so
currently we can't notify the fragment instances.

Here I've created a separate traversal in `commitPlacement`,
specifically for immutable renders when `enableFragmentRefs` is on.

Author: jackpope

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

@@ -3073,19 +3073,19 @@ export function updateFragmentInstanceFiber(
 }
 
 export function commitNewChildToFragmentInstance(
-  childElement: Instance,
+  childInstance: Instance,
   fragmentInstance: FragmentInstanceType,
 ): void {
   const eventListeners = fragmentInstance._eventListeners;
   if (eventListeners !== null) {
     for (let i = 0; i < eventListeners.length; i++) {
       const {type, listener, optionsOrUseCapture} = eventListeners[i];
-      childElement.addEventListener(type, listener, optionsOrUseCapture);
+      childInstance.addEventListener(type, listener, optionsOrUseCapture);
     }
   }
   if (fragmentInstance._observers !== null) {
     fragmentInstance._observers.forEach(observer => {
-      observer.observe(childElement);
+      observer.observe(childInstance);
     });
   }
 }

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

@@ -695,12 +695,17 @@ export function updateFragmentInstanceFiber(
 }
 
 export function commitNewChildToFragmentInstance(
-  child: Fiber,
+  childInstance: Instance,
   fragmentInstance: FragmentInstanceType,
 ): void {
+  const publicInstance = getPublicInstance(childInstance);
   if (fragmentInstance._observers !== null) {
+    if (publicInstance == null) {
+      throw new Error('Expected to find a host node. This is a bug in React.');
+    }
     fragmentInstance._observers.forEach(observer => {
-      observeChild(child, observer);
+      // $FlowFixMe[incompatible-call] Element types are behind a flag in RN
+      observer.observe(publicInstance);
     });
   }
 }

packages/react-native-renderer/src/__tests__/ReactFabricFragmentRefs-test.internal.js

@@ -80,4 +80,46 @@ describe('Fabric FragmentRefs', () => {
 
     expect(fragmentRef && fragmentRef._fragmentFiber).toBeTruthy();
   });
+
+  describe('observers', () => {
+    // @gate enableFragmentRefs
+    it('observes children, newly added children', async () => {
+      let logs = [];
+      const observer = {
+        observe: entry => {
+          // Here we reference internals because we don't need to mock the native observer
+          // We only need to test that each child node is observed on insertion
+          logs.push(entry.__internalInstanceHandle.pendingProps.nativeID);
+        },
+      };
+      function Test({showB}) {
+        const fragmentRef = React.useRef(null);
+        React.useEffect(() => {
+          fragmentRef.current.observeUsing(observer);
+          const lastRefValue = fragmentRef.current;
+          return () => {
+            lastRefValue.unobserveUsing(observer);
+          };
+        }, []);
+        return (
+          <View nativeID="parent">
+            <React.Fragment ref={fragmentRef}>
+              <View nativeID="A" />
+              {showB && <View nativeID="B" />}
+            </React.Fragment>
+          </View>
+        );
+      }
+
+      await act(() => {
+        ReactFabric.render(<Test showB={false} />, 11, null, true);
+      });
+      expect(logs).toEqual(['A']);
+      logs = [];
+      await act(() => {
+        ReactFabric.render(<Test showB={true} />, 11, null, true);
+      });
+      expect(logs).toEqual(['B']);
+    });
+  });
 });

packages/react-reconciler/src/ReactFiberCommitHostEffects.js

@@ -255,8 +255,16 @@ export function commitShowHideHostTextInstance(node: Fiber, isHidden: boolean) {
 
 export function commitNewChildToFragmentInstances(
   fiber: Fiber,
-  parentFragmentInstances: Array<FragmentInstanceType>,
+  parentFragmentInstances: null | Array<FragmentInstanceType>,
 ): void {
+  if (
+    fiber.tag !== HostComponent ||
+    // Only run fragment insertion effects for initial insertions
+    fiber.alternate !== null ||
+    parentFragmentInstances === null
+  ) {
+    return;
+  }
   for (let i = 0; i < parentFragmentInstances.length; i++) {
     const fragmentInstance = parentFragmentInstances[i];
     commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance);
@@ -384,14 +392,7 @@ function insertOrAppendPlacementNodeIntoContainer(
     } else {
       appendChildToContainer(parent, stateNode);
     }
-    // TODO: Enable HostText for RN
-    if (
-      enableFragmentRefs &&
-      tag === HostComponent &&
-      // Only run fragment insertion effects for initial insertions
-      node.alternate === null &&
-      parentFragmentInstances !== null
-    ) {
+    if (enableFragmentRefs) {
       commitNewChildToFragmentInstances(node, parentFragmentInstances);
     }
     trackHostMutation();
@@ -449,14 +450,7 @@ function insertOrAppendPlacementNode(
     } else {
       appendChild(parent, stateNode);
     }
-    // TODO: Enable HostText for RN
-    if (
-      enableFragmentRefs &&
-      tag === HostComponent &&
-      // Only run fragment insertion effects for initial insertions
-      node.alternate === null &&
-      parentFragmentInstances !== null
-    ) {
+    if (enableFragmentRefs) {
       commitNewChildToFragmentInstances(node, parentFragmentInstances);
     }
     trackHostMutation();
@@ -494,10 +488,6 @@ function insertOrAppendPlacementNode(
 }
 
 function commitPlacement(finishedWork: Fiber): void {
-  if (!supportsMutation) {
-    return;
-  }
-
   // Recursively insert all host nodes into the parent.
   let hostParentFiber;
   let parentFragmentInstances = null;
@@ -517,6 +507,17 @@ function commitPlacement(finishedWork: Fiber): void {
     }
     parentFiber = parentFiber.return;
   }
+
+  if (!supportsMutation) {
+    if (enableFragmentRefs) {
+      commitImmutablePlacementNodeToFragmentInstances(
+        finishedWork,
+        parentFragmentInstances,
+      );
+    }
+    return;
+  }
+
   if (hostParentFiber == null) {
     throw new Error(
       'Expected to find a host parent. This error is likely caused by a bug ' +
@@ -581,6 +582,41 @@ function commitPlacement(finishedWork: Fiber): void {
   }
 }
 
+function commitImmutablePlacementNodeToFragmentInstances(
+  finishedWork: Fiber,
+  parentFragmentInstances: null | Array<FragmentInstanceType>,
+): void {
+  if (!enableFragmentRefs) {
+    return;
+  }
+  const isHost = finishedWork.tag === HostComponent;
+  if (isHost) {
+    commitNewChildToFragmentInstances(finishedWork, parentFragmentInstances);
+    return;
+  } else if (finishedWork.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.
+    return;
+  }
+
+  const child = finishedWork.child;
+  if (child !== null) {
+    commitImmutablePlacementNodeToFragmentInstances(
+      child,
+      parentFragmentInstances,
+    );
+    let sibling = child.sibling;
+    while (sibling !== null) {
+      commitImmutablePlacementNodeToFragmentInstances(
+        sibling,
+        parentFragmentInstances,
+      );
+      sibling = sibling.sibling;
+    }
+  }
+}
+
 export function commitHostPlacement(finishedWork: Fiber) {
   try {
     if (__DEV__) {