Skip to content

Cleanup props diffing experiments #33381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 30, 2025
Merged

Cleanup props diffing experiments #33381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 11 additions & 64 deletions packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -361,7 +321,6 @@ function diffProperties(
) {
// case: CustomAttributeConfiguration
const shouldUpdate =
enableShallowPropDiffing ||
prevProp === undefined ||
(typeof attributeConfig.diff === 'function'
? attributeConfig.diff(prevProp, nextProp)
Expand Down Expand Up @@ -452,15 +411,15 @@ function diffProperties(
return updatePayload;
}

function fastAddProperties(
function addNestedProperty(
payload: null | Object,
props: Object,
validAttributes: AttributeConfiguration,
): null | Object {
// 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;
}
Expand Down Expand Up @@ -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.
Expand All @@ -533,15 +481,14 @@ function clearProperties(
prevProps: Object,
validAttributes: AttributeConfiguration,
): null | Object {
// TODO: Fast path
return diffProperties(updatePayload, prevProps, emptyObject, validAttributes);
}

export function create(
props: Object,
validAttributes: AttributeConfiguration,
): null | Object {
return fastAddProperties(null, props, validAttributes);
return addNestedProperty(null, props, validAttributes);
}

export function diff(
Expand Down
3 changes: 0 additions & 3 deletions ...es/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -235,7 +234,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
expect(diffB).not.toBeCalled();
});

// @gate !enableShallowPropDiffing
it('should do deep diffs of Objects by default', () => {
expect(
diff(
Expand Down Expand Up @@ -433,7 +431,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
).toEqual(null);
});

// @gate !enableShallowPropDiffing
it('should skip deeply-nested changed functions', () => {
expect(
diff(
Expand Down
3 changes: 0 additions & 3 deletions packages/shared/ReactFeatureFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ export const passChildrenWhenCloningPersistedNodes = false;
*/
export const enablePersistedModeClonedFlag = false;

export const enableShallowPropDiffing = false;

export const enableEagerAlternateStateNodeCleanup = true;

/**
Expand All @@ -159,7 +157,6 @@ export const transitionLaneExpirationMs = 5000;
*/
export const enableInfiniteRenderLoopDetection = false;

export const enableFastAddPropertiesInDiffing = true;
export const enableLazyPublicInstanceInFabric = false;

export const enableFragmentRefs = __EXPERIMENTAL__;
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__;
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ export const {
enableHiddenSubtreeInsertionEffectCleanup,
enableObjectFiber,
enablePersistedModeClonedFlag,
enableShallowPropDiffing,
enableEagerAlternateStateNodeCleanup,
passChildrenWhenCloningPersistedNodes,
enableFastAddPropertiesInDiffing,
enableLazyPublicInstanceInFabric,
renameElementSymbol,
enableFragmentRefs,
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,13 @@ export const disableClientCache = true;
export const enableInfiniteRenderLoopDetection = false;

export const renameElementSymbol = true;
export const enableShallowPropDiffing = false;
export const enableEagerAlternateStateNodeCleanup = false;

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;
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions packages/shared/forks/ReactFeatureFlags.www.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const {
retryLaneExpirationMs,
syncLaneExpirationMs,
transitionLaneExpirationMs,
enableFastAddPropertiesInDiffing,
enableViewTransition,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep enableViewTransition, probably a rebase merge error since I had it briefly removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, good catch.

enableComponentPerformanceTrack,
enableScrollEndPolyfill,
Expand Down Expand Up @@ -105,8 +104,6 @@ export const enableReactTestRendererWarning = false;

export const disableLegacyMode = true;

export const enableShallowPropDiffing = false;

export const enableEagerAlternateStateNodeCleanup = false;

export const enableLazyPublicInstanceInFabric = false;
Expand Down
Loading