Cleanup props diffing experiments (facebook#33381)

javache · web-flow · commit 8b55eb4e7242 · 2025-05-30T17:17:59.000+01:00
## 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
```
diff --git a/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js b/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js
@@ -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 = {};
@@ -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;
 }
@@ -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.
@@ -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
@@ -361,7 +321,6 @@ function diffProperties(
     ) {
       // case: CustomAttributeConfiguration
       const shouldUpdate =
-        enableShallowPropDiffing ||
         prevProp === undefined ||
         (typeof attributeConfig.diff === 'function'
           ? attributeConfig.diff(prevProp, nextProp)
@@ -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;
   }
@@ -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.
@@ -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(
diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js
@@ -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);
@@ -235,7 +234,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
     expect(diffB).not.toBeCalled();
   });
 
-  // @gate !enableShallowPropDiffing
   it('should do deep diffs of Objects by default', () => {
     expect(
       diff(
@@ -433,7 +431,6 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
     ).toEqual(null);
   });
 
-  // @gate !enableShallowPropDiffing
   it('should skip deeply-nested changed functions', () => {
     expect(
       diff(
diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -141,8 +141,6 @@ export const passChildrenWhenCloningPersistedNodes = false;
  */
 export const enablePersistedModeClonedFlag = false;
 
-export const enableShallowPropDiffing = false;
-
 export const enableEagerAlternateStateNodeCleanup = true;
 
 /**
@@ -159,7 +157,6 @@ export const transitionLaneExpirationMs = 5000;
  */
 export const enableInfiniteRenderLoopDetection = false;
 
-export const enableFastAddPropertiesInDiffing = true;
 export const enableLazyPublicInstanceInFabric = false;
 
 export const enableFragmentRefs = __EXPERIMENTAL__;
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js
@@ -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__;
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -23,10 +23,8 @@ export const {
   enableHiddenSubtreeInsertionEffectCleanup,
   enableObjectFiber,
   enablePersistedModeClonedFlag,
-  enableShallowPropDiffing,
   enableEagerAlternateStateNodeCleanup,
   passChildrenWhenCloningPersistedNodes,
-  enableFastAddPropertiesInDiffing,
   enableLazyPublicInstanceInFabric,
   renameElementSymbol,
   enableFragmentRefs,
diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -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;
@@ -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;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -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;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js
@@ -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;
@@ -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;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -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;
@@ -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;
diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -33,7 +33,6 @@ export const {
   retryLaneExpirationMs,
   syncLaneExpirationMs,
   transitionLaneExpirationMs,
-  enableFastAddPropertiesInDiffing,
   enableViewTransition,
   enableComponentPerformanceTrack,
   enableScrollEndPolyfill,
@@ -105,8 +104,6 @@ export const enableReactTestRendererWarning = false;
 
 export const disableLegacyMode = true;
 
-export const enableShallowPropDiffing = false;
-
 export const enableEagerAlternateStateNodeCleanup = false;
 
 export const enableLazyPublicInstanceInFabric = false;
