diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index 359c61a2fada8..b2249909dff45 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -14,7 +14,6 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils'); let React; -let ReactFeatureFlags; let ReactDOM; let ReactDOMServer; let ReactTestUtils; @@ -39,9 +38,6 @@ function initModules() { // Reset warning cache. jest.resetModuleRegistry(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - - ReactFeatureFlags.flushSuspenseFallbacksInTests = false; React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 5080609ea1854..48d8a259561d8 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -26,7 +26,6 @@ import { enableProfilerCommitHooks, enableSchedulerTracing, warnAboutUnmockedScheduler, - flushSuspenseFallbacksInTests, disableSchedulerTimeoutBasedOnReactExpirationTime, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -742,11 +741,7 @@ function finishConcurrentRender( if ( hasNotProcessedNewUpdates && // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) + !(__DEV__ && isFlushingAct) ) { // If we have not processed any new updates during this pass, then // this is either a retry of an existing fallback state or a @@ -805,11 +800,7 @@ function finishConcurrentRender( if ( // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) + !(__DEV__ && isFlushingAct) ) { // We're suspended in a state that should be avoided. We'll try to // avoid committing it for as long as the timeouts let us. @@ -881,11 +872,7 @@ function finishConcurrentRender( // The work completed. Ready to commit. if ( // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) && + !(__DEV__ && isFlushingAct) && workInProgressRootLatestProcessedEventTime !== Sync && workInProgressRootCanSuspendUsingConfig !== null ) { @@ -3206,24 +3193,45 @@ function finishPendingInteractions(root, committedExpirationTime) { // TODO: This is mostly a copy-paste from the legacy `act`, which does not have // access to the same internals that we do here. Some trade offs in the // implementation no longer make sense. -const isSchedulerMocked = - typeof Scheduler.unstable_flushAllWithoutAsserting === 'function'; -const flushSchedulerWork = - Scheduler.unstable_flushAllWithoutAsserting || - function() { - let didFlushWork = false; - while (flushPassiveEffects()) { - didFlushWork = true; - } - return didFlushWork; - }; +let isFlushingAct = false; + +const flushMockScheduler = Scheduler.unstable_flushAllWithoutAsserting; +const isSchedulerMocked = typeof flushMockScheduler === 'function'; + +// Returns whether additional work was scheduled. Caller should keep flushing +// until there's no work left. +function flushActWork(): boolean { + if (flushMockScheduler !== undefined) { + const prevIsFlushing = isFlushingAct; + isFlushingAct = true; + try { + return flushMockScheduler(); + } finally { + isFlushingAct = prevIsFlushing; + } + } else { + // No mock scheduler available. However, the only type of pending work is + // passive effects, which we control. So we can flush that. + const prevIsFlushing = isFlushingAct; + isFlushingAct = true; + try { + let didFlushWork = false; + while (flushPassiveEffects()) { + didFlushWork = true; + } + return didFlushWork; + } finally { + isFlushingAct = prevIsFlushing; + } + } +} function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { try { - flushSchedulerWork(); + flushActWork(); enqueueTask(() => { - if (flushSchedulerWork()) { + if (flushActWork()) { flushWorkAndMicroTasks(onDone); } else { onDone(); @@ -3362,7 +3370,7 @@ export function act(callback: () => Thenable): Thenable { ) { // we're about to exit the act() scope, // now's the time to flush effects - flushSchedulerWork(); + flushActWork(); } onDone(); } catch (err) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index dc52a174c0215..8ea328dfb621f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -26,7 +26,6 @@ import { enableProfilerCommitHooks, enableSchedulerTracing, warnAboutUnmockedScheduler, - flushSuspenseFallbacksInTests, disableSchedulerTimeoutBasedOnReactExpirationTime, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -735,11 +734,7 @@ function finishConcurrentRender( if ( hasNotProcessedNewUpdates && // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) + !(__DEV__ && isFlushingAct) ) { // If we have not processed any new updates during this pass, then // this is either a retry of an existing fallback state or a @@ -798,11 +793,7 @@ function finishConcurrentRender( if ( // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) + !(__DEV__ && isFlushingAct) ) { // We're suspended in a state that should be avoided. We'll try to // avoid committing it for as long as the timeouts let us. @@ -889,11 +880,7 @@ function finishConcurrentRender( // The work completed. Ready to commit. if ( // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) && + !(__DEV__ && isFlushingAct) && workInProgressRootLatestProcessedExpirationTime !== Sync && workInProgressRootCanSuspendUsingConfig !== null ) { @@ -3228,24 +3215,45 @@ function finishPendingInteractions(root, committedExpirationTime) { // TODO: This is mostly a copy-paste from the legacy `act`, which does not have // access to the same internals that we do here. Some trade offs in the // implementation no longer make sense. -const isSchedulerMocked = - typeof Scheduler.unstable_flushAllWithoutAsserting === 'function'; -const flushSchedulerWork = - Scheduler.unstable_flushAllWithoutAsserting || - function() { - let didFlushWork = false; - while (flushPassiveEffects()) { - didFlushWork = true; - } - return didFlushWork; - }; +let isFlushingAct = false; + +const flushMockScheduler = Scheduler.unstable_flushAllWithoutAsserting; +const isSchedulerMocked = typeof flushMockScheduler === 'function'; + +// Returns whether additional work was scheduled. Caller should keep flushing +// until there's no work left. +function flushActWork(): boolean { + if (flushMockScheduler !== undefined) { + const prevIsFlushing = isFlushingAct; + isFlushingAct = true; + try { + return flushMockScheduler(); + } finally { + isFlushingAct = prevIsFlushing; + } + } else { + // No mock scheduler available. However, the only type of pending work is + // passive effects, which we control. So we can flush that. + const prevIsFlushing = isFlushingAct; + isFlushingAct = true; + try { + let didFlushWork = false; + while (flushPassiveEffects()) { + didFlushWork = true; + } + return didFlushWork; + } finally { + isFlushingAct = prevIsFlushing; + } + } +} function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { try { - flushSchedulerWork(); + flushActWork(); enqueueTask(() => { - if (flushSchedulerWork()) { + if (flushActWork()) { flushWorkAndMicroTasks(onDone); } else { onDone(); @@ -3384,7 +3392,7 @@ export function act(callback: () => Thenable): Thenable { ) { // we're about to exit the act() scope, // now's the time to flush effects - flushSchedulerWork(); + flushActWork(); } onDone(); } catch (err) { diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 84ddb0c0dc96f..4e3a5259f4e1c 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -44,7 +44,6 @@ describe('ReactHooksWithNoopRenderer', () => { ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.enableSchedulerTracing = true; - ReactFeatureFlags.flushSuspenseFallbacksInTests = false; ReactFeatureFlags.enableProfilerTimer = true; deferPassiveEffectCleanupDuringUnmount = ReactFeatureFlags.deferPassiveEffectCleanupDuringUnmount; @@ -659,7 +658,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span(22)]); }); - it('discards render phase updates if something suspends', () => { + it('discards render phase updates if something suspends', async () => { const thenable = {then() {}}; function Foo({signal}) { return ( @@ -747,19 +746,20 @@ describe('ReactHooksWithNoopRenderer', () => { await ReactNoop.act(async () => { root.render(); setLabel('B'); - }); - expect(Scheduler).toHaveYielded(['Suspend!']); - expect(root).toMatchRenderedOutput(); - // Rendering again should suspend again. - root.render(); - expect(Scheduler).toFlushAndYield(['Suspend!']); + expect(Scheduler).toFlushAndYield(['Suspend!']); + expect(root).toMatchRenderedOutput(); - // Flip the signal back to "cancel" the update. However, the update to - // label should still proceed. It shouldn't have been dropped. - root.render(); - expect(Scheduler).toFlushAndYield(['B:0']); - expect(root).toMatchRenderedOutput(); + // Rendering again should suspend again. + root.render(); + expect(Scheduler).toFlushAndYield(['Suspend!']); + + // Flip the signal back to "cancel" the update. However, the update to + // label should still proceed. It shouldn't have been dropped. + root.render(); + expect(Scheduler).toFlushAndYield(['B:0']); + expect(root).toMatchRenderedOutput(); + }); }); it('regression: render phase updates cause lower pri work to be dropped', async () => { @@ -2755,39 +2755,40 @@ describe('ReactHooksWithNoopRenderer', () => { span('Before... Pending: false'), ]); - act(() => { + await act(async () => { Scheduler.unstable_runWithPriority( Scheduler.unstable_UserBlockingPriority, transition, ); - }); - Scheduler.unstable_advanceTime(500); - await advanceTimers(500); - expect(Scheduler).toHaveYielded([ - 'Before... Pending: true', - 'Suspend! [After... Pending: false]', - 'Loading... Pending: false', - ]); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: true'), - ]); - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); - expect(ReactNoop.getChildren()).toEqual([ - hiddenSpan('Before... Pending: true'), - span('Loading... Pending: false'), - ]); + expect(Scheduler).toFlushAndYield([ + 'Before... Pending: true', + 'Suspend! [After... Pending: false]', + 'Loading... Pending: false', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('Before... Pending: true'), + ]); + Scheduler.unstable_advanceTime(500); + await advanceTimers(500); - Scheduler.unstable_advanceTime(500); - await advanceTimers(500); - expect(Scheduler).toHaveYielded([ - 'Promise resolved [After... Pending: false]', - ]); - expect(Scheduler).toFlushAndYield(['After... Pending: false']); - expect(ReactNoop.getChildren()).toEqual([ - span('After... Pending: false'), - ]); + Scheduler.unstable_advanceTime(1000); + await advanceTimers(1000); + expect(ReactNoop.getChildren()).toEqual([ + hiddenSpan('Before... Pending: true'), + span('Loading... Pending: false'), + ]); + + Scheduler.unstable_advanceTime(500); + await advanceTimers(500); + expect(Scheduler).toHaveYielded([ + 'Promise resolved [After... Pending: false]', + ]); + expect(Scheduler).toFlushAndYield(['After... Pending: false']); + expect(ReactNoop.getChildren()).toEqual([ + span('After... Pending: false'), + ]); + }); }); // @gate experimental it('delays showing loading state until after busyDelayMs + busyMinDurationMs', async () => { @@ -2820,51 +2821,54 @@ describe('ReactHooksWithNoopRenderer', () => { span('Before... Pending: false'), ]); - act(() => { + await act(async () => { Scheduler.unstable_runWithPriority( Scheduler.unstable_UserBlockingPriority, transition, ); - }); - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); - expect(Scheduler).toHaveYielded([ - 'Before... Pending: true', - 'Suspend! [After... Pending: false]', - 'Loading... Pending: false', - ]); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: true'), - ]); - // Resolve the promise. The whole tree has now completed. However, - // because we exceeded the busy threshold, we won't commit the - // result yet. - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); - expect(Scheduler).toHaveYielded([ - 'Promise resolved [After... Pending: false]', - ]); - expect(Scheduler).toFlushAndYield(['After... Pending: false']); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: true'), - ]); + expect(Scheduler).toFlushAndYield([ + 'Before... Pending: true', + 'Suspend! [After... Pending: false]', + 'Loading... Pending: false', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('Before... Pending: true'), + ]); - // Advance time until just before the `busyMinDuration` threshold. - Scheduler.unstable_advanceTime(999); - await advanceTimers(999); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: true'), - ]); + Scheduler.unstable_advanceTime(1000); + await advanceTimers(1000); - // Advance time just a bit more. Now we complete the transition. - Scheduler.unstable_advanceTime(300); - await advanceTimers(300); - expect(ReactNoop.getChildren()).toEqual([ - span('After... Pending: false'), - ]); + // Resolve the promise. The whole tree has now completed. However, + // because we exceeded the busy threshold, we won't commit the + // result yet. + Scheduler.unstable_advanceTime(1000); + await advanceTimers(1000); + expect(Scheduler).toHaveYielded([ + 'Promise resolved [After... Pending: false]', + ]); + expect(Scheduler).toFlushAndYield(['After... Pending: false']); + expect(ReactNoop.getChildren()).toEqual([ + span('Before... Pending: true'), + ]); + + // Advance time until just before the `busyMinDuration` threshold. + Scheduler.unstable_advanceTime(999); + await advanceTimers(999); + expect(ReactNoop.getChildren()).toEqual([ + span('Before... Pending: true'), + ]); + + // Advance time just a bit more. Now we complete the transition. + Scheduler.unstable_advanceTime(300); + await advanceTimers(300); + expect(ReactNoop.getChildren()).toEqual([ + span('After... Pending: false'), + ]); + }); }); }); + describe('useDeferredValue', () => { // @gate experimental it('defers text value until specified timeout', async () => { @@ -2902,39 +2906,42 @@ describe('ReactHooksWithNoopRenderer', () => { expect(Scheduler).toFlushAndYield(['A']); expect(ReactNoop.getChildren()).toEqual([span('A'), span('A')]); - act(() => { + await act(async () => { _setText('B'); + expect(Scheduler).toFlushAndYield([ + 'B', + 'A', + 'B', + 'Suspend! [B]', + 'Loading', + ]); + expect(Scheduler).toFlushAndYield([]); + expect(ReactNoop.getChildren()).toEqual([span('B'), span('A')]); }); - expect(Scheduler).toHaveYielded([ - 'B', - 'A', - 'B', - 'Suspend! [B]', - 'Loading', - ]); - expect(Scheduler).toFlushAndYield([]); - expect(ReactNoop.getChildren()).toEqual([span('B'), span('A')]); - Scheduler.unstable_advanceTime(250); - await advanceTimers(250); - expect(Scheduler).toFlushAndYield([]); + await act(async () => { + Scheduler.unstable_advanceTime(250); + await advanceTimers(250); + }); + expect(Scheduler).toHaveYielded([]); expect(ReactNoop.getChildren()).toEqual([span('B'), span('A')]); - Scheduler.unstable_advanceTime(500); - await advanceTimers(500); + await act(async () => { + Scheduler.unstable_advanceTime(500); + await advanceTimers(500); + }); + expect(Scheduler).toHaveYielded([]); expect(ReactNoop.getChildren()).toEqual([ span('B'), hiddenSpan('A'), span('Loading'), ]); - Scheduler.unstable_advanceTime(250); - await advanceTimers(250); - expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - - act(() => { - expect(Scheduler).toFlushAndYield(['B']); + await act(async () => { + Scheduler.unstable_advanceTime(250); + await advanceTimers(250); }); + expect(Scheduler).toHaveYielded(['Promise resolved [B]', 'B']); expect(ReactNoop.getChildren()).toEqual([span('B'), span('B')]); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index b9520351c44c9..280e2bd715c7a 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -17,7 +17,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; - ReactFeatureFlags.flushSuspenseFallbacksInTests = false; React = require('react'); Fragment = React.Fragment; ReactNoop = require('react-noop-renderer'); @@ -3069,25 +3068,26 @@ describe('ReactSuspenseWithNoopRenderer', () => { setText('C'); }, ); - }); - expect(Scheduler).toHaveYielded([ - // First we attempt the high pri update. It suspends. - 'Suspend! [B]', - 'Loading...', - ]); - // Commit the placeholder to unblock the Idle update. - await advanceTimers(250); - expect(root).toMatchRenderedOutput( - <> -