Skip to content

Commit 2d51251

Browse files
acdliteacdlite
authored
Clean up deferRenderPhaseUpdateToNextBatch (#26511)
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.
1 parent 0ffc7f6 commit 2d51251

13 files changed

+16
-84
lines changed

‎packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import {
3030
enableProfilerCommitHooks,
3131
enableProfilerNestedUpdatePhase,
3232
enableProfilerNestedUpdateScheduledHook,
33-
deferRenderPhaseUpdateToNextBatch,
3433
enableDebugTracing,
3534
enableSchedulingProfiler,
3635
disableSchedulerTimeoutInWorkLoop,
@@ -636,7 +635,6 @@ export function requestUpdateLane(fiber: Fiber): Lane {
636635
if ((mode & ConcurrentMode) === NoMode) {
637636
return (SyncLane: Lane);
638637
} else if (
639-
!deferRenderPhaseUpdateToNextBatch &&
640638
(executionContext & RenderContext) !== NoContext &&
641639
workInProgressRootRenderLanes !== NoLanes
642640
) {
@@ -804,14 +802,8 @@ export function scheduleUpdateOnFiber(
804802

805803
if (root === workInProgressRoot) {
806804
// Received an update to a tree that's in the middle of rendering. Mark
807-
// that there was an interleaved update work on this root. Unless the
808-
// `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
809-
// phase update. In that case, we don't treat render phase updates as if
810-
// they were interleaved, for backwards compat reasons.
811-
if (
812-
deferRenderPhaseUpdateToNextBatch ||
813-
(executionContext & RenderContext) === NoContext
814-
) {
805+
// that there was an interleaved update work on this root.
806+
if ((executionContext & RenderContext) === NoContext) {
815807
workInProgressRootInterleavedUpdatedLanes = mergeLanes(
816808
workInProgressRootInterleavedUpdatedLanes,
817809
lane,
@@ -870,13 +862,7 @@ export function scheduleInitialHydrationOnRoot(
870862
export function isUnsafeClassRenderPhaseUpdate(fiber: Fiber): boolean {
871863
// Check if this is a render phase update. Only called by class components,
872864
// which special (deprecated) behavior for UNSAFE_componentWillReceive props.
873-
return (
874-
// TODO: Remove outdated deferRenderPhaseUpdateToNextBatch experiment. We
875-
// decided not to enable it.
876-
(!deferRenderPhaseUpdateToNextBatch ||
877-
(fiber.mode & ConcurrentMode) === NoMode) &&
878-
(executionContext & RenderContext) !== NoContext
879-
);
865+
return (executionContext & RenderContext) !== NoContext;
880866
}
881867

882868
// Use this function to schedule a task for a root. There's only one task per

‎packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -387,11 +387,7 @@ describe('ReactIncrementalUpdates', () => {
387387

388388
expect(instance.state).toEqual({a: 'a', b: 'b'});
389389

390-
if (gate(flags => flags.deferRenderPhaseUpdateToNextBatch)) {
391-
assertLog(['componentWillReceiveProps', 'render', 'render']);
392-
} else {
393-
assertLog(['componentWillReceiveProps', 'render']);
394-
}
390+
assertLog(['componentWillReceiveProps', 'render']);
395391
});
396392

397393
it('updates triggered from inside a class setState updater', async () => {
@@ -419,26 +415,12 @@ describe('ReactIncrementalUpdates', () => {
419415

420416
await expect(
421417
async () =>
422-
await waitForAll(
423-
gate(flags =>
424-
flags.deferRenderPhaseUpdateToNextBatch
425-
? [
426-
'setState updater',
427-
// In the new reconciler, updates inside the render phase are
428-
// treated as if they came from an event, so the update gets
429-
// shifted to a subsequent render.
430-
'render',
431-
'render',
432-
]
433-
: [
434-
'setState updater',
435-
// In the old reconciler, updates in the render phase receive
436-
// the currently rendering expiration time, so the update
437-
// flushes immediately in the same render.
438-
'render',
439-
],
440-
),
441-
),
418+
await waitForAll([
419+
'setState updater',
420+
// Updates in the render phase receive the currently rendering
421+
// lane, so the update flushes immediately in the same render.
422+
'render',
423+
]),
442424
).toErrorDev(
443425
'An update (setState, replaceState, or forceUpdate) was scheduled ' +
444426
'from inside an update function. Update functions should be pure, ' +
@@ -454,15 +436,9 @@ describe('ReactIncrementalUpdates', () => {
454436
});
455437
await waitForAll(
456438
gate(flags =>
457-
flags.deferRenderPhaseUpdateToNextBatch
458-
? // In the new reconciler, updates inside the render phase are
459-
// treated as if they came from an event, so the update gets shifted
460-
// to a subsequent render.
461-
['render', 'render']
462-
: // In the old reconciler, updates in the render phase receive
463-
// the currently rendering expiration time, so the update flushes
464-
// immediately in the same render.
465-
['render'],
439+
// Updates in the render phase receive the currently rendering
440+
// lane, so the update flushes immediately in the same render.
441+
['render'],
466442
),
467443
);
468444
});

‎packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,9 +264,7 @@ describe 'ReactCoffeeScriptClass', ->
264264
React.createElement('span', className: @state.bar)
265265

266266
test React.createElement(Foo, initialValue: 'foo'), 'SPAN', 'bar'
267-
# This is broken with deferRenderPhaseUpdateToNextBatch flag on.
268-
# We can't use the gate feature here because this test is also in CoffeeScript and TypeScript.
269-
expect(renderCount).toBe(if global.__WWW__ and !global.__VARIANT__ then 2 else 1)
267+
expect(renderCount).toBe(1)
270268

271269
it 'should warn with non-object in the initial state property', ->
272270
[['an array'], 'a string', 1234].forEach (state) ->

‎packages/react/src/__tests__/ReactES6Class-test.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,7 @@ describe('ReactES6Class', () => {
295295
}
296296
}
297297
test(<Foo initialValue="foo" />, 'SPAN', 'bar');
298-
// This is broken with deferRenderPhaseUpdateToNextBatch flag on.
299-
// We can't use the gate feature here because this test is also in CoffeeScript and TypeScript.
300-
expect(renderCount).toBe(global.__WWW__ && !global.__VARIANT__ ? 2 : 1);
298+
expect(renderCount).toBe(1);
301299
});
302300

303301
it('should warn with non-object in the initial state property', () => {

‎packages/react/src/__tests__/ReactTypeScriptClass-test.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,7 @@ describe('ReactTypeScriptClass', function() {
521521
it('renders only once when setting state in componentWillMount', function() {
522522
renderCount = 0;
523523
test(React.createElement(RenderOnce, {initialValue: 'foo'}), 'SPAN', 'bar');
524-
// This is broken with deferRenderPhaseUpdateToNextBatch flag on.
525-
// We can't use the gate feature in TypeScript.
526-
expect(renderCount).toBe(global.__WWW__ && !global.__VARIANT__ ? 2 : 1);
524+
expect(renderCount).toBe(1);
527525
});
528526

529527
it('should warn with non-object in the initial state property', function() {

‎packages/shared/ReactFeatureFlags.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -152,14 +152,6 @@ export const enableUnifiedSyncLane = __EXPERIMENTAL__;
152152
// startTransition. Only relevant when enableSyncDefaultUpdates is disabled.
153153
export const allowConcurrentByDefault = false;
154154

155-
// Updates that occur in the render phase are not officially supported. But when
156-
// they do occur, we defer them to a subsequent render by picking a lane that's
157-
// not currently rendering. We treat them the same as if they came from an
158-
// interleaved event. Remove this flag once we have migrated to the
159-
// new behavior.
160-
// NOTE: Not sure if we'll end up doing this or not.
161-
export const deferRenderPhaseUpdateToNextBatch = false;
162-
163155
// -----------------------------------------------------------------------------
164156
// React DOM Chopping Block
165157
//

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ export const enableLegacyFBSupport = false;
6060
export const enableFilterEmptyStringAttributesDOM = false;
6161
export const skipUnmountedBoundaries = false;
6262
export const enableGetInspectorDataForInstanceInProduction = true;
63-
export const deferRenderPhaseUpdateToNextBatch = false;
6463

6564
export const createRootStrictEffectsByDefault = false;
6665

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ export const enableLegacyFBSupport = false;
5050
export const enableFilterEmptyStringAttributesDOM = false;
5151
export const skipUnmountedBoundaries = false;
5252
export const enableGetInspectorDataForInstanceInProduction = false;
53-
export const deferRenderPhaseUpdateToNextBatch = false;
5453

5554
export const createRootStrictEffectsByDefault = false;
5655
export const enableUseRefAccessWarning = false;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ export const enableLegacyFBSupport = false;
5050
export const enableFilterEmptyStringAttributesDOM = false;
5151
export const skipUnmountedBoundaries = false;
5252
export const enableGetInspectorDataForInstanceInProduction = false;
53-
export const deferRenderPhaseUpdateToNextBatch = false;
5453

5554
export const createRootStrictEffectsByDefault = false;
5655
export const enableUseRefAccessWarning = false;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export const enableLegacyFBSupport = false;
4141
export const enableFilterEmptyStringAttributesDOM = false;
4242
export const skipUnmountedBoundaries = false;
4343
export const enableGetInspectorDataForInstanceInProduction = false;
44-
export const deferRenderPhaseUpdateToNextBatch = false;
4544
export const enableSuspenseAvoidThisFallback = false;
4645
export const enableSuspenseAvoidThisFallbackFizz = false;
4746
export const enableCPUSuspense = false;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ export const enableLegacyFBSupport = false;
5050
export const enableFilterEmptyStringAttributesDOM = true;
5151
export const skipUnmountedBoundaries = false;
5252
export const enableGetInspectorDataForInstanceInProduction = false;
53-
export const deferRenderPhaseUpdateToNextBatch = false;
5453

5554
export const createRootStrictEffectsByDefault = false;
5655
export const enableUseRefAccessWarning = false;

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,6 @@ export const enableDebugTracing = __EXPERIMENTAL__;
3535

3636
export const enableSchedulingProfiler = __VARIANT__;
3737

38-
// This only has an effect in the new reconciler. But also, the new reconciler
39-
// is only enabled when __VARIANT__ is true. So this is set to the opposite of
40-
// __VARIANT__ so that it's `false` when running against the new reconciler.
41-
// Ideally we would test both against the new reconciler, but until then, we
42-
// should test the value that is used in www. Which is `false`.
43-
//
44-
// Once Lanes has landed in both reconciler forks, we'll get coverage of
45-
// both branches.
46-
export const deferRenderPhaseUpdateToNextBatch = !__VARIANT__;
47-
4838
// These are already tested in both modes using the build type dimension,
4939
// so we don't need to use __VARIANT__ to get extra coverage.
5040
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ export const {
2121
revertRemovalOfSiblingPrerendering,
2222
replayFailedUnitOfWorkWithInvokeGuardedCallback,
2323
enableLegacyFBSupport,
24-
deferRenderPhaseUpdateToNextBatch,
2524
enableDebugTracing,
2625
skipUnmountedBoundaries,
2726
enableUseRefAccessWarning,

0 commit comments

Comments
 (0)