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/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/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..5e5567f9e0947 100644 --- a/packages/react-noop-renderer/src/ReactNoop.js +++ b/packages/react-noop-renderer/src/ReactNoop.js @@ -41,7 +41,6 @@ export const { unbatchedUpdates, discreteUpdates, idleUpdates, - flushDiscreteUpdates, flushSync, flushPassiveEffects, act, diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 8dbadafd2c22a..eb7e74193341c 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -915,8 +915,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..bf78e9e41c390 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -21,9 +21,9 @@ import { 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, @@ -59,9 +59,9 @@ import { 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, @@ -108,13 +108,13 @@ export const deferredUpdates = enableNewReconciler 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..c8b61182d5737 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -57,7 +57,7 @@ import { flushControlled, deferredUpdates, discreteUpdates, - flushDiscreteUpdates, + flushSyncWithoutWarningIfAlreadyRendering, flushPassiveEffects, } from './ReactFiberWorkLoop.new'; import { @@ -330,9 +330,9 @@ export { 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..7e74bf6a1feea 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -57,7 +57,7 @@ import { flushControlled, deferredUpdates, discreteUpdates, - flushDiscreteUpdates, + flushSyncWithoutWarningIfAlreadyRendering, flushPassiveEffects, } from './ReactFiberWorkLoop.old'; import { @@ -330,9 +330,9 @@ export { 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..c8e13fa3a5eb2 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -1044,34 +1044,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; @@ -1142,7 +1114,10 @@ export function unbatchedUpdates(fn: (a: A) => R, a: A): R { } } -export function flushSync(fn: A => R, a: A): R { +export function flushSyncWithoutWarningIfAlreadyRendering( + fn: A => R, + a: A, +): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; @@ -1165,18 +1140,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; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index f356fc3f1156a..c4f51481eaf98 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -1044,34 +1044,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; @@ -1142,7 +1114,10 @@ export function unbatchedUpdates(fn: (a: A) => R, a: A): R { } } -export function flushSync(fn: A => R, a: A): R { +export function flushSyncWithoutWarningIfAlreadyRendering( + fn: A => R, + a: A, +): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; @@ -1165,18 +1140,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; diff --git a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js index a3a74d739a9a3..c73e46ddc5e31 100644 --- a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js @@ -173,4 +173,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']); + }); });