Add enableShallowPropDiffing feature flag (#29664)
## Summary We currently do deep diffing for object props, and also use custom differs, if they are defined, for props with custom attribute config. The idea is to simply do a `===` comparison instead of all that work. We will do less computation on the JS side, but send more data to native. The hypothesis is that this change should be neutral in terms of performance. If that's the case, we'll be able to get rid of custom differs, and be one step closer to deleting view configs. This PR adds the `enableShallowPropDiffing` feature flag to support this experiment. ## How did you test this change? With `enableShallowPropDiffing` hardcoded to `true`: ``` yarn test packages/react-native-renderer ``` This fails on the following test cases: - should use the diff attribute - should do deep diffs of Objects by default - should skip deeply-nested changed functions Which makes sense with this change. These test cases should be deleted if the experiment is shipped.
Dmytro Rykun committed
Jun 5, 2024 at 15:07 UTC
eb259b5d3b20b053dc0444e6ae442774c396c4a7
10 files changed
+20
-5
packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js
+6
-2
@@ -14,7 +14,10 @@ import {
14
} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
15
import isArray from 'shared/isArray';
16
17
-import {enableAddPropertiesFastPath} from 'shared/ReactFeatureFlags';
17
+import {
18
+ enableAddPropertiesFastPath,
19
+ enableShallowPropDiffing,
20
+} from 'shared/ReactFeatureFlags';
21
22
import type {AttributeConfiguration} from './ReactNativeTypes';
23
@@ -342,7 +345,7 @@ function diffProperties(
345
// Pattern match on: attributeConfig
346
if (typeof attributeConfig !== 'object') {
347
// case: !Object is the default case
345
- if (defaultDiffer(prevProp, nextProp)) {
348
+ if (enableShallowPropDiffing || defaultDiffer(prevProp, nextProp)) {
349
// a normal leaf has changed
350
(updatePayload || (updatePayload = ({}: {[string]: $FlowFixMe})))[
351
propKey
@@ -354,6 +357,7 @@ function diffProperties(
357
) {
358
// case: CustomAttributeConfiguration
359
const shouldUpdate =
360
+ enableShallowPropDiffing ||
361
prevProp === undefined ||
362
(typeof attributeConfig.diff === 'function'
363
? attributeConfig.diff(prevProp, nextProp)
packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js
+5
-2
@@ -10,7 +10,7 @@
10
11
const {diff, create} = require('../ReactNativeAttributePayloadFabric');
12
13
-describe('ReactNativeAttributePayload.create', () => {
13
+describe('ReactNativeAttributePayloadFabric.create', () => {
14
it('should work with simple example', () => {
15
expect(create({b: 2, c: 3}, {a: true, b: true})).toEqual({
16
b: 2,
@@ -171,7 +171,7 @@ describe('ReactNativeAttributePayload.create', () => {
171
});
172
});
173
174
-describe('ReactNativeAttributePayload.diff', () => {
174
+describe('ReactNativeAttributePayloadFabric.diff', () => {
175
it('should work with simple example', () => {
176
expect(diff({a: 1, c: 3}, {b: 2, c: 3}, {a: true, b: true})).toEqual({
177
a: null,
@@ -201,6 +201,7 @@ describe('ReactNativeAttributePayload.diff', () => {
201
expect(diff({a: 1}, {b: 2}, {})).toEqual(null);
202
});
203
204
+ // @gate !enableShallowPropDiffing
205
it('should use the diff attribute', () => {
206
const diffA = jest.fn((a, b) => true);
207
const diffB = jest.fn((a, b) => false);
@@ -225,6 +226,7 @@ describe('ReactNativeAttributePayload.diff', () => {
226
expect(diffB).not.toBeCalled();
227
});
228
229
+ // @gate !enableShallowPropDiffing
230
it('should do deep diffs of Objects by default', () => {
231
expect(
232
diff(
@@ -422,6 +424,7 @@ describe('ReactNativeAttributePayload.diff', () => {
424
).toEqual(null);
425
});
426
427
+ // @gate !enableShallowPropDiffing
428
it('should skip deeply-nested changed functions', () => {
429
expect(
430
diff(
packages/shared/ReactFeatureFlags.js
+2
@@ -125,6 +125,8 @@ export const enableAddPropertiesFastPath = false;
125
126
export const enableOwnerStacks = __EXPERIMENTAL__;
127
128
+export const enableShallowPropDiffing = false;
129
+
130
/**
131
* Enables an expiration time for retry lanes to avoid starvation.
132
*/
packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js
+1
@@ -24,4 +24,5 @@ export const enableAddPropertiesFastPath = __VARIANT__;
24
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
25
export const enableFastJSX = __VARIANT__;
26
export const enableInfiniteRenderLoopDetection = __VARIANT__;
27
+export const enableShallowPropDiffing = __VARIANT__;
28
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -26,6 +26,7 @@ export const {
26
enableDeferRootSchedulingToMicrotask,
27
enableFastJSX,
28
enableInfiniteRenderLoopDetection,
29
+ enableShallowPropDiffing,
30
passChildrenWhenCloningPersistedNodes,
31
} = dynamicFlags;
32
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
-1
@@ -102,7 +102,7 @@ export const enableDO_NOT_USE_disableStrictPassiveEffect = false;
102
export const passChildrenWhenCloningPersistedNodes = false;
103
export const enableAsyncIterableChildren = false;
104
export const enableAddPropertiesFastPath = false;
105
-
105
+export const enableShallowPropDiffing = false;
106
export const renameElementSymbol = true;
107
108
export const enableOwnerStacks = __EXPERIMENTAL__;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -79,6 +79,7 @@ export const enableInfiniteRenderLoopDetection = false;
79
export const enableAddPropertiesFastPath = false;
80
81
export const renameElementSymbol = true;
82
+export const enableShallowPropDiffing = false;
83
84
// TODO: This must be in sync with the main ReactFeatureFlags file because
85
// the Test Renderer's value must be the same as the one used by the
packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js
+1
@@ -92,6 +92,7 @@ export const enableAddPropertiesFastPath = false;
92
export const renameElementSymbol = false;
93
94
export const enableOwnerStacks = false;
95
+export const enableShallowPropDiffing = false;
96
97
// Flow magic to verify the exports of this file match the original version.
98
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -92,6 +92,7 @@ export const enableAddPropertiesFastPath = false;
92
export const renameElementSymbol = false;
93
94
export const enableOwnerStacks = false;
95
+export const enableShallowPropDiffing = false;
96
97
// Flow magic to verify the exports of this file match the original version.
98
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -122,6 +122,7 @@ export const disableStringRefs = false;
122
export const disableLegacyMode = __EXPERIMENTAL__;
123
124
export const enableOwnerStacks = false;
125
+export const enableShallowPropDiffing = false;
126
127
// Flow magic to verify the exports of this file match the original version.
128
((((null: any): ExportsType): FeatureFlagsType): ExportsType);