Allow new children to notify fragment instances in Fabric

Author: jackpope

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

@@ -3045,19 +3045,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) {
+      appendImmutableNodeToFragmentInstances(
+        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,35 @@ function commitPlacement(finishedWork: Fiber): void {
   }
 }
 
+function appendImmutableNodeToFragmentInstances(
+  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) {
+    appendImmutableNodeToFragmentInstances(child, parentFragmentInstances);
+    let sibling = child.sibling;
+    while (sibling !== null) {
+      appendImmutableNodeToFragmentInstances(sibling, parentFragmentInstances);
+      sibling = sibling.sibling;
+    }
+  }
+}
+
 export function commitHostPlacement(finishedWork: Fiber) {
   try {
     if (__DEV__) {