diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 2aac7caeda335..285c1a0f107a0 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/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; 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.'); diff --git a/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/InitializeNativeFabricUIManager.js b/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/InitializeNativeFabricUIManager.js index 33dccc1e8fb56..9a4cfc59e3d03 100644 --- a/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/InitializeNativeFabricUIManager.js +++ b/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, }; }, ), diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 4e97cead6f349..644d72974036d 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/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}) => ( + + + + ); + + await act(() => ReactFabric.render(, 11)); + expect(nativeFabricUIManager.completeRoot).toBeCalled(); + jest.clearAllMocks(); + + await act(() => ReactFabric.render(, 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}, diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 78249d500090f..6fc58593b7076 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/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 { 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 { + createContainerChildSet(): Array { return []; }, @@ -742,17 +739,8 @@ 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; }, @@ -760,7 +748,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { cloneHiddenTextInstance( instance: TextInstance, text: string, - internalInstanceHandle: Object, ): TextInstance { const clone = { text: instance.text, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 96ba1d5963274..73889d8b8180c 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/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); } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index ec46a5932f5ca..ecd129bbb391e 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -38,6 +38,7 @@ import { enableCache, enableTransitionTracing, enableFloat, + passChildrenWhenCloningPersistedNodes, } from 'shared/ReactFeatureFlags'; import {now} from './Scheduler'; @@ -258,14 +259,13 @@ function appendAllChildren( // 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) { @@ -273,7 +273,7 @@ function appendAllChildren( 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,13 +333,13 @@ 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) { @@ -342,7 +347,7 @@ function appendAllChildrenToContainer( 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) { @@ -364,8 +369,8 @@ function appendAllChildrenToContainer( appendAllChildrenToContainer( containerChildSet, node, - _needsVisibilityToggle, - true, + /* needsVisibilityToggle */ _needsVisibilityToggle, + /* isHidden */ true, ); } else if (node.child !== null) { node.child.return = node; @@ -390,6 +395,7 @@ function appendAllChildrenToContainer( } } } + function updateHostContainer(current: null | Fiber, workInProgress: Fiber) { if (supportsPersistence) { const portalOrRoot: { @@ -402,9 +408,14 @@ function updateHostContainer(current: null | Fiber, workInProgress: Fiber) { // No changes, just reuse the existing instance. } else { const container = portalOrRoot.containerInfo; - const newChildSet = createContainerChildSet(container); + const newChildSet = createContainerChildSet(); // If children might have changed, we have to add them all to the set. - appendAllChildrenToContainer(newChildSet, workInProgress, false, false); + appendAllChildrenToContainer( + newChildSet, + workInProgress, + /* needsVisibilityToggle */ false, + /* isHidden */ false, + ); portalOrRoot.pendingChildren = newChildSet; // Schedule an update on the container to swap out the container. markUpdate(workInProgress); @@ -412,6 +423,7 @@ function updateHostContainer(current: null | Fiber, workInProgress: Fiber) { } } } + function updateHostComponent( current: Fiber, workInProgress: Fiber, @@ -442,16 +454,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. @@ -460,6 +483,9 @@ function updateHostComponent( return; } + // Certain renderers require commit-time effects for initial mount. + // (eg DOM renderer supports auto-focus for certain elements). + // Make sure such renderers get scheduled for later work. if ( finalizeInitialChildren(newInstance, type, newProps, currentHostContext) ) { @@ -471,9 +497,14 @@ function updateHostComponent( // 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 { + } else if (!passChildrenWhenCloningPersistedNodes) { // If children might have changed, we have to add them all to the set. - appendAllChildren(newInstance, workInProgress, false, false); + appendAllChildren( + newInstance, + workInProgress, + /* needsVisibilityToggle */ false, + /* isHidden */ false, + ); } } } @@ -1259,6 +1290,8 @@ function completeWork( currentHostContext, workInProgress, ); + // TODO: For persistent renderers, we should pass children as part + // of the initial instance creation appendAllChildren(instance, workInProgress, false, false); workInProgress.stateNode = instance; diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 5178f2c90ff7b..5ca0e6e291fcd 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -124,6 +124,8 @@ export const alwaysThrottleRetries = true; export const useMicrotasksForSchedulingInFabric = false; +export const passChildrenWhenCloningPersistedNodes = false; + // ----------------------------------------------------------------------------- // Chopping Block // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index 7ef1ef3013f17..f2b6a5ae36d93 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -24,6 +24,7 @@ export const enableUseRefAccessWarning = __VARIANT__; export const enableDeferRootSchedulingToMicrotask = __VARIANT__; export const alwaysThrottleRetries = __VARIANT__; export const useMicrotasksForSchedulingInFabric = __VARIANT__; +export const passChildrenWhenCloningPersistedNodes = __VARIANT__; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): DynamicFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 9b7b7f2f383f2..99d7392719256 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -22,6 +22,7 @@ export const { enableDeferRootSchedulingToMicrotask, alwaysThrottleRetries, useMicrotasksForSchedulingInFabric, + passChildrenWhenCloningPersistedNodes, } = dynamicFlags; // The rest of the flags are static for better dead code elimination. diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index ebab18441223f..ee0b2d91d387c 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -79,6 +79,7 @@ export const enableAsyncActions = false; export const alwaysThrottleRetries = true; export const useMicrotasksForSchedulingInFabric = false; +export const passChildrenWhenCloningPersistedNodes = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 392c8cbb538b9..5d541fa3948ef 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -79,6 +79,7 @@ export const enableAsyncActions = true; export const alwaysThrottleRetries = true; export const useMicrotasksForSchedulingInFabric = false; +export const passChildrenWhenCloningPersistedNodes = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index b690f237b130b..23a44293eeb68 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -76,6 +76,7 @@ export const enableAsyncActions = true; export const alwaysThrottleRetries = true; export const useMicrotasksForSchedulingInFabric = false; +export const passChildrenWhenCloningPersistedNodes = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e426ade2df12a..121442de9660d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -79,6 +79,7 @@ export const enableAsyncActions = true; export const alwaysThrottleRetries = true; export const useMicrotasksForSchedulingInFabric = false; +export const passChildrenWhenCloningPersistedNodes = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 16e6114e5153e..a004364fa4ab4 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -110,6 +110,7 @@ export const enableFizzExternalRuntime = true; export const forceConcurrentByDefaultForTesting = false; export const useMicrotasksForSchedulingInFabric = false; +export const passChildrenWhenCloningPersistedNodes = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/scripts/flow/react-native-host-hooks.js b/scripts/flow/react-native-host-hooks.js index 6616342a618ef..7fd2e18f1c516 100644 --- a/scripts/flow/react-native-host-hooks.js +++ b/scripts/flow/react-native-host-hooks.js @@ -176,12 +176,19 @@ declare var nativeFabricUIManager: { eventTarget: Object, ) => Object, cloneNode: (node: Object) => Object, - cloneNodeWithNewChildren: (node: Object) => Object, + cloneNodeWithNewChildren: ( + node: Object, + children?: $ReadOnlyArray, + ) => Object, cloneNodeWithNewProps: (node: Object, newProps: ?Object) => Object, - cloneNodeWithNewChildrenAndProps: (node: Object, newProps: ?Object) => Object, + cloneNodeWithNewChildrenAndProps: ( + node: Object, + newPropsOrChildren: ?Object | $ReadOnlyArray, + newProps?: ?Object, + ) => Object, appendChild: (node: Object, childNode: Object) => void, - createChildSet: (rootTag: number) => Object, + createChildSet: () => Object, appendChildToSet: (childSet: Object, childNode: Object) => void, completeRoot: (rootTag: number, childSet: Object) => void, registerEventHandler: ( diff --git a/scripts/flow/xplat.js b/scripts/flow/xplat.js index 889d76ea93371..86583241f6625 100644 --- a/scripts/flow/xplat.js +++ b/scripts/flow/xplat.js @@ -12,4 +12,5 @@ declare module 'ReactNativeInternalFeatureFlags' { declare export var enableDeferRootSchedulingToMicrotask: boolean; declare export var alwaysThrottleRetries: boolean; declare export var useMicrotasksForSchedulingInFabric: boolean; + declare export var passChildrenWhenCloningPersistedNodes: boolean; }