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.
This commit is contained in:
Dmytro Rykun
2024-06-05 15:07:58 +01:00
committed by GitHub
parent f11b27653f
commit 3b2f2433c3
10 changed files with 20 additions and 5 deletions
@@ -14,7 +14,10 @@ import {
} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
import isArray from 'shared/isArray';
import {enableAddPropertiesFastPath} from 'shared/ReactFeatureFlags';
import {
enableAddPropertiesFastPath,
enableShallowPropDiffing,
} from 'shared/ReactFeatureFlags';
import type {AttributeConfiguration} from './ReactNativeTypes';
@@ -342,7 +345,7 @@ function diffProperties(
// Pattern match on: attributeConfig
if (typeof attributeConfig !== 'object') {
// case: !Object is the default case
if (defaultDiffer(prevProp, nextProp)) {
if (enableShallowPropDiffing || defaultDiffer(prevProp, nextProp)) {
// a normal leaf has changed
(updatePayload || (updatePayload = ({}: {[string]: $FlowFixMe})))[
propKey
@@ -354,6 +357,7 @@ function diffProperties(
) {
// case: CustomAttributeConfiguration
const shouldUpdate =
enableShallowPropDiffing ||
prevProp === undefined ||
(typeof attributeConfig.diff === 'function'
? attributeConfig.diff(prevProp, nextProp)
@@ -10,7 +10,7 @@
const {diff, create} = require('../ReactNativeAttributePayloadFabric');
describe('ReactNativeAttributePayload.create', () => {
describe('ReactNativeAttributePayloadFabric.create', () => {
it('should work with simple example', () => {
expect(create({b: 2, c: 3}, {a: true, b: true})).toEqual({
b: 2,
@@ -171,7 +171,7 @@ describe('ReactNativeAttributePayload.create', () => {
});
});
describe('ReactNativeAttributePayload.diff', () => {
describe('ReactNativeAttributePayloadFabric.diff', () => {
it('should work with simple example', () => {
expect(diff({a: 1, c: 3}, {b: 2, c: 3}, {a: true, b: true})).toEqual({
a: null,
@@ -201,6 +201,7 @@ describe('ReactNativeAttributePayload.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);
@@ -225,6 +226,7 @@ describe('ReactNativeAttributePayload.diff', () => {
expect(diffB).not.toBeCalled();
});
// @gate !enableShallowPropDiffing
it('should do deep diffs of Objects by default', () => {
expect(
diff(
@@ -422,6 +424,7 @@ describe('ReactNativeAttributePayload.diff', () => {
).toEqual(null);
});
// @gate !enableShallowPropDiffing
it('should skip deeply-nested changed functions', () => {
expect(
diff(
+2
View File
@@ -125,6 +125,8 @@ export const enableAddPropertiesFastPath = false;
export const enableOwnerStacks = __EXPERIMENTAL__;
export const enableShallowPropDiffing = false;
/**
* Enables an expiration time for retry lanes to avoid starvation.
*/
@@ -24,4 +24,5 @@ export const enableAddPropertiesFastPath = __VARIANT__;
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
export const enableFastJSX = __VARIANT__;
export const enableInfiniteRenderLoopDetection = __VARIANT__;
export const enableShallowPropDiffing = __VARIANT__;
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;
@@ -26,6 +26,7 @@ export const {
enableDeferRootSchedulingToMicrotask,
enableFastJSX,
enableInfiniteRenderLoopDetection,
enableShallowPropDiffing,
passChildrenWhenCloningPersistedNodes,
} = dynamicFlags;
@@ -102,7 +102,7 @@ export const enableDO_NOT_USE_disableStrictPassiveEffect = false;
export const passChildrenWhenCloningPersistedNodes = false;
export const enableAsyncIterableChildren = false;
export const enableAddPropertiesFastPath = false;
export const enableShallowPropDiffing = false;
export const renameElementSymbol = true;
export const enableOwnerStacks = __EXPERIMENTAL__;
@@ -79,6 +79,7 @@ export const enableInfiniteRenderLoopDetection = false;
export const enableAddPropertiesFastPath = false;
export const renameElementSymbol = true;
export const enableShallowPropDiffing = false;
// TODO: This must be in sync with the main ReactFeatureFlags file because
// the Test Renderer's value must be the same as the one used by the
@@ -92,6 +92,7 @@ export const enableAddPropertiesFastPath = false;
export const renameElementSymbol = false;
export const enableOwnerStacks = false;
export const enableShallowPropDiffing = false;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
@@ -92,6 +92,7 @@ export const enableAddPropertiesFastPath = false;
export const renameElementSymbol = false;
export const enableOwnerStacks = false;
export const enableShallowPropDiffing = false;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
@@ -122,6 +122,7 @@ export const disableStringRefs = false;
export const disableLegacyMode = __EXPERIMENTAL__;
export const enableOwnerStacks = false;
export const enableShallowPropDiffing = false;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);