diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap index b27231efdffc7..2ab8751e3cbcb 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap @@ -40,7 +40,7 @@ Object { 6 => 1, }, "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 16, "updaters": Array [ Object { @@ -87,7 +87,7 @@ Object { 4 => 2, }, "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 15, "updaters": Array [ Object { @@ -186,7 +186,7 @@ Object { 6 => 1, }, "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 12, "updaters": Array [ Object { @@ -445,7 +445,7 @@ Object { ], ], "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 12, "updaters": Array [ Object { @@ -938,7 +938,7 @@ Object { ], ], "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 11, "updaters": Array [ Object { @@ -1597,7 +1597,7 @@ Object { 17 => 1, }, "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 24, "updaters": Array [ Object { @@ -1687,7 +1687,7 @@ Object { "fiberActualDurations": Map {}, "fiberSelfDurations": Map {}, "passiveEffectDuration": 0, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 34, "updaters": Array [ Object { @@ -2223,7 +2223,7 @@ Object { ], ], "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 24, "updaters": Array [ Object { @@ -2310,7 +2310,7 @@ Object { "fiberActualDurations": Array [], "fiberSelfDurations": Array [], "passiveEffectDuration": 0, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 34, "updaters": Array [ Object { @@ -2431,7 +2431,7 @@ Object { 2 => 0, }, "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { @@ -2506,7 +2506,7 @@ Object { 3 => 0, }, "passiveEffectDuration": 0, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { @@ -2715,7 +2715,7 @@ Object { ], ], "passiveEffectDuration": 0, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { @@ -3071,7 +3071,7 @@ Object { 7 => 0, }, "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { @@ -3515,7 +3515,7 @@ Object { ], ], "passiveEffectDuration": null, - "priorityLevel": "Normal", + "priorityLevel": "Immediate", "timestamp": 0, "updaters": Array [ Object { diff --git a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js index 36c8367af5d48..ff1ea1771fc87 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js @@ -1154,19 +1154,13 @@ describe('ReactDOMFiber', () => { expect(ops).toEqual(['A']); if (__DEV__) { - const errorCalls = console.error.calls.count(); + expect(console.error.calls.count()).toBe(2); expect(console.error.calls.argsFor(0)[0]).toMatch( 'ReactDOM.render is no longer supported in React 18', ); expect(console.error.calls.argsFor(1)[0]).toMatch( 'ReactDOM.render is no longer supported in React 18', ); - // TODO: this warning shouldn't be firing in the first place if user didn't call it. - for (let i = 2; i < errorCalls; i++) { - expect(console.error.calls.argsFor(i)[0]).toMatch( - 'unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.', - ); - } } }); diff --git a/packages/react-dom/src/__tests__/ReactMount-test.js b/packages/react-dom/src/__tests__/ReactMount-test.js index a214c32f58d06..9571905edaf52 100644 --- a/packages/react-dom/src/__tests__/ReactMount-test.js +++ b/packages/react-dom/src/__tests__/ReactMount-test.js @@ -277,7 +277,7 @@ describe('ReactMount', () => { expect(calls).toBe(5); }); - it('initial mount is sync inside batchedUpdates, but task work is deferred until the end of the batch', () => { + it('initial mount of legacy root is sync inside batchedUpdates, as if it were wrapped in flushSync', () => { const container1 = document.createElement('div'); const container2 = document.createElement('div'); @@ -302,12 +302,12 @@ describe('ReactMount', () => { // Initial mount on another root. Should flush immediately. ReactDOM.render(a, container2); - // The update did not flush yet. - expect(container1.textContent).toEqual('1'); - // The initial mount flushed, but not the update scheduled in cDM. - expect(container2.textContent).toEqual('a'); + // The earlier update also flushed, since flushSync flushes all pending + // sync work across all roots. + expect(container1.textContent).toEqual('2'); + // Layout updates are also flushed synchronously + expect(container2.textContent).toEqual('a!'); }); - // All updates have flushed. expect(container1.textContent).toEqual('2'); expect(container2.textContent).toEqual('a!'); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 8e974697075f1..80fc3d1e4a05e 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -23,8 +23,8 @@ import {createEventHandle} from './ReactDOMEventHandle'; import { batchedUpdates, discreteUpdates, - flushDiscreteUpdates, flushSync, + flushSyncWithoutWarningIfAlreadyRendering, flushControlled, injectIntoDevTools, attemptSynchronousHydration, @@ -100,7 +100,7 @@ setRestoreImplementation(restoreControlledState); setBatchingImplementation( batchedUpdates, discreteUpdates, - flushDiscreteUpdates, + flushSyncWithoutWarningIfAlreadyRendering, ); function createPortal( diff --git a/packages/react-dom/src/client/ReactDOMLegacy.js b/packages/react-dom/src/client/ReactDOMLegacy.js index a62a7fb444c5b..f03350cb29240 100644 --- a/packages/react-dom/src/client/ReactDOMLegacy.js +++ b/packages/react-dom/src/client/ReactDOMLegacy.js @@ -29,7 +29,7 @@ import { createContainer, findHostInstanceWithNoPortals, updateContainer, - unbatchedUpdates, + flushSyncWithoutWarningIfAlreadyRendering, getPublicRootInstance, findHostInstance, findHostInstanceWithWarning, @@ -174,7 +174,7 @@ function legacyRenderSubtreeIntoContainer( }; } // Initial mount should not be batched. - unbatchedUpdates(() => { + flushSyncWithoutWarningIfAlreadyRendering(() => { updateContainer(children, fiberRoot, parentComponent, callback); }); } else { @@ -357,7 +357,7 @@ export function unmountComponentAtNode(container: Container) { } // Unmount should not be batched. - unbatchedUpdates(() => { + flushSyncWithoutWarningIfAlreadyRendering(() => { legacyRenderSubtreeIntoContainer(null, null, container, false, () => { // $FlowFixMe This should probably use `delete container._reactRootContainer` container._reactRootContainer = null; diff --git a/packages/react-dom/src/events/ReactDOMUpdateBatching.js b/packages/react-dom/src/events/ReactDOMUpdateBatching.js index f2fbfe4c3f329..d3c46630b148e 100644 --- a/packages/react-dom/src/events/ReactDOMUpdateBatching.js +++ b/packages/react-dom/src/events/ReactDOMUpdateBatching.js @@ -23,7 +23,7 @@ let batchedUpdatesImpl = function(fn, bookkeeping) { let discreteUpdatesImpl = function(fn, a, b, c, d) { return fn(a, b, c, d); }; -let flushDiscreteUpdatesImpl = function() {}; +let flushSyncImpl = function() {}; let isInsideEventHandler = false; @@ -39,7 +39,7 @@ function finishEventHandler() { // bails out of the update without touching the DOM. // TODO: Restore state in the microtask, after the discrete updates flush, // instead of early flushing them here. - flushDiscreteUpdatesImpl(); + flushSyncImpl(); restoreStateIfNeeded(); } } @@ -67,9 +67,9 @@ export function discreteUpdates(fn, a, b, c, d) { export function setBatchingImplementation( _batchedUpdatesImpl, _discreteUpdatesImpl, - _flushDiscreteUpdatesImpl, + _flushSyncImpl, ) { batchedUpdatesImpl = _batchedUpdatesImpl; discreteUpdatesImpl = _discreteUpdatesImpl; - flushDiscreteUpdatesImpl = _flushDiscreteUpdatesImpl; + flushSyncImpl = _flushSyncImpl; } diff --git a/packages/react-noop-renderer/src/ReactNoop.js b/packages/react-noop-renderer/src/ReactNoop.js index c09fa2d8000f5..a225deaf961f1 100644 --- a/packages/react-noop-renderer/src/ReactNoop.js +++ b/packages/react-noop-renderer/src/ReactNoop.js @@ -38,10 +38,8 @@ export const { flushExpired, batchedUpdates, deferredUpdates, - unbatchedUpdates, discreteUpdates, idleUpdates, - flushDiscreteUpdates, flushSync, flushPassiveEffects, act, diff --git a/packages/react-noop-renderer/src/ReactNoopPersistent.js b/packages/react-noop-renderer/src/ReactNoopPersistent.js index 97876990a9b57..86bb87c065e48 100644 --- a/packages/react-noop-renderer/src/ReactNoopPersistent.js +++ b/packages/react-noop-renderer/src/ReactNoopPersistent.js @@ -38,7 +38,6 @@ export const { flushExpired, batchedUpdates, deferredUpdates, - unbatchedUpdates, discreteUpdates, idleUpdates, flushDiscreteUpdates, diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 8dbadafd2c22a..5ea510d73d6ea 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -901,8 +901,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { deferredUpdates: NoopRenderer.deferredUpdates, - unbatchedUpdates: NoopRenderer.unbatchedUpdates, - discreteUpdates: NoopRenderer.discreteUpdates, idleUpdates(fn: () => T): T { @@ -915,8 +913,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { } }, - flushDiscreteUpdates: NoopRenderer.flushDiscreteUpdates, - flushSync(fn: () => mixed) { NoopRenderer.flushSync(fn); }, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index bc169fd8a6475..d25783164e984 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -18,12 +18,11 @@ import { createContainer as createContainer_old, updateContainer as updateContainer_old, batchedUpdates as batchedUpdates_old, - unbatchedUpdates as unbatchedUpdates_old, deferredUpdates as deferredUpdates_old, discreteUpdates as discreteUpdates_old, - flushDiscreteUpdates as flushDiscreteUpdates_old, flushControlled as flushControlled_old, flushSync as flushSync_old, + flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_old, flushPassiveEffects as flushPassiveEffects_old, getPublicRootInstance as getPublicRootInstance_old, attemptSynchronousHydration as attemptSynchronousHydration_old, @@ -56,12 +55,11 @@ import { createContainer as createContainer_new, updateContainer as updateContainer_new, batchedUpdates as batchedUpdates_new, - unbatchedUpdates as unbatchedUpdates_new, deferredUpdates as deferredUpdates_new, discreteUpdates as discreteUpdates_new, - flushDiscreteUpdates as flushDiscreteUpdates_new, flushControlled as flushControlled_new, flushSync as flushSync_new, + flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_new, flushPassiveEffects as flushPassiveEffects_new, getPublicRootInstance as getPublicRootInstance_new, attemptSynchronousHydration as attemptSynchronousHydration_new, @@ -99,22 +97,19 @@ export const updateContainer = enableNewReconciler export const batchedUpdates = enableNewReconciler ? batchedUpdates_new : batchedUpdates_old; -export const unbatchedUpdates = enableNewReconciler - ? unbatchedUpdates_new - : unbatchedUpdates_old; export const deferredUpdates = enableNewReconciler ? deferredUpdates_new : deferredUpdates_old; export const discreteUpdates = enableNewReconciler ? discreteUpdates_new : discreteUpdates_old; -export const flushDiscreteUpdates = enableNewReconciler - ? flushDiscreteUpdates_new - : flushDiscreteUpdates_old; export const flushControlled = enableNewReconciler ? flushControlled_new : flushControlled_old; export const flushSync = enableNewReconciler ? flushSync_new : flushSync_old; +export const flushSyncWithoutWarningIfAlreadyRendering = enableNewReconciler + ? flushSyncWithoutWarningIfAlreadyRendering_new + : flushSyncWithoutWarningIfAlreadyRendering_old; export const flushPassiveEffects = enableNewReconciler ? flushPassiveEffects_new : flushPassiveEffects_old; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index f406f8592b0d1..7ab995a5fcb02 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -52,12 +52,11 @@ import { scheduleUpdateOnFiber, flushRoot, batchedUpdates, - unbatchedUpdates, flushSync, flushControlled, deferredUpdates, discreteUpdates, - flushDiscreteUpdates, + flushSyncWithoutWarningIfAlreadyRendering, flushPassiveEffects, } from './ReactFiberWorkLoop.new'; import { @@ -327,12 +326,11 @@ export function updateContainer( export { batchedUpdates, - unbatchedUpdates, deferredUpdates, discreteUpdates, - flushDiscreteUpdates, flushControlled, flushSync, + flushSyncWithoutWarningIfAlreadyRendering, flushPassiveEffects, }; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index f6ae425b4400b..da127689c7c6e 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -52,12 +52,11 @@ import { scheduleUpdateOnFiber, flushRoot, batchedUpdates, - unbatchedUpdates, flushSync, flushControlled, deferredUpdates, discreteUpdates, - flushDiscreteUpdates, + flushSyncWithoutWarningIfAlreadyRendering, flushPassiveEffects, } from './ReactFiberWorkLoop.old'; import { @@ -327,12 +326,11 @@ export function updateContainer( export { batchedUpdates, - unbatchedUpdates, deferredUpdates, discreteUpdates, - flushDiscreteUpdates, flushControlled, flushSync, + flushSyncWithoutWarningIfAlreadyRendering, flushPassiveEffects, }; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 7f429c873438a..b6f697edfee38 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -246,12 +246,11 @@ const { type ExecutionContext = number; -export const NoContext = /* */ 0b00000; -const BatchedContext = /* */ 0b00001; -const LegacyUnbatchedContext = /* */ 0b00010; -const RenderContext = /* */ 0b00100; -const CommitContext = /* */ 0b01000; -export const RetryAfterError = /* */ 0b10000; +export const NoContext = /* */ 0b0000; +const BatchedContext = /* */ 0b0001; +const RenderContext = /* */ 0b0010; +const CommitContext = /* */ 0b0100; +export const RetryAfterError = /* */ 0b1000; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; const RootIncomplete = 0; @@ -515,35 +514,19 @@ export function scheduleUpdateOnFiber( } } - if (lane === SyncLane) { - if ( - // Check if we're inside unbatchedUpdates - (executionContext & LegacyUnbatchedContext) !== NoContext && - // Check if we're not already rendering - (executionContext & (RenderContext | CommitContext)) === NoContext - ) { - // This is a legacy edge case. The initial mount of a ReactDOM.render-ed - // root inside of batchedUpdates should be synchronous, but layout updates - // should be deferred until the end of the batch. - performSyncWorkOnRoot(root); - } else { - ensureRootIsScheduled(root, eventTime); - if ( - executionContext === NoContext && - (fiber.mode & ConcurrentMode) === NoMode - ) { - // Flush the synchronous work now, unless we're already working or inside - // a batch. This is intentionally inside scheduleUpdateOnFiber instead of - // scheduleCallbackForFiber to preserve the ability to schedule a callback - // without immediately flushing it. We only do this for user-initiated - // updates, to preserve historical behavior of legacy mode. - resetRenderTimer(); - flushSyncCallbacksOnlyInLegacyMode(); - } - } - } else { - // Schedule other updates after in case the callback is sync. - ensureRootIsScheduled(root, eventTime); + ensureRootIsScheduled(root, eventTime); + if ( + lane === SyncLane && + executionContext === NoContext && + (fiber.mode & ConcurrentMode) === NoMode + ) { + // Flush the synchronous work now, unless we're already working or inside + // a batch. This is intentionally inside scheduleUpdateOnFiber instead of + // scheduleCallbackForFiber to preserve the ability to schedule a callback + // without immediately flushing it. We only do this for user-initiated + // updates, to preserve historical behavior of legacy mode. + resetRenderTimer(); + flushSyncCallbacksOnlyInLegacyMode(); } return root; @@ -1044,34 +1027,6 @@ export function getExecutionContext(): ExecutionContext { return executionContext; } -export function flushDiscreteUpdates() { - // TODO: Should be able to flush inside batchedUpdates, but not inside `act`. - // However, `act` uses `batchedUpdates`, so there's no way to distinguish - // those two cases. Need to fix this before exposing flushDiscreteUpdates - // as a public API. - if ( - (executionContext & (BatchedContext | RenderContext | CommitContext)) !== - NoContext - ) { - if (__DEV__) { - if ((executionContext & RenderContext) !== NoContext) { - console.error( - 'unstable_flushDiscreteUpdates: Cannot flush updates when React is ' + - 'already rendering.', - ); - } - } - // We're already rendering, so we can't synchronously flush pending work. - // This is probably a nested event dispatch triggered by a lifecycle/effect, - // like `el.focus()`. Exit. - return; - } - flushSyncCallbacks(); - // If the discrete updates scheduled passive effects, flush them now so that - // they fire before the next serial event. - flushPassiveEffects(); -} - export function deferredUpdates(fn: () => A): A { const previousPriority = getCurrentUpdatePriority(); const prevTransition = ReactCurrentBatchConfig.transition; @@ -1123,26 +1078,19 @@ export function discreteUpdates( } } -export function unbatchedUpdates(fn: (a: A) => R, a: A): R { - const prevExecutionContext = executionContext; - executionContext &= ~BatchedContext; - executionContext |= LegacyUnbatchedContext; - try { - return fn(a); - } finally { - executionContext = prevExecutionContext; - // If there were legacy sync updates, flush them at the end of the outer - // most batchedUpdates-like method. - if (executionContext === NoContext) { - resetRenderTimer(); - // TODO: I think this call is redundant, because we flush inside - // scheduleUpdateOnFiber when LegacyUnbatchedContext is set. - flushSyncCallbacksOnlyInLegacyMode(); - } +export function flushSyncWithoutWarningIfAlreadyRendering( + fn: A => R, + a: A, +): R { + // In legacy mode, we flush pending passive effects at the beginning of the + // next event, not at the end of the previous one. + if ( + rootWithPendingPassiveEffects !== null && + rootWithPendingPassiveEffects.tag === LegacyRoot + ) { + flushPassiveEffects(); } -} -export function flushSync(fn: A => R, a: A): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; @@ -1165,18 +1113,23 @@ export function flushSync(fn: A => R, a: A): R { // the stack. if ((executionContext & (RenderContext | CommitContext)) === NoContext) { flushSyncCallbacks(); - } else { - if (__DEV__) { - console.error( - 'flushSync was called from inside a lifecycle method. React cannot ' + - 'flush when React is already rendering. Consider moving this call to ' + - 'a scheduler task or micro task.', - ); - } } } } +export function flushSync(fn: A => R, a: A): R { + if (__DEV__) { + if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { + console.error( + 'flushSync was called from inside a lifecycle method. React cannot ' + + 'flush when React is already rendering. Consider moving this call to ' + + 'a scheduler task or micro task.', + ); + } + } + return flushSyncWithoutWarningIfAlreadyRendering(fn, a); +} + export function flushControlled(fn: () => mixed): void { const prevExecutionContext = executionContext; executionContext |= BatchedContext; @@ -1974,24 +1927,6 @@ function commitRootImpl(root, renderPriorityLevel) { throw error; } - if ((executionContext & LegacyUnbatchedContext) !== NoContext) { - if (__DEV__) { - if (enableDebugTracing) { - logCommitStopped(); - } - } - - if (enableSchedulingProfiler) { - markCommitStopped(); - } - - // This is a legacy edge case. We just committed the initial mount of - // a ReactDOM.render-ed root inside of batchedUpdates. The commit fired - // synchronously, but layout updates should be deferred until the end - // of the batch. - return null; - } - // If the passive effects are the result of a discrete render, flush them // synchronously at the end of the current task so that the result is // immediately observable. Otherwise, we assume that they are not diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index f356fc3f1156a..22c8395cb0988 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -246,12 +246,11 @@ const { type ExecutionContext = number; -export const NoContext = /* */ 0b00000; -const BatchedContext = /* */ 0b00001; -const LegacyUnbatchedContext = /* */ 0b00010; -const RenderContext = /* */ 0b00100; -const CommitContext = /* */ 0b01000; -export const RetryAfterError = /* */ 0b10000; +export const NoContext = /* */ 0b0000; +const BatchedContext = /* */ 0b0001; +const RenderContext = /* */ 0b0010; +const CommitContext = /* */ 0b0100; +export const RetryAfterError = /* */ 0b1000; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; const RootIncomplete = 0; @@ -515,35 +514,19 @@ export function scheduleUpdateOnFiber( } } - if (lane === SyncLane) { - if ( - // Check if we're inside unbatchedUpdates - (executionContext & LegacyUnbatchedContext) !== NoContext && - // Check if we're not already rendering - (executionContext & (RenderContext | CommitContext)) === NoContext - ) { - // This is a legacy edge case. The initial mount of a ReactDOM.render-ed - // root inside of batchedUpdates should be synchronous, but layout updates - // should be deferred until the end of the batch. - performSyncWorkOnRoot(root); - } else { - ensureRootIsScheduled(root, eventTime); - if ( - executionContext === NoContext && - (fiber.mode & ConcurrentMode) === NoMode - ) { - // Flush the synchronous work now, unless we're already working or inside - // a batch. This is intentionally inside scheduleUpdateOnFiber instead of - // scheduleCallbackForFiber to preserve the ability to schedule a callback - // without immediately flushing it. We only do this for user-initiated - // updates, to preserve historical behavior of legacy mode. - resetRenderTimer(); - flushSyncCallbacksOnlyInLegacyMode(); - } - } - } else { - // Schedule other updates after in case the callback is sync. - ensureRootIsScheduled(root, eventTime); + ensureRootIsScheduled(root, eventTime); + if ( + lane === SyncLane && + executionContext === NoContext && + (fiber.mode & ConcurrentMode) === NoMode + ) { + // Flush the synchronous work now, unless we're already working or inside + // a batch. This is intentionally inside scheduleUpdateOnFiber instead of + // scheduleCallbackForFiber to preserve the ability to schedule a callback + // without immediately flushing it. We only do this for user-initiated + // updates, to preserve historical behavior of legacy mode. + resetRenderTimer(); + flushSyncCallbacksOnlyInLegacyMode(); } return root; @@ -1044,34 +1027,6 @@ export function getExecutionContext(): ExecutionContext { return executionContext; } -export function flushDiscreteUpdates() { - // TODO: Should be able to flush inside batchedUpdates, but not inside `act`. - // However, `act` uses `batchedUpdates`, so there's no way to distinguish - // those two cases. Need to fix this before exposing flushDiscreteUpdates - // as a public API. - if ( - (executionContext & (BatchedContext | RenderContext | CommitContext)) !== - NoContext - ) { - if (__DEV__) { - if ((executionContext & RenderContext) !== NoContext) { - console.error( - 'unstable_flushDiscreteUpdates: Cannot flush updates when React is ' + - 'already rendering.', - ); - } - } - // We're already rendering, so we can't synchronously flush pending work. - // This is probably a nested event dispatch triggered by a lifecycle/effect, - // like `el.focus()`. Exit. - return; - } - flushSyncCallbacks(); - // If the discrete updates scheduled passive effects, flush them now so that - // they fire before the next serial event. - flushPassiveEffects(); -} - export function deferredUpdates(fn: () => A): A { const previousPriority = getCurrentUpdatePriority(); const prevTransition = ReactCurrentBatchConfig.transition; @@ -1123,26 +1078,19 @@ export function discreteUpdates( } } -export function unbatchedUpdates(fn: (a: A) => R, a: A): R { - const prevExecutionContext = executionContext; - executionContext &= ~BatchedContext; - executionContext |= LegacyUnbatchedContext; - try { - return fn(a); - } finally { - executionContext = prevExecutionContext; - // If there were legacy sync updates, flush them at the end of the outer - // most batchedUpdates-like method. - if (executionContext === NoContext) { - resetRenderTimer(); - // TODO: I think this call is redundant, because we flush inside - // scheduleUpdateOnFiber when LegacyUnbatchedContext is set. - flushSyncCallbacksOnlyInLegacyMode(); - } +export function flushSyncWithoutWarningIfAlreadyRendering( + fn: A => R, + a: A, +): R { + // In legacy mode, we flush pending passive effects at the beginning of the + // next event, not at the end of the previous one. + if ( + rootWithPendingPassiveEffects !== null && + rootWithPendingPassiveEffects.tag === LegacyRoot + ) { + flushPassiveEffects(); } -} -export function flushSync(fn: A => R, a: A): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; @@ -1165,18 +1113,23 @@ export function flushSync(fn: A => R, a: A): R { // the stack. if ((executionContext & (RenderContext | CommitContext)) === NoContext) { flushSyncCallbacks(); - } else { - if (__DEV__) { - console.error( - 'flushSync was called from inside a lifecycle method. React cannot ' + - 'flush when React is already rendering. Consider moving this call to ' + - 'a scheduler task or micro task.', - ); - } } } } +export function flushSync(fn: A => R, a: A): R { + if (__DEV__) { + if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { + console.error( + 'flushSync was called from inside a lifecycle method. React cannot ' + + 'flush when React is already rendering. Consider moving this call to ' + + 'a scheduler task or micro task.', + ); + } + } + return flushSyncWithoutWarningIfAlreadyRendering(fn, a); +} + export function flushControlled(fn: () => mixed): void { const prevExecutionContext = executionContext; executionContext |= BatchedContext; @@ -1974,24 +1927,6 @@ function commitRootImpl(root, renderPriorityLevel) { throw error; } - if ((executionContext & LegacyUnbatchedContext) !== NoContext) { - if (__DEV__) { - if (enableDebugTracing) { - logCommitStopped(); - } - } - - if (enableSchedulingProfiler) { - markCommitStopped(); - } - - // This is a legacy edge case. We just committed the initial mount of - // a ReactDOM.render-ed root inside of batchedUpdates. The commit fired - // synchronously, but layout updates should be deferred until the end - // of the batch. - return null; - } - // If the passive effects are the result of a discrete render, flush them // synchronously at the end of the current task so that the result is // immediately observable. Otherwise, we assume that they are not diff --git a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js index a3a74d739a9a3..20d846f64b03e 100644 --- a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js @@ -128,7 +128,7 @@ describe('ReactFlushSync', () => { }); }); - test('do not flush passive effects synchronously in legacy mode', async () => { + test('do not flush passive effects synchronously after render in legacy mode', async () => { function App() { useEffect(() => { Scheduler.unstable_yieldValue('Effect'); @@ -152,6 +152,40 @@ describe('ReactFlushSync', () => { expect(Scheduler).toHaveYielded(['Effect']); }); + test('flush pending passive effects before scope is called in legacy mode', async () => { + let currentStep = 0; + + function App({step}) { + useEffect(() => { + currentStep = step; + Scheduler.unstable_yieldValue('Effect: ' + step); + }, [step]); + return ; + } + + const root = ReactNoop.createLegacyRoot(); + await act(async () => { + ReactNoop.flushSync(() => { + root.render(); + }); + expect(Scheduler).toHaveYielded([ + 1, + // Because we're in legacy mode, we shouldn't have flushed the passive + // effects yet. + ]); + expect(root).toMatchRenderedOutput('1'); + + ReactNoop.flushSync(() => { + // This should render step 2 because the passive effect has already + // fired, before the scope function is called. + root.render(); + }); + expect(Scheduler).toHaveYielded(['Effect: 1', 2]); + expect(root).toMatchRenderedOutput('2'); + }); + expect(Scheduler).toHaveYielded(['Effect: 2']); + }); + test("do not flush passive effects synchronously when they aren't the result of a sync render", async () => { function App() { useEffect(() => { @@ -173,4 +207,27 @@ describe('ReactFlushSync', () => { // Effect flushes after paint. expect(Scheduler).toHaveYielded(['Effect']); }); + + test('does not flush pending passive effects', async () => { + function App() { + useEffect(() => { + Scheduler.unstable_yieldValue('Effect'); + }, []); + return ; + } + + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + expect(Scheduler).toFlushUntilNextPaint(['Child']); + expect(root).toMatchRenderedOutput('Child'); + + // Passive effects are pending. Calling flushSync should not affect them. + ReactNoop.flushSync(); + // Effects still haven't fired. + expect(Scheduler).toHaveYielded([]); + }); + // Now the effects have fired. + expect(Scheduler).toHaveYielded(['Effect']); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index fc5e61cd78ee6..7de5232934486 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -1795,10 +1795,11 @@ describe('ReactHooksWithNoopRenderer', () => { return ; } await act(async () => { - ReactNoop.renderLegacySyncRoot(); + ReactNoop.flushSync(() => { + ReactNoop.renderLegacySyncRoot(); + }); // Even in legacy mode, effects are deferred until after paint - ReactNoop.flushSync(); expect(Scheduler).toHaveYielded(['Count: (empty)']); expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js index b603b03c696f5..9c94900851475 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js @@ -349,63 +349,4 @@ describe('ReactIncrementalScheduling', () => { // The updates should all be flushed with Task priority expect(ReactNoop).toMatchRenderedOutput(); }); - - it('can opt-out of batching using unbatchedUpdates', () => { - ReactNoop.flushSync(() => { - ReactNoop.render(); - expect(ReactNoop.getChildren()).toEqual([]); - // Should not have flushed yet because we're still batching - - // unbatchedUpdates reverses the effect of batchedUpdates, so sync - // updates are not batched - ReactNoop.unbatchedUpdates(() => { - ReactNoop.render(); - expect(ReactNoop).toMatchRenderedOutput(); - ReactNoop.render(); - expect(ReactNoop).toMatchRenderedOutput(); - }); - - ReactNoop.render(); - expect(ReactNoop).toMatchRenderedOutput(); - }); - // Remaining update is now flushed - expect(ReactNoop).toMatchRenderedOutput(); - }); - - it('nested updates are always deferred, even inside unbatchedUpdates', () => { - let instance; - class Foo extends React.Component { - state = {step: 0}; - componentDidUpdate() { - Scheduler.unstable_yieldValue('componentDidUpdate: ' + this.state.step); - if (this.state.step === 1) { - ReactNoop.unbatchedUpdates(() => { - // This is a nested state update, so it should not be - // flushed synchronously, even though we wrapped it - // in unbatchedUpdates. - this.setState({step: 2}); - }); - expect(Scheduler).toHaveYielded([ - 'render: 1', - 'componentDidUpdate: 1', - ]); - expect(ReactNoop).toMatchRenderedOutput(); - } - } - render() { - Scheduler.unstable_yieldValue('render: ' + this.state.step); - instance = this; - return ; - } - } - ReactNoop.render(); - expect(Scheduler).toFlushAndYield(['render: 0']); - expect(ReactNoop).toMatchRenderedOutput(); - - ReactNoop.flushSync(() => { - instance.setState({step: 1}); - }); - expect(Scheduler).toHaveYielded(['render: 2', 'componentDidUpdate: 2']); - expect(ReactNoop).toMatchRenderedOutput(); - }); });