From 4113c256b212d443cd2689752db495ccc715c50b Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 30 May 2025 14:30:21 +0100 Subject: [PATCH] Cleanup props diffing experiments --- .../src/ReactNativeAttributePayloadFabric.js | 75 +++---------------- ...iveAttributePayloadFabric-test.internal.js | 3 - packages/shared/ReactFeatureFlags.js | 3 - .../ReactFeatureFlags.native-fb-dynamic.js | 2 - .../forks/ReactFeatureFlags.native-fb.js | 2 - .../forks/ReactFeatureFlags.native-oss.js | 2 - .../forks/ReactFeatureFlags.test-renderer.js | 2 - ...actFeatureFlags.test-renderer.native-fb.js | 2 - .../ReactFeatureFlags.test-renderer.www.js | 2 - .../shared/forks/ReactFeatureFlags.www.js | 3 - 10 files changed, 11 insertions(+), 85 deletions(-) diff --git a/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js b/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js index 35149914859fc..ecbef65733d4d 100644 --- a/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js +++ b/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js @@ -14,11 +14,6 @@ import { } from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; import isArray from 'shared/isArray'; -import { - enableShallowPropDiffing, - enableFastAddPropertiesInDiffing, -} from 'shared/ReactFeatureFlags'; - import type {AttributeConfiguration} from './ReactNativeTypes'; const emptyObject = {}; @@ -141,12 +136,12 @@ function diffNestedArrayProperty( ); } for (; i < nextArray.length; i++) { - // Add all remaining properties. - updatePayload = addNestedProperty( - updatePayload, - nextArray[i], - validAttributes, - ); + // Add all remaining properties + const nextProp = nextArray[i]; + if (!nextProp) { + continue; + } + updatePayload = addNestedProperty(updatePayload, nextProp, validAttributes); } return updatePayload; } @@ -205,41 +200,6 @@ function diffNestedProperty( ); } -/** - * addNestedProperty takes a single set of props and valid attribute - * attribute configurations. It processes each prop and adds it to the - * updatePayload. - */ -function addNestedProperty( - updatePayload: null | Object, - nextProp: NestedNode, - validAttributes: AttributeConfiguration, -): $FlowFixMe { - if (!nextProp) { - return updatePayload; - } - - if (enableFastAddPropertiesInDiffing) { - return fastAddProperties(updatePayload, nextProp, validAttributes); - } - - if (!isArray(nextProp)) { - // Add each property of the leaf. - return slowAddProperties(updatePayload, nextProp, validAttributes); - } - - for (let i = 0; i < nextProp.length; i++) { - // Add all the properties of the array. - updatePayload = addNestedProperty( - updatePayload, - nextProp[i], - validAttributes, - ); - } - - return updatePayload; -} - /** * clearNestedProperty takes a single set of props and valid attributes. It * adds a null sentinel to the updatePayload, for each prop key. @@ -349,7 +309,7 @@ function diffProperties( // Pattern match on: attributeConfig if (typeof attributeConfig !== 'object') { // case: !Object is the default case - if (enableShallowPropDiffing || defaultDiffer(prevProp, nextProp)) { + if (defaultDiffer(prevProp, nextProp)) { // a normal leaf has changed (updatePayload || (updatePayload = ({}: {[string]: $FlowFixMe})))[ propKey @@ -361,7 +321,6 @@ function diffProperties( ) { // case: CustomAttributeConfiguration const shouldUpdate = - enableShallowPropDiffing || prevProp === undefined || (typeof attributeConfig.diff === 'function' ? attributeConfig.diff(prevProp, nextProp) @@ -452,7 +411,7 @@ function diffProperties( return updatePayload; } -function fastAddProperties( +function addNestedProperty( payload: null | Object, props: Object, validAttributes: AttributeConfiguration, @@ -460,7 +419,7 @@ function fastAddProperties( // Flatten nested style props. if (isArray(props)) { for (let i = 0; i < props.length; i++) { - payload = fastAddProperties(payload, props[i], validAttributes); + payload = addNestedProperty(payload, props[i], validAttributes); } return payload; } @@ -507,23 +466,12 @@ function fastAddProperties( continue; } - payload = fastAddProperties(payload, prop, attributeConfig); + payload = addNestedProperty(payload, prop, attributeConfig); } return payload; } -/** - * addProperties adds all the valid props to the payload after being processed. - */ -function slowAddProperties( - updatePayload: null | Object, - props: Object, - validAttributes: AttributeConfiguration, -): null | Object { - return diffProperties(updatePayload, emptyObject, props, validAttributes); -} - /** * clearProperties clears all the previous props by adding a null sentinel * to the payload for each valid key. @@ -533,7 +481,6 @@ function clearProperties( prevProps: Object, validAttributes: AttributeConfiguration, ): null | Object { - // TODO: Fast path return diffProperties(updatePayload, prevProps, emptyObject, validAttributes); } @@ -541,7 +488,7 @@ export function create( props: Object, validAttributes: AttributeConfiguration, ): null | Object { - return fastAddProperties(null, props, validAttributes); + return addNestedProperty(null, props, validAttributes); } export function diff( diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js index 9136c932e601e..0225ca46012e6 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js @@ -210,7 +210,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => { expect(diff({a: 1}, {b: 2}, {})).toEqual(null); }); - // @gate !enableShallowPropDiffing it('should use the diff attribute', () => { const diffA = jest.fn((a, b) => true); const diffB = jest.fn((a, b) => false); @@ -235,7 +234,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => { expect(diffB).not.toBeCalled(); }); - // @gate !enableShallowPropDiffing it('should do deep diffs of Objects by default', () => { expect( diff( @@ -433,7 +431,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => { ).toEqual(null); }); - // @gate !enableShallowPropDiffing it('should skip deeply-nested changed functions', () => { expect( diff( diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index d8b5f448d5297..5b483297ee1a7 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -141,8 +141,6 @@ export const passChildrenWhenCloningPersistedNodes = false; */ export const enablePersistedModeClonedFlag = false; -export const enableShallowPropDiffing = false; - export const enableEagerAlternateStateNodeCleanup = true; /** @@ -159,7 +157,6 @@ export const transitionLaneExpirationMs = 5000; */ export const enableInfiniteRenderLoopDetection = false; -export const enableFastAddPropertiesInDiffing = true; export const enableLazyPublicInstanceInFabric = false; export const enableFragmentRefs = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index aa413984e8f38..f41147dc9ef17 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -21,10 +21,8 @@ export const alwaysThrottleRetries = __VARIANT__; export const enableObjectFiber = __VARIANT__; export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__; export const enablePersistedModeClonedFlag = __VARIANT__; -export const enableShallowPropDiffing = __VARIANT__; export const enableEagerAlternateStateNodeCleanup = __VARIANT__; export const passChildrenWhenCloningPersistedNodes = __VARIANT__; -export const enableFastAddPropertiesInDiffing = __VARIANT__; export const enableLazyPublicInstanceInFabric = __VARIANT__; export const renameElementSymbol = __VARIANT__; export const enableFragmentRefs = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 4298a267a36ff..1fb2a24671f91 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -23,10 +23,8 @@ export const { enableHiddenSubtreeInsertionEffectCleanup, enableObjectFiber, enablePersistedModeClonedFlag, - enableShallowPropDiffing, enableEagerAlternateStateNodeCleanup, passChildrenWhenCloningPersistedNodes, - enableFastAddPropertiesInDiffing, enableLazyPublicInstanceInFabric, renameElementSymbol, enableFragmentRefs, diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 8a7a59ebb2654..e142ef2865df3 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -48,7 +48,6 @@ export const enableRetryLaneExpiration = false; export const enableSchedulingProfiler = __PROFILE__; export const enableComponentPerformanceTrack = false; export const enableScopeAPI = false; -export const enableShallowPropDiffing = false; export const enableEagerAlternateStateNodeCleanup = false; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseCallback = false; @@ -69,7 +68,6 @@ export const enableYieldingBeforePassive = false; export const enableThrottledScheduling = false; export const enableViewTransition = false; export const enableGestureTransition = false; -export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 6de2578838ff6..811f777f2b80b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -61,7 +61,6 @@ export const disableClientCache = true; export const enableInfiniteRenderLoopDetection = false; export const renameElementSymbol = true; -export const enableShallowPropDiffing = false; export const enableEagerAlternateStateNodeCleanup = false; export const enableYieldingBeforePassive = true; @@ -69,7 +68,6 @@ export const enableYieldingBeforePassive = true; export const enableThrottledScheduling = false; export const enableViewTransition = false; export const enableGestureTransition = false; -export const enableFastAddPropertiesInDiffing = true; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 643626d6f9731..61d6017642ea8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -46,7 +46,6 @@ export const enableRetryLaneExpiration = false; export const enableSchedulingProfiler = __PROFILE__; export const enableComponentPerformanceTrack = false; export const enableScopeAPI = false; -export const enableShallowPropDiffing = false; export const enableEagerAlternateStateNodeCleanup = false; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseCallback = false; @@ -66,7 +65,6 @@ export const enableYieldingBeforePassive = false; export const enableThrottledScheduling = false; export const enableViewTransition = false; export const enableGestureTransition = false; -export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index d22d5cadd744d..d17c23524f76d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -70,7 +70,6 @@ export const disableDefaultPropsExceptForClasses = true; export const renameElementSymbol = false; export const enableObjectFiber = false; -export const enableShallowPropDiffing = false; export const enableEagerAlternateStateNodeCleanup = false; export const enableHydrationLaneScheduling = true; @@ -80,7 +79,6 @@ export const enableYieldingBeforePassive = false; export const enableThrottledScheduling = false; export const enableViewTransition = false; export const enableGestureTransition = false; -export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 839248e2e7b04..e22e6d2aff362 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -33,7 +33,6 @@ export const { retryLaneExpirationMs, syncLaneExpirationMs, transitionLaneExpirationMs, - enableFastAddPropertiesInDiffing, enableViewTransition, enableComponentPerformanceTrack, enableScrollEndPolyfill, @@ -105,8 +104,6 @@ export const enableReactTestRendererWarning = false; export const disableLegacyMode = true; -export const enableShallowPropDiffing = false; - export const enableEagerAlternateStateNodeCleanup = false; export const enableLazyPublicInstanceInFabric = false;