Cleanup props diffing experiments (#33381)

## Summary

We completed testing on these internally, so can cleanup the separate
fast and slow paths and remove the `enableShallowPropDiffing` flag which
we're not pursuing.

## How did you test this change?

```
yarn test ReactNativeAttributePayloadFabric
```
This commit is contained in:
Pieter De Baets 2025-05-30 17:17:59 +01:00 committed by GitHub
parent 14094f80cb
commit 8b55eb4e72
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 11 additions and 85 deletions

View File

@ -14,11 +14,6 @@ import {
} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; } from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
import isArray from 'shared/isArray'; import isArray from 'shared/isArray';
import {
enableShallowPropDiffing,
enableFastAddPropertiesInDiffing,
} from 'shared/ReactFeatureFlags';
import type {AttributeConfiguration} from './ReactNativeTypes'; import type {AttributeConfiguration} from './ReactNativeTypes';
const emptyObject = {}; const emptyObject = {};
@ -141,12 +136,12 @@ function diffNestedArrayProperty(
); );
} }
for (; i < nextArray.length; i++) { for (; i < nextArray.length; i++) {
// Add all remaining properties. // Add all remaining properties
updatePayload = addNestedProperty( const nextProp = nextArray[i];
updatePayload, if (!nextProp) {
nextArray[i], continue;
validAttributes, }
); updatePayload = addNestedProperty(updatePayload, nextProp, validAttributes);
} }
return updatePayload; 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 * clearNestedProperty takes a single set of props and valid attributes. It
* adds a null sentinel to the updatePayload, for each prop key. * adds a null sentinel to the updatePayload, for each prop key.
@ -349,7 +309,7 @@ function diffProperties(
// Pattern match on: attributeConfig // Pattern match on: attributeConfig
if (typeof attributeConfig !== 'object') { if (typeof attributeConfig !== 'object') {
// case: !Object is the default case // case: !Object is the default case
if (enableShallowPropDiffing || defaultDiffer(prevProp, nextProp)) { if (defaultDiffer(prevProp, nextProp)) {
// a normal leaf has changed // a normal leaf has changed
(updatePayload || (updatePayload = ({}: {[string]: $FlowFixMe})))[ (updatePayload || (updatePayload = ({}: {[string]: $FlowFixMe})))[
propKey propKey
@ -361,7 +321,6 @@ function diffProperties(
) { ) {
// case: CustomAttributeConfiguration // case: CustomAttributeConfiguration
const shouldUpdate = const shouldUpdate =
enableShallowPropDiffing ||
prevProp === undefined || prevProp === undefined ||
(typeof attributeConfig.diff === 'function' (typeof attributeConfig.diff === 'function'
? attributeConfig.diff(prevProp, nextProp) ? attributeConfig.diff(prevProp, nextProp)
@ -452,7 +411,7 @@ function diffProperties(
return updatePayload; return updatePayload;
} }
function fastAddProperties( function addNestedProperty(
payload: null | Object, payload: null | Object,
props: Object, props: Object,
validAttributes: AttributeConfiguration, validAttributes: AttributeConfiguration,
@ -460,7 +419,7 @@ function fastAddProperties(
// Flatten nested style props. // Flatten nested style props.
if (isArray(props)) { if (isArray(props)) {
for (let i = 0; i < props.length; i++) { for (let i = 0; i < props.length; i++) {
payload = fastAddProperties(payload, props[i], validAttributes); payload = addNestedProperty(payload, props[i], validAttributes);
} }
return payload; return payload;
} }
@ -507,23 +466,12 @@ function fastAddProperties(
continue; continue;
} }
payload = fastAddProperties(payload, prop, attributeConfig); payload = addNestedProperty(payload, prop, attributeConfig);
} }
return payload; 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 * clearProperties clears all the previous props by adding a null sentinel
* to the payload for each valid key. * to the payload for each valid key.
@ -533,7 +481,6 @@ function clearProperties(
prevProps: Object, prevProps: Object,
validAttributes: AttributeConfiguration, validAttributes: AttributeConfiguration,
): null | Object { ): null | Object {
// TODO: Fast path
return diffProperties(updatePayload, prevProps, emptyObject, validAttributes); return diffProperties(updatePayload, prevProps, emptyObject, validAttributes);
} }
@ -541,7 +488,7 @@ export function create(
props: Object, props: Object,
validAttributes: AttributeConfiguration, validAttributes: AttributeConfiguration,
): null | Object { ): null | Object {
return fastAddProperties(null, props, validAttributes); return addNestedProperty(null, props, validAttributes);
} }
export function diff( export function diff(

View File

@ -210,7 +210,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
expect(diff({a: 1}, {b: 2}, {})).toEqual(null); expect(diff({a: 1}, {b: 2}, {})).toEqual(null);
}); });
// @gate !enableShallowPropDiffing
it('should use the diff attribute', () => { it('should use the diff attribute', () => {
const diffA = jest.fn((a, b) => true); const diffA = jest.fn((a, b) => true);
const diffB = jest.fn((a, b) => false); const diffB = jest.fn((a, b) => false);
@ -235,7 +234,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
expect(diffB).not.toBeCalled(); expect(diffB).not.toBeCalled();
}); });
// @gate !enableShallowPropDiffing
it('should do deep diffs of Objects by default', () => { it('should do deep diffs of Objects by default', () => {
expect( expect(
diff( diff(
@ -433,7 +431,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
).toEqual(null); ).toEqual(null);
}); });
// @gate !enableShallowPropDiffing
it('should skip deeply-nested changed functions', () => { it('should skip deeply-nested changed functions', () => {
expect( expect(
diff( diff(

View File

@ -141,8 +141,6 @@ export const passChildrenWhenCloningPersistedNodes = false;
*/ */
export const enablePersistedModeClonedFlag = false; export const enablePersistedModeClonedFlag = false;
export const enableShallowPropDiffing = false;
export const enableEagerAlternateStateNodeCleanup = true; export const enableEagerAlternateStateNodeCleanup = true;
/** /**
@ -159,7 +157,6 @@ export const transitionLaneExpirationMs = 5000;
*/ */
export const enableInfiniteRenderLoopDetection = false; export const enableInfiniteRenderLoopDetection = false;
export const enableFastAddPropertiesInDiffing = true;
export const enableLazyPublicInstanceInFabric = false; export const enableLazyPublicInstanceInFabric = false;
export const enableFragmentRefs = __EXPERIMENTAL__; export const enableFragmentRefs = __EXPERIMENTAL__;

View File

@ -21,10 +21,8 @@ export const alwaysThrottleRetries = __VARIANT__;
export const enableObjectFiber = __VARIANT__; export const enableObjectFiber = __VARIANT__;
export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__; export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__;
export const enablePersistedModeClonedFlag = __VARIANT__; export const enablePersistedModeClonedFlag = __VARIANT__;
export const enableShallowPropDiffing = __VARIANT__;
export const enableEagerAlternateStateNodeCleanup = __VARIANT__; export const enableEagerAlternateStateNodeCleanup = __VARIANT__;
export const passChildrenWhenCloningPersistedNodes = __VARIANT__; export const passChildrenWhenCloningPersistedNodes = __VARIANT__;
export const enableFastAddPropertiesInDiffing = __VARIANT__;
export const enableLazyPublicInstanceInFabric = __VARIANT__; export const enableLazyPublicInstanceInFabric = __VARIANT__;
export const renameElementSymbol = __VARIANT__; export const renameElementSymbol = __VARIANT__;
export const enableFragmentRefs = __VARIANT__; export const enableFragmentRefs = __VARIANT__;

View File

@ -23,10 +23,8 @@ export const {
enableHiddenSubtreeInsertionEffectCleanup, enableHiddenSubtreeInsertionEffectCleanup,
enableObjectFiber, enableObjectFiber,
enablePersistedModeClonedFlag, enablePersistedModeClonedFlag,
enableShallowPropDiffing,
enableEagerAlternateStateNodeCleanup, enableEagerAlternateStateNodeCleanup,
passChildrenWhenCloningPersistedNodes, passChildrenWhenCloningPersistedNodes,
enableFastAddPropertiesInDiffing,
enableLazyPublicInstanceInFabric, enableLazyPublicInstanceInFabric,
renameElementSymbol, renameElementSymbol,
enableFragmentRefs, enableFragmentRefs,

View File

@ -48,7 +48,6 @@ export const enableRetryLaneExpiration = false;
export const enableSchedulingProfiler = __PROFILE__; export const enableSchedulingProfiler = __PROFILE__;
export const enableComponentPerformanceTrack = false; export const enableComponentPerformanceTrack = false;
export const enableScopeAPI = false; export const enableScopeAPI = false;
export const enableShallowPropDiffing = false;
export const enableEagerAlternateStateNodeCleanup = false; export const enableEagerAlternateStateNodeCleanup = false;
export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallback = false;
export const enableSuspenseCallback = false; export const enableSuspenseCallback = false;
@ -69,7 +68,6 @@ export const enableYieldingBeforePassive = false;
export const enableThrottledScheduling = false; export const enableThrottledScheduling = false;
export const enableViewTransition = false; export const enableViewTransition = false;
export const enableGestureTransition = false; export const enableGestureTransition = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false; export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true; export const enableScrollEndPolyfill = true;
export const enableSuspenseyImages = false; export const enableSuspenseyImages = false;

View File

@ -61,7 +61,6 @@ export const disableClientCache = true;
export const enableInfiniteRenderLoopDetection = false; export const enableInfiniteRenderLoopDetection = false;
export const renameElementSymbol = true; export const renameElementSymbol = true;
export const enableShallowPropDiffing = false;
export const enableEagerAlternateStateNodeCleanup = false; export const enableEagerAlternateStateNodeCleanup = false;
export const enableYieldingBeforePassive = true; export const enableYieldingBeforePassive = true;
@ -69,7 +68,6 @@ export const enableYieldingBeforePassive = true;
export const enableThrottledScheduling = false; export const enableThrottledScheduling = false;
export const enableViewTransition = false; export const enableViewTransition = false;
export const enableGestureTransition = false; export const enableGestureTransition = false;
export const enableFastAddPropertiesInDiffing = true;
export const enableLazyPublicInstanceInFabric = false; export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true; export const enableScrollEndPolyfill = true;
export const enableSuspenseyImages = false; export const enableSuspenseyImages = false;

View File

@ -46,7 +46,6 @@ export const enableRetryLaneExpiration = false;
export const enableSchedulingProfiler = __PROFILE__; export const enableSchedulingProfiler = __PROFILE__;
export const enableComponentPerformanceTrack = false; export const enableComponentPerformanceTrack = false;
export const enableScopeAPI = false; export const enableScopeAPI = false;
export const enableShallowPropDiffing = false;
export const enableEagerAlternateStateNodeCleanup = false; export const enableEagerAlternateStateNodeCleanup = false;
export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallback = false;
export const enableSuspenseCallback = false; export const enableSuspenseCallback = false;
@ -66,7 +65,6 @@ export const enableYieldingBeforePassive = false;
export const enableThrottledScheduling = false; export const enableThrottledScheduling = false;
export const enableViewTransition = false; export const enableViewTransition = false;
export const enableGestureTransition = false; export const enableGestureTransition = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false; export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true; export const enableScrollEndPolyfill = true;
export const enableSuspenseyImages = false; export const enableSuspenseyImages = false;

View File

@ -70,7 +70,6 @@ export const disableDefaultPropsExceptForClasses = true;
export const renameElementSymbol = false; export const renameElementSymbol = false;
export const enableObjectFiber = false; export const enableObjectFiber = false;
export const enableShallowPropDiffing = false;
export const enableEagerAlternateStateNodeCleanup = false; export const enableEagerAlternateStateNodeCleanup = false;
export const enableHydrationLaneScheduling = true; export const enableHydrationLaneScheduling = true;
@ -80,7 +79,6 @@ export const enableYieldingBeforePassive = false;
export const enableThrottledScheduling = false; export const enableThrottledScheduling = false;
export const enableViewTransition = false; export const enableViewTransition = false;
export const enableGestureTransition = false; export const enableGestureTransition = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false; export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true; export const enableScrollEndPolyfill = true;
export const enableSuspenseyImages = false; export const enableSuspenseyImages = false;

View File

@ -33,7 +33,6 @@ export const {
retryLaneExpirationMs, retryLaneExpirationMs,
syncLaneExpirationMs, syncLaneExpirationMs,
transitionLaneExpirationMs, transitionLaneExpirationMs,
enableFastAddPropertiesInDiffing,
enableViewTransition, enableViewTransition,
enableComponentPerformanceTrack, enableComponentPerformanceTrack,
enableScrollEndPolyfill, enableScrollEndPolyfill,
@ -105,8 +104,6 @@ export const enableReactTestRendererWarning = false;
export const disableLegacyMode = true; export const disableLegacyMode = true;
export const enableShallowPropDiffing = false;
export const enableEagerAlternateStateNodeCleanup = false; export const enableEagerAlternateStateNodeCleanup = false;
export const enableLazyPublicInstanceInFabric = false; export const enableLazyPublicInstanceInFabric = false;