Fix asserts caused by OffscreenComponent rendering in React Native with passChildrenWhenCloningPersistedNodes (#32528)

lenaic · Nick · web-flow · commit cc680065c337 · 2025-03-07T17:03:59.000Z
## Summary  This PR fixes asserts when `passChildrenWhenCloningPersistedNodes` is enabled for React Native and OffscreenComponent child rendering unhides host components. Discussions around possible fixes for the asserts seen in React Native suggested changing the way we handle hiding/unhiding host components by updating the fiber state with the hidden host component instead of submitting a hidden clone Fabric and keeping the original as the current fiber. Implementing this fix would require holding onto the original styling of the hidden host component. The reconciler updates the styling by adding `display: none` to hide the contents. If the original host component was already hidden, the renderer would lose that information and remove the styling when showing the contents again. To reduce the changes required to make `passChildrenWhenCloningPersistedNodes` work, this PR falls back to the original cloning method when OffscreenComponents are part of the children needed to be added back. This effectively resolve the asserts triggered by the feature in RN and improves overall performance. ## How did you test this change?  This fix was tested by enabling `passChildrenWhenCloningPersistedNodes` in an app built with React Native that had a repro for triggering the asserts. The asserts do not occur anymore when using the changes in this PR. --------- Co-authored-by: Nick <lefever@meta.com>
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js
@@ -341,7 +341,12 @@ function appendAllChildrenToContainer(
   workInProgress: Fiber,
   needsVisibilityToggle: boolean,
   isHidden: boolean,
-) {
+): boolean {
+  // Host components that have their visibility toggled by an OffscreenComponent
+  // do not support passChildrenWhenCloningPersistedNodes. To inform the callee
+  // about their presence, we track and return if they were added to the
+  // child set.
+  let hasOffscreenComponentChild = false;
   if (supportsPersistence) {
     // We only have the top Fiber that was created but we need recurse down its
     // children to find all the terminal nodes.
@@ -386,20 +391,22 @@ function appendAllChildrenToContainer(
           /* needsVisibilityToggle */ _needsVisibilityToggle,
           /* isHidden */ true,
         );
+
+        hasOffscreenComponentChild = true;
       } else if (node.child !== null) {
         node.child.return = node;
         node = node.child;
         continue;
       }
       node = (node: Fiber);
       if (node === workInProgress) {
-        return;
+        return hasOffscreenComponentChild;
       }
       // $FlowFixMe[incompatible-use] found when upgrading Flow
       while (node.sibling === null) {
         // $FlowFixMe[incompatible-use] found when upgrading Flow
         if (node.return === null || node.return === workInProgress) {
-          return;
+          return hasOffscreenComponentChild;
         }
         node = node.return;
       }
@@ -408,6 +415,8 @@ function appendAllChildrenToContainer(
       node = node.sibling;
     }
   }
+
+  return hasOffscreenComponentChild;
 }
 
 function updateHostContainer(current: null | Fiber, workInProgress: Fiber) {
@@ -468,11 +477,12 @@ function updateHostComponent(
     const currentHostContext = getHostContext();
 
     let newChildSet = null;
+    let hasOffscreenComponentChild = false;
     if (requiresClone && passChildrenWhenCloningPersistedNodes) {
       markCloned(workInProgress);
       newChildSet = createContainerChildSet();
       // If children might have changed, we have to add them all to the set.
-      appendAllChildrenToContainer(
+      hasOffscreenComponentChild = appendAllChildrenToContainer(
         newChildSet,
         workInProgress,
         /* needsVisibilityToggle */ false,
@@ -486,7 +496,7 @@ function updateHostComponent(
       oldProps,
       newProps,
       !requiresClone,
-      newChildSet,
+      !hasOffscreenComponentChild ? newChildSet : undefined,
     );
     if (newInstance === currentInstance) {
       // No changes, just reuse the existing instance.
@@ -513,7 +523,10 @@ function updateHostComponent(
         // Otherwise parents won't know that there are new children to propagate upwards.
         markUpdate(workInProgress);
       }
-    } else if (!passChildrenWhenCloningPersistedNodes) {
+    } else if (
+      !passChildrenWhenCloningPersistedNodes ||
+      hasOffscreenComponentChild
+    ) {
       // If children have changed, we have to add them all to the set.
       appendAllChildren(
         newInstance,
