From 3a12db4adb34f57410cded6fea8a6844ae4f8311 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 13 Apr 2020 18:02:19 -0700 Subject: [PATCH] Delete `flushSuspenseFallbacksInTests` This was meant to be a temporary hack to unblock the `act` work, but it quickly spread throughout our tests. What it's meant to do is force fallbacks to flush inside `act` even in Concurrent Mode. It does this by wrapping the `setTimeout` call in a check to see if it's in an `act` context. If so, it skips the delay and immediately commits the fallback. Really this is only meant for our internal React tests that need to incrementally render. Nobody outside our team (and Relay) needs to do that, yet. Even if/when we do support that, it may or may not be with the same `flushAndYield` pattern we use internally. However, even for our internal purposes, the behavior isn't right because a really common reason we flush work incrementally is to make assertions on the "suspended" state, before the fallback has committed. There's no way to do that from inside `act` with the behavior of this flag, because it causes the fallback to immediately commit. This has led us to *not* use `act` in a lot of our tests, or to write code that doesn't match what would actually happen in a real environment. What we really want is for the fallbacks to be flushed at the *end` of the `act` scope. Not within it. This only affects the noop and test renderer versions of `act`, which are implemented inside the reconciler. Whereas `ReactTestUtils.act` is implemented in "userspace" for backwards compatibility. This is fine because we didn't have any DOM Suspense tests that relied on this flag; they all use test renderer or noop. In the future, we'll probably want to move always use the reconciler implementation of `act`. It will not affect the prod bundle, because we currently only plan to support `act` in dev. Though we still haven't completely figured that out. However, regardless of whether we support a production `act` for users, we'll still need to write internal React tests in production mode. For that use case, we'll likely add our own internal version of `act` that assumes a mock Scheduler and might rely on hacks that don't 100% align up with the public one. --- ...DOMServerIntegrationHooks-test.internal.js | 31 ++- .../src/ReactFiberWorkLoop.new.js | 88 ++++--- .../src/ReactFiberWorkLoop.old.js | 88 ++++--- ...eactHooksWithNoopRenderer-test.internal.js | 201 +++++++-------- .../ReactSuspenseList-test.internal.js | 44 ++-- ...tSuspenseWithNoopRenderer-test.internal.js | 228 +++++++++--------- .../ReactTransition-test.internal.js | 6 - .../useMutableSource-test.internal.js | 1 - packages/shared/ReactFeatureFlags.js | 4 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.testing.js | 1 - .../forks/ReactFeatureFlags.testing.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 2 - 16 files changed, 369 insertions(+), 330 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index 359c61a2fada8..9664ea1cacb78 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'); @@ -1281,13 +1277,26 @@ describe('ReactDOMServerHooks', () => { // State update should trigger the ID to update, which changes the props // of ChildWithID. This should cause ChildWithID to hydrate before Children - expect(Scheduler).toFlushAndYieldThrough([ - 'Child with ID', - 'Child with ID', - 'Child with ID', - 'Child One', - 'Child Two', - ]); + + expect(Scheduler).toFlushAndYieldThrough( + __DEV__ + ? [ + 'Child with ID', + // Fallbacks are immdiately committed in TestUtils version + // of act + // 'Child with ID', + // 'Child with ID', + 'Child One', + 'Child Two', + ] + : [ + 'Child with ID', + 'Child with ID', + 'Child with ID', + 'Child One', + 'Child Two', + ], + ); expect(child1Ref.current).toBe(null); expect(childWithIDRef.current).toEqual( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 5080609ea1854..e3faf2b44fd06 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 - ) + !shouldForceFlushFallbacksInDEV() ) { // 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 - ) + !shouldForceFlushFallbacksInDEV() ) { // 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 - ) && + !shouldForceFlushFallbacksInDEV() && workInProgressRootLatestProcessedEventTime !== Sync && workInProgressRootCanSuspendUsingConfig !== null ) { @@ -3206,24 +3193,62 @@ 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; +let isInsideThisAct = false; + +// TODO: Yes, this is confusing. See above comment. We'll refactor it. +function shouldForceFlushFallbacksInDEV() { + if (!__DEV__) { + // Never force flush in production. This function should get stripped out. + return false; + } + // `IsThisRendererActing.current` is used by ReactTestUtils version of `act`. + if (IsThisRendererActing.current) { + // `isInsideAct` is only used by the reconciler implementation of `act`. + // We don't want to flush suspense fallbacks until the end. + return !isInsideThisAct; + } + // Flush callbacks at the end. + return isFlushingAct; +} + +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(); @@ -3256,13 +3281,16 @@ export function act(callback: () => Thenable): Thenable { const previousIsSomeRendererActing = IsSomeRendererActing.current; const previousIsThisRendererActing = IsThisRendererActing.current; + const previousIsInsideThisAct = isInsideThisAct; IsSomeRendererActing.current = true; IsThisRendererActing.current = true; + isInsideThisAct = true; function onDone() { actingUpdatesScopeDepth--; IsSomeRendererActing.current = previousIsSomeRendererActing; IsThisRendererActing.current = previousIsThisRendererActing; + isInsideThisAct = previousIsInsideThisAct; if (__DEV__) { if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) { // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned @@ -3362,7 +3390,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..30d5c828fc9e7 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 - ) + !shouldForceFlushFallbacksInDEV() ) { // 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 - ) + !shouldForceFlushFallbacksInDEV() ) { // 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 - ) && + !shouldForceFlushFallbacksInDEV() && workInProgressRootLatestProcessedExpirationTime !== Sync && workInProgressRootCanSuspendUsingConfig !== null ) { @@ -3228,24 +3215,62 @@ 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; +let isInsideThisAct = false; + +// TODO: Yes, this is confusing. See above comment. We'll refactor it. +function shouldForceFlushFallbacksInDEV() { + if (!__DEV__) { + // Never force flush in production. This function should get stripped out. + return false; + } + // `IsThisRendererActing.current` is used by ReactTestUtils version of `act`. + if (IsThisRendererActing.current) { + // `isInsideAct` is only used by the reconciler implementation of `act`. + // We don't want to flush suspense fallbacks until the end. + return !isInsideThisAct; + } + // Flush callbacks at the end. + return isFlushingAct; +} + +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(); @@ -3278,13 +3303,16 @@ export function act(callback: () => Thenable): Thenable { const previousIsSomeRendererActing = IsSomeRendererActing.current; const previousIsThisRendererActing = IsThisRendererActing.current; + const previousIsInsideThisAct = isInsideThisAct; IsSomeRendererActing.current = true; IsThisRendererActing.current = true; + isInsideThisAct = true; function onDone() { actingUpdatesScopeDepth--; IsSomeRendererActing.current = previousIsSomeRendererActing; IsThisRendererActing.current = previousIsThisRendererActing; + isInsideThisAct = previousIsInsideThisAct; if (__DEV__) { if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) { // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned @@ -3384,7 +3412,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__/ReactSuspenseList-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js index 413f9a72e6db3..db9cb1c5396ba 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js @@ -2469,35 +2469,21 @@ describe('ReactSuspenseList', () => { jest.runAllTimers(); - expect(Scheduler).toHaveYielded( - __DEV__ - ? [ - // First attempt at high pri. - 'Suspend! [A]', - 'Loading A', - // Re-render at forced. - 'Suspend! [A]', - 'Loading A', - // We auto-commit this on DEV. - // Try again on low-pri. - 'Suspend! [A]', - 'Loading A', - ] - : [ - // First attempt at high pri. - 'Suspend! [A]', - 'Loading A', - // Re-render at forced. - 'Suspend! [A]', - 'Loading A', - // We didn't commit so retry at low-pri. - 'Suspend! [A]', - 'Loading A', - // Re-render at forced. - 'Suspend! [A]', - 'Loading A', - ], - ); + expect(Scheduler).toHaveYielded([ + // First attempt at high pri. + 'Suspend! [A]', + 'Loading A', + // Re-render at forced. + 'Suspend! [A]', + 'Loading A', + // We auto-commit this on DEV. + // Try again on low-pri. + 'Suspend! [A]', + 'Loading A', + // Re-render at forced. + 'Suspend! [A]', + 'Loading A', + ]); expect(ReactNoop).toMatchRenderedOutput(Loading A); 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( - <> -