Skip to content

Commit 8b55eb4

Browse files
javachejavache
authored
Cleanup props diffing experiments (facebook#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 ```
1 parent 14094f8 commit 8b55eb4

10 files changed

+11
-85
lines changed

‎packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js

Lines changed: 11 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,6 @@ import {
1414
} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
1515
import isArray from 'shared/isArray';
1616

17-
import {
18-
enableShallowPropDiffing,
19-
enableFastAddPropertiesInDiffing,
20-
} from 'shared/ReactFeatureFlags';
21-
2217
import type {AttributeConfiguration} from './ReactNativeTypes';
2318

2419
const emptyObject = {};
@@ -141,12 +136,12 @@ function diffNestedArrayProperty(
141136
);
142137
}
143138
for (; i < nextArray.length; i++) {
144-
// Add all remaining properties.
145-
updatePayload = addNestedProperty(
146-
updatePayload,
147-
nextArray[i],
148-
validAttributes,
149-
);
139+
// Add all remaining properties
140+
const nextProp = nextArray[i];
141+
if (!nextProp) {
142+
continue;
143+
}
144+
updatePayload = addNestedProperty(updatePayload, nextProp, validAttributes);
150145
}
151146
return updatePayload;
152147
}
@@ -205,41 +200,6 @@ function diffNestedProperty(
205200
);
206201
}
207202

208-
/**
209-
* addNestedProperty takes a single set of props and valid attribute
210-
* attribute configurations. It processes each prop and adds it to the
211-
* updatePayload.
212-
*/
213-
function addNestedProperty(
214-
updatePayload: null | Object,
215-
nextProp: NestedNode,
216-
validAttributes: AttributeConfiguration,
217-
): $FlowFixMe {
218-
if (!nextProp) {
219-
return updatePayload;
220-
}
221-
222-
if (enableFastAddPropertiesInDiffing) {
223-
return fastAddProperties(updatePayload, nextProp, validAttributes);
224-
}
225-
226-
if (!isArray(nextProp)) {
227-
// Add each property of the leaf.
228-
return slowAddProperties(updatePayload, nextProp, validAttributes);
229-
}
230-
231-
for (let i = 0; i < nextProp.length; i++) {
232-
// Add all the properties of the array.
233-
updatePayload = addNestedProperty(
234-
updatePayload,
235-
nextProp[i],
236-
validAttributes,
237-
);
238-
}
239-
240-
return updatePayload;
241-
}
242-
243203
/**
244204
* clearNestedProperty takes a single set of props and valid attributes. It
245205
* adds a null sentinel to the updatePayload, for each prop key.
@@ -349,7 +309,7 @@ function diffProperties(
349309
// Pattern match on: attributeConfig
350310
if (typeof attributeConfig !== 'object') {
351311
// case: !Object is the default case
352-
if (enableShallowPropDiffing || defaultDiffer(prevProp, nextProp)) {
312+
if (defaultDiffer(prevProp, nextProp)) {
353313
// a normal leaf has changed
354314
(updatePayload || (updatePayload = ({}: {[string]: $FlowFixMe})))[
355315
propKey
@@ -361,7 +321,6 @@ function diffProperties(
361321
) {
362322
// case: CustomAttributeConfiguration
363323
const shouldUpdate =
364-
enableShallowPropDiffing ||
365324
prevProp === undefined ||
366325
(typeof attributeConfig.diff === 'function'
367326
? attributeConfig.diff(prevProp, nextProp)
@@ -452,15 +411,15 @@ function diffProperties(
452411
return updatePayload;
453412
}
454413

455-
function fastAddProperties(
414+
function addNestedProperty(
456415
payload: null | Object,
457416
props: Object,
458417
validAttributes: AttributeConfiguration,
459418
): null | Object {
460419
// Flatten nested style props.
461420
if (isArray(props)) {
462421
for (let i = 0; i < props.length; i++) {
463-
payload = fastAddProperties(payload, props[i], validAttributes);
422+
payload = addNestedProperty(payload, props[i], validAttributes);
464423
}
465424
return payload;
466425
}
@@ -507,23 +466,12 @@ function fastAddProperties(
507466
continue;
508467
}
509468

510-
payload = fastAddProperties(payload, prop, attributeConfig);
469+
payload = addNestedProperty(payload, prop, attributeConfig);
511470
}
512471

513472
return payload;
514473
}
515474

516-
/**
517-
* addProperties adds all the valid props to the payload after being processed.
518-
*/
519-
function slowAddProperties(
520-
updatePayload: null | Object,
521-
props: Object,
522-
validAttributes: AttributeConfiguration,
523-
): null | Object {
524-
return diffProperties(updatePayload, emptyObject, props, validAttributes);
525-
}
526-
527475
/**
528476
* clearProperties clears all the previous props by adding a null sentinel
529477
* to the payload for each valid key.
@@ -533,15 +481,14 @@ function clearProperties(
533481
prevProps: Object,
534482
validAttributes: AttributeConfiguration,
535483
): null | Object {
536-
// TODO: Fast path
537484
return diffProperties(updatePayload, prevProps, emptyObject, validAttributes);
538485
}
539486

540487
export function create(
541488
props: Object,
542489
validAttributes: AttributeConfiguration,
543490
): null | Object {
544-
return fastAddProperties(null, props, validAttributes);
491+
return addNestedProperty(null, props, validAttributes);
545492
}
546493

547494
export function diff(

‎packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
210210
expect(diff({a: 1}, {b: 2}, {})).toEqual(null);
211211
});
212212

213-
// @gate !enableShallowPropDiffing
214213
it('should use the diff attribute', () => {
215214
const diffA = jest.fn((a, b) => true);
216215
const diffB = jest.fn((a, b) => false);
@@ -235,7 +234,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
235234
expect(diffB).not.toBeCalled();
236235
});
237236

238-
// @gate !enableShallowPropDiffing
239237
it('should do deep diffs of Objects by default', () => {
240238
expect(
241239
diff(
@@ -433,7 +431,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
433431
).toEqual(null);
434432
});
435433

436-
// @gate !enableShallowPropDiffing
437434
it('should skip deeply-nested changed functions', () => {
438435
expect(
439436
diff(

‎packages/shared/ReactFeatureFlags.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,6 @@ export const passChildrenWhenCloningPersistedNodes = false;
141141
*/
142142
export const enablePersistedModeClonedFlag = false;
143143

144-
export const enableShallowPropDiffing = false;
145-
146144
export const enableEagerAlternateStateNodeCleanup = true;
147145

148146
/**
@@ -159,7 +157,6 @@ export const transitionLaneExpirationMs = 5000;
159157
*/
160158
export const enableInfiniteRenderLoopDetection = false;
161159

162-
export const enableFastAddPropertiesInDiffing = true;
163160
export const enableLazyPublicInstanceInFabric = false;
164161

165162
export const enableFragmentRefs = __EXPERIMENTAL__;

‎packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@ export const alwaysThrottleRetries = __VARIANT__;
2121
export const enableObjectFiber = __VARIANT__;
2222
export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__;
2323
export const enablePersistedModeClonedFlag = __VARIANT__;
24-
export const enableShallowPropDiffing = __VARIANT__;
2524
export const enableEagerAlternateStateNodeCleanup = __VARIANT__;
2625
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;
27-
export const enableFastAddPropertiesInDiffing = __VARIANT__;
2826
export const enableLazyPublicInstanceInFabric = __VARIANT__;
2927
export const renameElementSymbol = __VARIANT__;
3028
export const enableFragmentRefs = __VARIANT__;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,8 @@ export const {
2323
enableHiddenSubtreeInsertionEffectCleanup,
2424
enableObjectFiber,
2525
enablePersistedModeClonedFlag,
26-
enableShallowPropDiffing,
2726
enableEagerAlternateStateNodeCleanup,
2827
passChildrenWhenCloningPersistedNodes,
29-
enableFastAddPropertiesInDiffing,
3028
enableLazyPublicInstanceInFabric,
3129
renameElementSymbol,
3230
enableFragmentRefs,

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ export const enableRetryLaneExpiration = false;
4848
export const enableSchedulingProfiler = __PROFILE__;
4949
export const enableComponentPerformanceTrack = false;
5050
export const enableScopeAPI = false;
51-
export const enableShallowPropDiffing = false;
5251
export const enableEagerAlternateStateNodeCleanup = false;
5352
export const enableSuspenseAvoidThisFallback = false;
5453
export const enableSuspenseCallback = false;
@@ -69,7 +68,6 @@ export const enableYieldingBeforePassive = false;
6968
export const enableThrottledScheduling = false;
7069
export const enableViewTransition = false;
7170
export const enableGestureTransition = false;
72-
export const enableFastAddPropertiesInDiffing = false;
7371
export const enableLazyPublicInstanceInFabric = false;
7472
export const enableScrollEndPolyfill = true;
7573
export const enableSuspenseyImages = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,13 @@ export const disableClientCache = true;
6161
export const enableInfiniteRenderLoopDetection = false;
6262

6363
export const renameElementSymbol = true;
64-
export const enableShallowPropDiffing = false;
6564
export const enableEagerAlternateStateNodeCleanup = false;
6665

6766
export const enableYieldingBeforePassive = true;
6867

6968
export const enableThrottledScheduling = false;
7069
export const enableViewTransition = false;
7170
export const enableGestureTransition = false;
72-
export const enableFastAddPropertiesInDiffing = true;
7371
export const enableLazyPublicInstanceInFabric = false;
7472
export const enableScrollEndPolyfill = true;
7573
export const enableSuspenseyImages = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ export const enableRetryLaneExpiration = false;
4646
export const enableSchedulingProfiler = __PROFILE__;
4747
export const enableComponentPerformanceTrack = false;
4848
export const enableScopeAPI = false;
49-
export const enableShallowPropDiffing = false;
5049
export const enableEagerAlternateStateNodeCleanup = false;
5150
export const enableSuspenseAvoidThisFallback = false;
5251
export const enableSuspenseCallback = false;
@@ -66,7 +65,6 @@ export const enableYieldingBeforePassive = false;
6665
export const enableThrottledScheduling = false;
6766
export const enableViewTransition = false;
6867
export const enableGestureTransition = false;
69-
export const enableFastAddPropertiesInDiffing = false;
7068
export const enableLazyPublicInstanceInFabric = false;
7169
export const enableScrollEndPolyfill = true;
7270
export const enableSuspenseyImages = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ export const disableDefaultPropsExceptForClasses = true;
7070
export const renameElementSymbol = false;
7171

7272
export const enableObjectFiber = false;
73-
export const enableShallowPropDiffing = false;
7473
export const enableEagerAlternateStateNodeCleanup = false;
7574

7675
export const enableHydrationLaneScheduling = true;
@@ -80,7 +79,6 @@ export const enableYieldingBeforePassive = false;
8079
export const enableThrottledScheduling = false;
8180
export const enableViewTransition = false;
8281
export const enableGestureTransition = false;
83-
export const enableFastAddPropertiesInDiffing = false;
8482
export const enableLazyPublicInstanceInFabric = false;
8583
export const enableScrollEndPolyfill = true;
8684
export const enableSuspenseyImages = false;

‎packages/shared/forks/ReactFeatureFlags.www.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ export const {
3333
retryLaneExpirationMs,
3434
syncLaneExpirationMs,
3535
transitionLaneExpirationMs,
36-
enableFastAddPropertiesInDiffing,
3736
enableViewTransition,
3837
enableComponentPerformanceTrack,
3938
enableScrollEndPolyfill,
@@ -105,8 +104,6 @@ export const enableReactTestRendererWarning = false;
105104

106105
export const disableLegacyMode = true;
107106

108-
export const enableShallowPropDiffing = false;
109-
110107
export const enableEagerAlternateStateNodeCleanup = false;
111108

112109
export const enableLazyPublicInstanceInFabric = false;

0 commit comments

Comments
 (0)