Clean up deferRenderPhaseUpdateToNextBatch (#26511)

acdlite · web-flow · commit 2d51251e608b · 2023-03-30T14:10:19.000-04:00
This is a change to some undefined behavior that we though we would do
at one point but decided not to roll out. It's already disabled
everywhere, so this just deletes the branch from the implementation and
the tests.
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -30,7 +30,6 @@ import {
   enableProfilerCommitHooks,
   enableProfilerNestedUpdatePhase,
   enableProfilerNestedUpdateScheduledHook,
-  deferRenderPhaseUpdateToNextBatch,
   enableDebugTracing,
   enableSchedulingProfiler,
   disableSchedulerTimeoutInWorkLoop,
@@ -636,7 +635,6 @@ export function requestUpdateLane(fiber: Fiber): Lane {
   if ((mode & ConcurrentMode) === NoMode) {
     return (SyncLane: Lane);
   } else if (
-    !deferRenderPhaseUpdateToNextBatch &&
     (executionContext & RenderContext) !== NoContext &&
     workInProgressRootRenderLanes !== NoLanes
   ) {
@@ -804,14 +802,8 @@ export function scheduleUpdateOnFiber(
 
     if (root === workInProgressRoot) {
       // Received an update to a tree that's in the middle of rendering. Mark
-      // that there was an interleaved update work on this root. Unless the
-      // `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
-      // phase update. In that case, we don't treat render phase updates as if
-      // they were interleaved, for backwards compat reasons.
-      if (
-        deferRenderPhaseUpdateToNextBatch ||
-        (executionContext & RenderContext) === NoContext
-      ) {
+      // that there was an interleaved update work on this root.
+      if ((executionContext & RenderContext) === NoContext) {
         workInProgressRootInterleavedUpdatedLanes = mergeLanes(
           workInProgressRootInterleavedUpdatedLanes,
           lane,
@@ -870,13 +862,7 @@ export function scheduleInitialHydrationOnRoot(
 export function isUnsafeClassRenderPhaseUpdate(fiber: Fiber): boolean {
   // Check if this is a render phase update. Only called by class components,
   // which special (deprecated) behavior for UNSAFE_componentWillReceive props.
-  return (
-    // TODO: Remove outdated deferRenderPhaseUpdateToNextBatch experiment. We
-    // decided not to enable it.
-    (!deferRenderPhaseUpdateToNextBatch ||
-      (fiber.mode & ConcurrentMode) === NoMode) &&
-    (executionContext & RenderContext) !== NoContext
-  );
+  return (executionContext & RenderContext) !== NoContext;
 }
 
 // Use this function to schedule a task for a root. There's only one task per
diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
@@ -387,11 +387,7 @@ describe('ReactIncrementalUpdates', () => {
 
     expect(instance.state).toEqual({a: 'a', b: 'b'});
 
-    if (gate(flags => flags.deferRenderPhaseUpdateToNextBatch)) {
-      assertLog(['componentWillReceiveProps', 'render', 'render']);
-    } else {
-      assertLog(['componentWillReceiveProps', 'render']);
-    }
+    assertLog(['componentWillReceiveProps', 'render']);
   });
 
   it('updates triggered from inside a class setState updater', async () => {
@@ -419,26 +415,12 @@ describe('ReactIncrementalUpdates', () => {
 
     await expect(
       async () =>
-        await waitForAll(
-          gate(flags =>
-            flags.deferRenderPhaseUpdateToNextBatch
-              ? [
-                  'setState updater',
-                  // In the new reconciler, updates inside the render phase are
-                  // treated as if they came from an event, so the update gets
-                  // shifted to a subsequent render.
-                  'render',
-                  'render',
-                ]
-              : [
-                  'setState updater',
-                  // In the old reconciler, updates in the render phase receive
-                  // the currently rendering expiration time, so the update
-                  // flushes immediately in the same render.
-                  'render',
-                ],
-          ),
-        ),
+        await waitForAll([
+          'setState updater',
+          // Updates in the render phase receive the currently rendering
+          // lane, so the update flushes immediately in the same render.
+          'render',
+        ]),
     ).toErrorDev(
       'An update (setState, replaceState, or forceUpdate) was scheduled ' +
         'from inside an update function. Update functions should be pure, ' +
@@ -454,15 +436,9 @@ describe('ReactIncrementalUpdates', () => {
     });
     await waitForAll(
       gate(flags =>
-        flags.deferRenderPhaseUpdateToNextBatch
-          ? // In the new reconciler, updates inside the render phase are
-            // treated as if they came from an event, so the update gets shifted
-            // to a subsequent render.
-            ['render', 'render']
-          : // In the old reconciler, updates in the render phase receive
-            // the currently rendering expiration time, so the update flushes
-            // immediately in the same render.
-            ['render'],
+        // Updates in the render phase receive the currently rendering
+        // lane, so the update flushes immediately in the same render.
+        ['render'],
       ),
     );
   });
diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
@@ -264,9 +264,7 @@ describe 'ReactCoffeeScriptClass', ->
         React.createElement('span', className: @state.bar)
 
     test React.createElement(Foo, initialValue: 'foo'), 'SPAN', 'bar'
-    # This is broken with deferRenderPhaseUpdateToNextBatch flag on.
-    # We can't use the gate feature here because this test is also in CoffeeScript and TypeScript.
-    expect(renderCount).toBe(if global.__WWW__ and !global.__VARIANT__ then 2 else 1)
+    expect(renderCount).toBe(1)
 
   it 'should warn with non-object in the initial state property', ->
     [['an array'], 'a string', 1234].forEach (state) ->
diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js
@@ -295,9 +295,7 @@ describe('ReactES6Class', () => {
       }
     }
     test(<Foo initialValue="foo" />, 'SPAN', 'bar');
-    // This is broken with deferRenderPhaseUpdateToNextBatch flag on.
-    // We can't use the gate feature here because this test is also in CoffeeScript and TypeScript.
-    expect(renderCount).toBe(global.__WWW__ && !global.__VARIANT__ ? 2 : 1);
+    expect(renderCount).toBe(1);
   });
 
   it('should warn with non-object in the initial state property', () => {
diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts
@@ -521,9 +521,7 @@ describe('ReactTypeScriptClass', function() {
   it('renders only once when setting state in componentWillMount', function() {
     renderCount = 0;
     test(React.createElement(RenderOnce, {initialValue: 'foo'}), 'SPAN', 'bar');
-    // This is broken with deferRenderPhaseUpdateToNextBatch flag on.
-    // We can't use the gate feature in TypeScript.
-    expect(renderCount).toBe(global.__WWW__ && !global.__VARIANT__ ? 2 : 1);
+    expect(renderCount).toBe(1);
   });
 
   it('should warn with non-object in the initial state property', function() {
diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -152,14 +152,6 @@ export const enableUnifiedSyncLane = __EXPERIMENTAL__;
 // startTransition. Only relevant when enableSyncDefaultUpdates is disabled.
 export const allowConcurrentByDefault = false;
 
-// Updates that occur in the render phase are not officially supported. But when
-// they do occur, we defer them to a subsequent render by picking a lane that's
-// not currently rendering. We treat them the same as if they came from an
-// interleaved event. Remove this flag once we have migrated to the
-// new behavior.
-// NOTE: Not sure if we'll end up doing this or not.
-export const deferRenderPhaseUpdateToNextBatch = false;
-
 // -----------------------------------------------------------------------------
 // React DOM Chopping Block
 //
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -60,7 +60,6 @@ export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
 export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = true;
-export const deferRenderPhaseUpdateToNextBatch = false;
 
 export const createRootStrictEffectsByDefault = false;
 
diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -50,7 +50,6 @@ export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
 export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
-export const deferRenderPhaseUpdateToNextBatch = false;
 
 export const createRootStrictEffectsByDefault = false;
 export const enableUseRefAccessWarning = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -50,7 +50,6 @@ export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
 export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
-export const deferRenderPhaseUpdateToNextBatch = false;
 
 export const createRootStrictEffectsByDefault = false;
 export const enableUseRefAccessWarning = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
@@ -41,7 +41,6 @@ export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
 export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
-export const deferRenderPhaseUpdateToNextBatch = false;
 export const enableSuspenseAvoidThisFallback = false;
 export const enableSuspenseAvoidThisFallbackFizz = false;
 export const enableCPUSuspense = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -50,7 +50,6 @@ export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = true;
 export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
-export const deferRenderPhaseUpdateToNextBatch = false;
 
 export const createRootStrictEffectsByDefault = false;
 export const enableUseRefAccessWarning = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js
@@ -35,16 +35,6 @@ export const enableDebugTracing = __EXPERIMENTAL__;
 
 export const enableSchedulingProfiler = __VARIANT__;
 
-// This only has an effect in the new reconciler. But also, the new reconciler
-// is only enabled when __VARIANT__ is true. So this is set to the opposite of
-// __VARIANT__ so that it's `false` when running against the new reconciler.
-// Ideally we would test both against the new reconciler, but until then, we
-// should test the value that is used in www. Which is `false`.
-//
-// Once Lanes has landed in both reconciler forks, we'll get coverage of
-// both branches.
-export const deferRenderPhaseUpdateToNextBatch = !__VARIANT__;
-
 // These are already tested in both modes using the build type dimension,
 // so we don't need to use __VARIANT__ to get extra coverage.
 export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -21,7 +21,6 @@ export const {
   revertRemovalOfSiblingPrerendering,
   replayFailedUnitOfWorkWithInvokeGuardedCallback,
   enableLegacyFBSupport,
-  deferRenderPhaseUpdateToNextBatch,
   enableDebugTracing,
   skipUnmountedBoundaries,
   enableUseRefAccessWarning,
