[Fabric] Pass children when cloning nodes

Author: javache

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

@@ -349,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);
@@ -370,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,
@@ -386,7 +399,6 @@ export function cloneHiddenInstance(
   instance: Instance,
   type: string,
   props: Props,
-  internalInstanceHandle: InternalInstanceHandle,
 ): Instance {
   const viewConfig = instance.canonical.viewConfig;
   const node = instance.node;
@@ -403,7 +415,6 @@ export function cloneHiddenInstance(
 export function cloneHiddenTextInstance(
   instance: Instance,
   text: string,
-  internalInstanceHandle: InternalInstanceHandle,
 ): TextInstance {
   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

@@ -220,6 +220,60 @@ describe('ReactFabric', () => {
     ).toMatchSnapshot();
   });
 
+  it.each([true, false])(
+    'should not clone nodes without children when updating props (feature flag = %p)',
+    async passChildren => {
+      const ReactFeatureFlags = require('shared/ReactFeatureFlags');
+      ReactFeatureFlags.passChildrenWhenCloningPersistedNodes = passChildren;
+
+      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 (passChildren) {
+        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,
@@ -740,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/ReactFiberCompleteWork.js

@@ -38,6 +38,7 @@ import {
   enableCache,
   enableTransitionTracing,
   enableFloat,
+  passChildrenWhenCloningPersistedNodes,
 } from 'shared/ReactFeatureFlags';
 
 import {now} from './Scheduler';
@@ -253,27 +254,26 @@ function appendAllChildren(
       node.sibling.return = node.return;
       node = node.sibling;
     }
-  } else if (supportsPersistence) {
+  } else if (supportsPersistence && !passChildrenWhenCloningPersistedNodes) {
     // We only have the top Fiber that was created but we need recurse down its
     // children to find all the terminal nodes.
     let node = workInProgress.child;
     while (node !== null) {
-      // eslint-disable-next-line no-labels
-      branches: if (node.tag === HostComponent) {
+      if (node.tag === HostComponent) {
         let instance = node.stateNode;
         if (needsVisibilityToggle && isHidden) {
           // This child is inside a timed out tree. Hide it.
           const props = node.memoizedProps;
           const type = node.type;
-          instance = cloneHiddenInstance(instance, type, props, node);
+          instance = cloneHiddenInstance(instance, type, props);
         }
         appendInitialChild(parent, instance);
       } else if (node.tag === HostText) {
         let instance = node.stateNode;
         if (needsVisibilityToggle && isHidden) {
           // This child is inside a timed out tree. Hide it.
           const text = node.memoizedProps;
-          instance = cloneHiddenTextInstance(instance, text, node);
+          instance = cloneHiddenTextInstance(instance, text);
         }
         appendInitialChild(parent, instance);
       } else if (node.tag === HostPortal) {
@@ -290,7 +290,12 @@ function appendAllChildren(
         if (child !== null) {
           child.return = node;
         }
-        appendAllChildren(parent, node, true, true);
+        appendAllChildren(
+          parent,
+          node,
+          /* needsVisibilityToggle */ true,
+          /* isHidden */ true,
+        );
       } else if (node.child !== null) {
         node.child.return = node;
         node = node.child;
@@ -328,31 +333,28 @@ function appendAllChildrenToContainer(
     let node = workInProgress.child;
     while (node !== null) {
       // eslint-disable-next-line no-labels
-      branches: if (node.tag === HostComponent) {
+      if (node.tag === HostComponent) {
         let instance = node.stateNode;
         if (needsVisibilityToggle && isHidden) {
           // This child is inside a timed out tree. Hide it.
           const props = node.memoizedProps;
           const type = node.type;
-          instance = cloneHiddenInstance(instance, type, props, node);
+          instance = cloneHiddenInstance(instance, type, props);
         }
         appendChildToContainerChildSet(containerChildSet, instance);
       } else if (node.tag === HostText) {
         let instance = node.stateNode;
         if (needsVisibilityToggle && isHidden) {
           // This child is inside a timed out tree. Hide it.
           const text = node.memoizedProps;
-          instance = cloneHiddenTextInstance(instance, text, node);
+          instance = cloneHiddenTextInstance(instance, text);
         }
         appendChildToContainerChildSet(containerChildSet, instance);
       } else if (node.tag === HostPortal) {
         // If we have a portal child, then we don't want to traverse
         // down its children. Instead, we'll get insertions from each child in
         // the portal directly.
-      } else if (
-        node.tag === OffscreenComponent &&
-        node.memoizedState !== null
-      ) {
+      } else if (node.tag === OffscreenComponent && node.memoizedState !== null) {
         // The children in this boundary are hidden. Toggle their visibility
         // before appending.
         const child = node.child;
@@ -364,8 +366,8 @@ function appendAllChildrenToContainer(
         appendAllChildrenToContainer(
           containerChildSet,
           node,
-          _needsVisibilityToggle,
-          true,
+          /* needsVisibilityToggle */ _needsVisibilityToggle,
+          /* isHidden */ true,
         );
       } else if (node.child !== null) {
         node.child.return = node;
@@ -449,16 +451,27 @@ function updateHostComponent(
       workInProgress.stateNode = currentInstance;
       return;
     }
-    const recyclableInstance: Instance = workInProgress.stateNode;
     const currentHostContext = getHostContext();
+
+    let newChildSet = null;
+    if (!childrenUnchanged && passChildrenWhenCloningPersistedNodes) {
+      newChildSet = createContainerChildSet();
+      // If children might have changed, we have to add them all to the set.
+      appendAllChildrenToContainer(
+        newChildSet,
+        workInProgress,
+        /* needsVisibilityToggle */ false,
+        /* isHidden */ false,
+      );
+    }
+
     const newInstance = cloneInstance(
       currentInstance,
       type,
       oldProps,
       newProps,
-      workInProgress,
       childrenUnchanged,
-      recyclableInstance,
+      newChildSet,
     );
     if (newInstance === currentInstance) {
       // No changes, just reuse the existing instance.

scripts/flow/react-native-host-hooks.js

@@ -176,9 +176,16 @@ declare var nativeFabricUIManager: {
     eventTarget: Object,
   ) => Object,
   cloneNode: (node: Object) => Object,
-  cloneNodeWithNewChildren: (node: Object) => Object,
+  cloneNodeWithNewChildren: (
+    node: Object,
+    children?: $ReadOnlyArray<Object>,
+  ) => Object,
   cloneNodeWithNewProps: (node: Object, newProps: ?Object) => Object,
-  cloneNodeWithNewChildrenAndProps: (node: Object, newProps: ?Object) => Object,
+  cloneNodeWithNewChildrenAndProps: (
+    node: Object,
+    newPropsOrChildren: ?Object | $ReadOnlyArray<Object>,
+    newProps?: ?Object,
+  ) => Object,
   appendChild: (node: Object, childNode: Object) => void,
 
   createChildSet: () => Object,