diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index 8c90d593a7..79345cc146 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -1686,7 +1686,9 @@ describe('ReactDOMServerHooks', () => { , ); - if (gate(flags => flags.new)) { + if ( + gate(flags => flags.new && flags.deferRenderPhaseUpdateToNextBatch) + ) { expect(() => Scheduler.unstable_flushAll()).toErrorDev([ 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + 'Do not read the value directly.', @@ -1730,7 +1732,9 @@ describe('ReactDOMServerHooks', () => { , ); - if (gate(flags => flags.new)) { + if ( + gate(flags => flags.new && flags.deferRenderPhaseUpdateToNextBatch) + ) { expect(() => Scheduler.unstable_flushAll()).toErrorDev([ 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + 'Do not read the value directly.', diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index e2ff1032b8..3c6b2105d0 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -27,6 +27,7 @@ import { enableProfilerCommitHooks, enableSchedulerTracing, warnAboutUnmockedScheduler, + deferRenderPhaseUpdateToNextBatch, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -123,6 +124,7 @@ import { isSubsetOfLanes, mergeLanes, removeLanes, + pickArbitraryLane, hasDiscreteLanes, hasUpdatePriority, getNextLanes, @@ -354,6 +356,21 @@ export function requestUpdateLane( return getCurrentPriorityLevel() === ImmediateSchedulerPriority ? (SyncLane: Lane) : (SyncBatchedLane: Lane); + } else if ( + !deferRenderPhaseUpdateToNextBatch && + (executionContext & RenderContext) !== NoContext && + workInProgressRootRenderLanes !== NoLanes + ) { + // This is a render phase update. These are not officially supported. The + // old behavior is to give this the same "thread" (expiration time) as + // whatever is currently rendering. So if you call `setState` on a component + // that happens later in the same render, it will flush. Ideally, we want to + // remove the special case and treat them as if they came from an + // interleaved event. Regardless, this pattern is not officially supported. + // This behavior is only a fallback. The flag only exists until we can roll + // out the setState warnning, since existing code might accidentally rely on + // the current behavior. + return pickArbitraryLane(workInProgressRootRenderLanes); } // The algorithm for assigning an update to a lane should be stable for all @@ -542,11 +559,19 @@ function markUpdateLaneFromFiberToRoot( markRootUpdated(root, lane); if (workInProgressRoot === root) { // Received an update to a tree that's in the middle of rendering. Mark - // that there is unprocessed work on this root. - workInProgressRootUpdatedLanes = mergeLanes( - workInProgressRootUpdatedLanes, - lane, - ); + // 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 + ) { + workInProgressRootUpdatedLanes = mergeLanes( + workInProgressRootUpdatedLanes, + lane, + ); + } if (workInProgressRootExitStatus === RootSuspendedWithDelay) { // The root already suspended with a delay, which means this render // definitely won't finish. Since we have a new update, let's mark it as diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js index 90a327d734..055a2ee5c6 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js @@ -394,7 +394,7 @@ describe('ReactIncrementalUpdates', () => { expect(() => expect(Scheduler).toFlushAndYield( gate(flags => - flags.new + flags.new && flags.deferRenderPhaseUpdateToNextBatch ? [ 'setState updater', // In the new reconciler, updates inside the render phase are @@ -427,7 +427,7 @@ describe('ReactIncrementalUpdates', () => { }); expect(Scheduler).toFlushAndYield( gate(flags => - flags.new + flags.new && 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. diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 314b9a2447..b0e85d43d3 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -127,3 +127,11 @@ export const enableModernEventSystem = false; // Support legacy Primer support on internal FB www export const enableLegacyFBSupport = false; + +// Updates that occur in the render phase are not officially supported. But when +// they do occur, in the new reconciler, 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. In the old reconciler, we use whatever +// expiration time is currently rendering. Remove this flag once we have +// migrated to the new behavior. +export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 1d4198c3fb..978c5ff8cd 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -46,6 +46,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const enableNewReconciler = false; +export const deferRenderPhaseUpdateToNextBatch = true; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 6ba9fa00cf..9c00c127f2 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const enableNewReconciler = false; +export const deferRenderPhaseUpdateToNextBatch = true; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 21ed6623e1..93ff1a7fa0 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const enableNewReconciler = false; +export const deferRenderPhaseUpdateToNextBatch = true; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 5cd46d0cff..c84df882fb 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const enableNewReconciler = false; +export const deferRenderPhaseUpdateToNextBatch = true; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 41971ca576..2aa405ebe6 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const enableNewReconciler = false; +export const deferRenderPhaseUpdateToNextBatch = true; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 12c09a97e6..3c7ae77f8e 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = !__EXPERIMENTAL__; export const enableFilterEmptyStringAttributesDOM = false; export const enableNewReconciler = false; +export const deferRenderPhaseUpdateToNextBatch = true; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 766b4ebe7d..a047ec5d72 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -21,6 +21,16 @@ export const enableModernEventSystem = __VARIANT__; export const enableLegacyFBSupport = __VARIANT__; export const enableDebugTracing = !__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 debugRenderPhaseSideEffectsForStrictMode = __DEV__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index d21c7b7b69..cc982e8cd5 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -26,6 +26,7 @@ export const { enableFilterEmptyStringAttributesDOM, enableLegacyFBSupport, enableDebugTracing, + deferRenderPhaseUpdateToNextBatch, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.