[Fabric] Pass children when cloning

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

@@ -47,7 +47,10 @@ const {
   unstable_getCurrentEventPriority: fabricGetCurrentEventPriority,
 } = nativeFabricUIManager;
 
-import {useMicrotasksForSchedulingInFabric} from 'shared/ReactFeatureFlags';
+import {
+  useMicrotasksForSchedulingInFabric,
+  passChildrenWhenCloningPersistedNodes,
+} from 'shared/ReactFeatureFlags';
 
 const {get: getViewConfigForType} = ReactNativeViewConfigRegistry;
 
@@ -87,7 +90,7 @@ export type TextInstance = {
 export type HydratableInstance = Instance | TextInstance;
 export type PublicInstance = ReactNativePublicInstance;
 export type Container = number;
-export type ChildSet = Object;
+export type ChildSet = Object | Array<Node>;
 export type HostContext = $ReadOnly<{
   isInAParentText: boolean,
 }>;
@@ -346,9 +349,8 @@ export function cloneInstance(
   type: string,
   oldProps: Props,
   newProps: Props,
-  internalInstanceHandle: InternalInstanceHandle,
   keepChildren: boolean,
-  recyclableInstance: null | Instance,
+  newChildSet: ?ChildSet,
 ): Instance {
   const viewConfig = instance.canonical.viewConfig;
   const updatePayload = diff(oldProps, newProps, viewConfig.validAttributes);
@@ -367,12 +369,26 @@ export function cloneInstance(
       return instance;
     }
   } else {
-    if (updatePayload !== null) {
-      clone = cloneNodeWithNewChildrenAndProps(node, updatePayload);
+    // If passChildrenWhenCloningPersistedNodes is enabled, children will be non-null
+    if (newChildSet != null) {
+      if (updatePayload !== null) {
+        clone = cloneNodeWithNewChildrenAndProps(
+          node,
+          newChildSet,
+          updatePayload,
+        );
+      } else {
+        clone = cloneNodeWithNewChildren(node, newChildSet);
+      }
     } else {
-      clone = cloneNodeWithNewChildren(node);
+      if (updatePayload !== null) {
+        clone = cloneNodeWithNewChildrenAndProps(node, updatePayload);
+      } else {
+        clone = cloneNodeWithNewChildren(node);
+      }
     }
   }
+
   return {
     node: clone,
     canonical: instance.canonical,
@@ -383,7 +399,6 @@ export function cloneHiddenInstance(
   instance: Instance,
   type: string,
   props: Props,
-  internalInstanceHandle: InternalInstanceHandle,
 ): Instance {
   const viewConfig = instance.canonical.viewConfig;
   const node = instance.node;
@@ -400,20 +415,27 @@ export function cloneHiddenInstance(
 export function cloneHiddenTextInstance(
   instance: Instance,
   text: string,
-  internalInstanceHandle: InternalInstanceHandle,
 ): TextInstance {
   throw new Error('Not yet implemented.');
 }
 
-export function createContainerChildSet(container: Container): ChildSet {
-  return createChildNodeSet(container);
+export function createContainerChildSet(): ChildSet {
+  if (passChildrenWhenCloningPersistedNodes) {
+    return [];
+  } else {
+    return createChildNodeSet();
+  }
 }
 
 export function appendChildToContainerChildSet(
   childSet: ChildSet,
   child: Instance | TextInstance,
 ): void {
-  appendChildNodeToSet(childSet, child.node);
+  if (passChildrenWhenCloningPersistedNodes) {
+    childSet.push(child.node);
+  } else {
+    appendChildNodeToSet(childSet, child.node);
+  }
 }
 
 export function finalizeContainerChildren(
@@ -426,7 +448,9 @@ export function finalizeContainerChildren(
 export function replaceContainerChildren(
   container: Container,
   newChildren: ChildSet,
-): void {}
+): void {
+  // Noop - children will be replaced in finalizeContainerChildren
+}
 
 export function getInstanceFromNode(node: any): empty {
   throw new Error('Not yet implemented.');

packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/InitializeNativeFabricUIManager.js

@@ -70,12 +70,15 @@ const RCTFabricUIManager = {
       children: node.children,
     };
   }),
-  cloneNodeWithNewChildren: jest.fn(function cloneNodeWithNewChildren(node) {
+  cloneNodeWithNewChildren: jest.fn(function cloneNodeWithNewChildren(
+    node,
+    children,
+  ) {
     return {
       reactTag: node.reactTag,
       viewName: node.viewName,
       props: node.props,
-      children: [],
+      children: children ?? [],
     };
   }),
   cloneNodeWithNewProps: jest.fn(function cloneNodeWithNewProps(
@@ -91,11 +94,17 @@ const RCTFabricUIManager = {
   }),
   cloneNodeWithNewChildrenAndProps: jest.fn(
     function cloneNodeWithNewChildrenAndProps(node, newPropsDiff) {
+      let children = [];
+      if (arguments.length === 3) {
+        children = newPropsDiff;
+        newPropsDiff = arguments[2];
+      }
+
       return {
         reactTag: node.reactTag,
         viewName: node.viewName,
         props: {...node.props, ...newPropsDiff},
-        children: [],
+        children,
       };
     },
   ),

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

@@ -210,8 +210,13 @@ describe('ReactFabric', () => {
         11,
       );
     });
+    const argIndex = gate(flags => flags.passChildrenWhenCloningPersistedNodes)
+      ? 2
+      : 1;
     expect(
-      nativeFabricUIManager.cloneNodeWithNewChildrenAndProps.mock.calls[0][1],
+      nativeFabricUIManager.cloneNodeWithNewChildrenAndProps.mock.calls[0][
+        argIndex
+      ],
     ).toEqual({
       foo: 'b',
     });
@@ -220,6 +225,54 @@ describe('ReactFabric', () => {
     ).toMatchSnapshot();
   });
 
+  it('should not clone nodes without children when updating props', async () => {
+    const View = createReactNativeComponentClass('RCTView', () => ({
+      validAttributes: {foo: true},
+      uiViewClassName: 'RCTView',
+    }));
+
+    const Component = ({foo}) => (
+      <View>
+        <View foo={foo} />
+      </View>
+    );
+
+    await act(() => ReactFabric.render(<Component foo={true} />, 11));
+    expect(nativeFabricUIManager.completeRoot).toBeCalled();
+    jest.clearAllMocks();
+
+    await act(() => ReactFabric.render(<Component foo={false} />, 11));
+    expect(nativeFabricUIManager.cloneNode).not.toBeCalled();
+    expect(nativeFabricUIManager.cloneNodeWithNewProps).toHaveBeenCalledTimes(
+      1,
+    );
+    expect(nativeFabricUIManager.cloneNodeWithNewProps).toHaveBeenCalledWith(
+      expect.anything(),
+      {foo: false},
+    );
+
+    expect(
+      nativeFabricUIManager.cloneNodeWithNewChildren,
+    ).toHaveBeenCalledTimes(1);
+    if (gate(flags => flags.passChildrenWhenCloningPersistedNodes)) {
+      expect(
+        nativeFabricUIManager.cloneNodeWithNewChildren,
+      ).toHaveBeenCalledWith(expect.anything(), [
+        expect.objectContaining({props: {foo: false}}),
+      ]);
+      expect(nativeFabricUIManager.appendChild).not.toBeCalled();
+    } else {
+      expect(
+        nativeFabricUIManager.cloneNodeWithNewChildren,
+      ).toHaveBeenCalledWith(expect.anything());
+      expect(nativeFabricUIManager.appendChild).toHaveBeenCalledTimes(1);
+    }
+    expect(
+      nativeFabricUIManager.cloneNodeWithNewChildrenAndProps,
+    ).not.toBeCalled();
+    expect(nativeFabricUIManager.completeRoot).toBeCalled();
+  });
+
   it('should call dispatchCommand for native refs', async () => {
     const View = createReactNativeComponentClass('RCTView', () => ({
       validAttributes: {foo: true},

packages/react-noop-renderer/src/createReactNoop.js

@@ -223,9 +223,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
     type: string,
     oldProps: Props,
     newProps: Props,
-    internalInstanceHandle: Object,
     keepChildren: boolean,
-    recyclableInstance: null | Instance,
+    children: ?$ReadOnlyArray<Instance>,
   ): Instance {
     if (__DEV__) {
       checkPropStringCoercion(newProps.children, 'children');
@@ -234,7 +233,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       id: instance.id,
       type: type,
       parent: instance.parent,
-      children: keepChildren ? instance.children : [],
+      children: keepChildren ? instance.children : children ?? [],
       text: shouldSetTextContent(type, newProps)
         ? computeText((newProps.children: any) + '', instance.context)
         : null,
@@ -704,9 +703,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
         cloneInstance,
         clearContainer,
 
-        createContainerChildSet(
-          container: Container,
-        ): Array<Instance | TextInstance> {
+        createContainerChildSet(): Array<Instance | TextInstance> {
           return [];
         },
 
@@ -742,25 +739,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
           instance: Instance,
           type: string,
           props: Props,
-          internalInstanceHandle: Object,
         ): Instance {
-          const clone = cloneInstance(
-            instance,
-            type,
-            props,
-            props,
-            internalInstanceHandle,
-            true,
-            null,
-          );
+          const clone = cloneInstance(instance, type, props, props, true, null);
           clone.hidden = true;
           return clone;
         },
 
         cloneHiddenTextInstance(
           instance: TextInstance,
           text: string,
-          internalInstanceHandle: Object,
         ): TextInstance {
           const clone = {
             text: instance.text,

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -1724,7 +1724,7 @@ function emptyPortalContainer(current: Fiber) {
     ...
   } = current.stateNode;
   const {containerInfo} = portal;
-  const emptyChildSet = createContainerChildSet(containerInfo);
+  const emptyChildSet = createContainerChildSet();
   replaceContainerChildren(containerInfo, emptyChildSet);
 }