add a Cloned flag to indicate a required clone in persistent mode

Author: kassens

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

@@ -273,6 +273,46 @@ describe('ReactFabric', () => {
     expect(nativeFabricUIManager.completeRoot).toBeCalled();
   });
 
+  it('should not clone nodes when layout effects are used', async () => {
+    const View = createReactNativeComponentClass('RCTView', () => ({
+      validAttributes: {foo: true},
+      uiViewClassName: 'RCTView',
+    }));
+
+    const ComponentWithEffect = () => {
+      // Same thing happens with `ref` and `useImperativeHandle`
+      React.useLayoutEffect(() => {});
+      return null;
+    };
+
+    await act(() =>
+      ReactFabric.render(
+        <View>
+          <ComponentWithEffect />
+        </View>,
+        11,
+      ),
+    );
+    expect(nativeFabricUIManager.completeRoot).toBeCalled();
+    jest.clearAllMocks();
+
+    await act(() =>
+      ReactFabric.render(
+        <View>
+          <ComponentWithEffect />
+        </View>,
+        11,
+      ),
+    );
+    expect(nativeFabricUIManager.cloneNode).not.toBeCalled();
+    expect(nativeFabricUIManager.cloneNodeWithNewChildren).not.toBeCalled();
+    expect(nativeFabricUIManager.cloneNodeWithNewProps).not.toBeCalled();
+    expect(
+      nativeFabricUIManager.cloneNodeWithNewChildrenAndProps,
+    ).not.toBeCalled();
+    expect(nativeFabricUIManager.completeRoot).not.toBeCalled();
+  });
+
   it('should call dispatchCommand for native refs', async () => {
     const View = createReactNativeComponentClass('RCTView', () => ({
       validAttributes: {foo: true},

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -97,6 +97,7 @@ import {
   Visibility,
   ShouldSuspendCommit,
   MaySuspendCommit,
+  Cloned,
 } from './ReactFiberFlags';
 import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
 import {
@@ -2522,7 +2523,7 @@ function recursivelyTraverseMutationEffects(
   }
 
   const prevDebugFiber = getCurrentDebugFiberInDEV();
-  if (parentFiber.subtreeFlags & MutationMask) {
+  if (parentFiber.subtreeFlags & (MutationMask | Cloned)) {
     let child = parentFiber.child;
     while (child !== null) {
       setCurrentDebugFiberInDEV(child);

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -83,12 +83,12 @@ import {
   Snapshot,
   ChildDeletion,
   StaticMask,
-  MutationMask,
   Passive,
   ForceClientRender,
   MaySuspendCommit,
   ScheduleRetry,
   ShouldSuspendCommit,
+  Cloned,
 } from './ReactFiberFlags';
 
 import {
@@ -182,6 +182,16 @@ function markUpdate(workInProgress: Fiber) {
   workInProgress.flags |= Update;
 }
 
+/**
+ * Tag the fiber with Cloned in persistent mode to signal that
+ * it received an update that requires a clone of the tree above.
+ */
+function markCloned(workInProgress: Fiber) {
+  if (supportsPersistence) {
+    workInProgress.flags |= Cloned;
+  }
+}
+
 /**
  * In persistent mode, return whether this update needs to clone the subtree.
  */
@@ -200,8 +210,8 @@ function doesRequireClone(current: null | Fiber, completedWork: Fiber) {
   let child = completedWork.child;
   while (child !== null) {
     if (
-      (child.flags & MutationMask) !== NoFlags ||
-      (child.subtreeFlags & MutationMask) !== NoFlags
+      (child.flags & (Cloned | Visibility | Placement)) !== NoFlags ||
+      (child.subtreeFlags & (Cloned | Visibility | Placement)) !== NoFlags
     ) {
       return true;
     }
@@ -451,6 +461,7 @@ function updateHostComponent(
 
     let newChildSet = null;
     if (requiresClone && passChildrenWhenCloningPersistedNodes) {
+      markCloned(workInProgress);
       newChildSet = createContainerChildSet();
       // If children might have changed, we have to add them all to the set.
       appendAllChildrenToContainer(
@@ -474,6 +485,8 @@ function updateHostComponent(
       // Note that this might release a previous clone.
       workInProgress.stateNode = currentInstance;
       return;
+    } else {
+      markCloned(workInProgress);
     }
 
     // Certain renderers require commit-time effects for initial mount.
@@ -485,13 +498,8 @@ function updateHostComponent(
       markUpdate(workInProgress);
     }
     workInProgress.stateNode = newInstance;
-    if (!requiresClone) {
-      // If there are no other effects in this tree, we need to flag this node as having one.
-      // Even though we're not going to use it for anything.
-      // Otherwise parents won't know that there are new children to propagate upwards.
-      markUpdate(workInProgress);
-    } else if (!passChildrenWhenCloningPersistedNodes) {
-      // If children might have changed, we have to add them all to the set.
+    if (requiresClone && !passChildrenWhenCloningPersistedNodes) {
+      // If children have changed, we have to add them all to the set.
       appendAllChildren(
         newInstance,
         workInProgress,
@@ -650,15 +658,13 @@ function updateHostText(
       // If the text content differs, we'll create a new text instance for it.
       const rootContainerInstance = getRootHostContainer();
       const currentHostContext = getHostContext();
+      markCloned(workInProgress);
       workInProgress.stateNode = createTextInstance(
         newText,
         rootContainerInstance,
         currentHostContext,
         workInProgress,
       );
-      // We'll have to mark it as having an effect, even though we won't use the effect for anything.
-      // This lets the parents know that at least one of their children has changed.
-      markUpdate(workInProgress);
     } else {
       workInProgress.stateNode = current.stateNode;
     }
@@ -1254,6 +1260,7 @@ function completeWork(
           );
           // TODO: For persistent renderers, we should pass children as part
           // of the initial instance creation
+          markCloned(workInProgress);
           appendAllChildren(instance, workInProgress, false, false);
           workInProgress.stateNode = instance;
 
@@ -1311,6 +1318,7 @@ function completeWork(
             markUpdate(workInProgress);
           }
         } else {
+          markCloned(workInProgress);
           workInProgress.stateNode = createTextInstance(
             newText,
             rootContainerInstance,

packages/react-reconciler/src/ReactFiberFlags.js

@@ -20,7 +20,7 @@ export const Hydrating = /*                    */ 0b0000000000000001000000000000
 
 // You can change the rest (and add more).
 export const Update = /*                       */ 0b0000000000000000000000000100;
-/* Skipped value:                                 0b0000000000000000000000001000; */
+export const Cloned = /*                       */ 0b0000000000000000000000001000;
 
 export const ChildDeletion = /*                */ 0b0000000000000000000000010000;
 export const ContentReset = /*                 */ 0b0000000000000000000000100000;