From 3400ee60eb491bd4fed6b9385e33e6b2cb4a3c83 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 21 Jun 2021 22:44:32 -0400 Subject: [PATCH] `act` should work without mock Scheduler Currently, in a React 18 root, `act` only works if you mock the Scheduler package. This was because we didn't want to add additional checks at runtime. But now that the `act` testing API is dev-only, we can simplify its implementation. Now when an update is wrapped with `act`, React will bypass Scheduler entirely and push its tasks onto a special internal queue. Then, when the outermost `act` scope exists, we'll flush that queue. I also removed the "wrong act" warning, because the plan is to move `act` to an isomorphic entry point, simlar to `startTransition`. That's not directly related to this PR, but I didn't want to bother re-implementing that warning only to immediately remove it. I'll add the isomorphic API in a follow up. Note that the internal version of `act` that we use in our own tests still depends on mocking the Scheduler package, because it needs to work in production. I'm planning to move that implementation to a shared (internal) module, too. --- .../src/__tests__/ReactTestUtilsAct-test.js | 67 +-- ...tUnmockedSchedulerWarning-test.internal.js | 56 --- .../ReactUnmockedSchedulerWarning-test.js | 42 -- .../test-utils/ReactTestUtilsInternalAct.js | 10 +- .../src/createReactNoop.js | 10 +- .../src/ReactFiberHooks.new.js | 2 - .../src/ReactFiberHooks.old.js | 2 - .../src/ReactFiberReconciler.new.js | 9 - .../src/ReactFiberReconciler.old.js | 9 - .../src/ReactFiberWorkLoop.new.js | 457 ++++++++---------- .../src/ReactFiberWorkLoop.old.js | 457 ++++++++---------- .../src/ReactTestRenderer.js | 10 +- .../__tests__/ReactTestRendererAct-test.js | 17 - packages/react/src/ReactCurrentActQueue.js | 22 + packages/react/src/ReactSharedInternals.js | 2 + packages/shared/ReactFeatureFlags.js | 1 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.native.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 - scripts/jest/setupTests.www.js | 1 - 25 files changed, 470 insertions(+), 713 deletions(-) delete mode 100644 packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.internal.js delete mode 100644 packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.js create mode 100644 packages/react/src/ReactCurrentActQueue.js diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js index 2dabcfc5281b7..0e10207b0bb04 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js @@ -354,32 +354,6 @@ function runActTests(label, render, unmount, rerender) { expect(container.innerHTML).toBe('2'); }); }); - - // @gate __DEV__ - it('warns if you return a value inside act', () => { - expect(() => act(() => null)).toErrorDev( - [ - 'The callback passed to act(...) function must return undefined, or a Promise.', - ], - {withoutStack: true}, - ); - expect(() => act(() => 123)).toErrorDev( - [ - 'The callback passed to act(...) function must return undefined, or a Promise.', - ], - {withoutStack: true}, - ); - }); - - // @gate __DEV__ - it('warns if you try to await a sync .act call', () => { - expect(() => act(() => {}).then(() => {})).toErrorDev( - [ - 'Do not await the result of calling act(...) with sync logic, it is not a Promise.', - ], - {withoutStack: true}, - ); - }); }); describe('asynchronous tests', () => { @@ -401,15 +375,17 @@ function runActTests(label, render, unmount, rerender) { await act(async () => { render(, container); - // flush a little to start the timer - expect(Scheduler).toFlushAndYield([]); + }); + expect(container.innerHTML).toBe('0'); + // Flush the pending timers + await act(async () => { await sleep(100); }); expect(container.innerHTML).toBe('1'); }); // @gate __DEV__ - it('flushes microtasks before exiting', async () => { + it('flushes microtasks before exiting (async function)', async () => { function App() { const [ctr, setCtr] = React.useState(0); async function someAsyncFunction() { @@ -431,6 +407,31 @@ function runActTests(label, render, unmount, rerender) { expect(container.innerHTML).toEqual('1'); }); + // @gate __DEV__ + it('flushes microtasks before exiting (sync function)', async () => { + // Same as previous test, but the callback passed to `act` is not itself + // an async function. + function App() { + const [ctr, setCtr] = React.useState(0); + async function someAsyncFunction() { + // queue a bunch of promises to be sure they all flush + await null; + await null; + await null; + setCtr(1); + } + React.useEffect(() => { + someAsyncFunction(); + }, []); + return ctr; + } + + await act(() => { + render(, container); + }); + expect(container.innerHTML).toEqual('1'); + }); + // @gate __DEV__ it('warns if you do not await an act call', async () => { spyOnDevAndProd(console, 'error'); @@ -461,7 +462,13 @@ function runActTests(label, render, unmount, rerender) { await sleep(150); if (__DEV__) { - expect(console.error).toHaveBeenCalledTimes(1); + expect(console.error).toHaveBeenCalledTimes(2); + expect(console.error.calls.argsFor(0)[0]).toMatch( + 'You seem to have overlapping act() calls', + ); + expect(console.error.calls.argsFor(1)[0]).toMatch( + 'You seem to have overlapping act() calls', + ); } }); diff --git a/packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.internal.js b/packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.internal.js deleted file mode 100644 index 501b01336b1b3..0000000000000 --- a/packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.internal.js +++ /dev/null @@ -1,56 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @emails react-core - */ - -let React; -let ReactDOM; -let ReactFeatureFlags; - -function App() { - return null; -} - -beforeEach(() => { - jest.resetModules(); - jest.unmock('scheduler'); - React = require('react'); - ReactDOM = require('react-dom'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.warnAboutUnmockedScheduler = true; -}); - -afterEach(() => { - ReactFeatureFlags.warnAboutUnmockedScheduler = false; -}); - -it('should warn in legacy mode', () => { - expect(() => { - ReactDOM.render(, document.createElement('div')); - }).toErrorDev( - ['Starting from React v18, the "scheduler" module will need to be mocked'], - {withoutStack: true}, - ); - // does not warn twice - expect(() => { - ReactDOM.render(, document.createElement('div')); - }).toErrorDev([]); -}); - -it('does not warn if Scheduler is mocked', () => { - jest.resetModules(); - jest.mock('scheduler', () => require('scheduler/unstable_mock')); - React = require('react'); - ReactDOM = require('react-dom'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.warnAboutUnmockedScheduler = true; - - // This should not warn - expect(() => { - ReactDOM.render(, document.createElement('div')); - }).toErrorDev([]); -}); diff --git a/packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.js b/packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.js deleted file mode 100644 index 960154dbea253..0000000000000 --- a/packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.js +++ /dev/null @@ -1,42 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @emails react-core - */ - -let React; -let ReactDOM; - -function App() { - return null; -} - -beforeEach(() => { - jest.resetModules(); - jest.unmock('scheduler'); - React = require('react'); - ReactDOM = require('react-dom'); -}); - -it('does not warn when rendering in legacy mode', () => { - expect(() => { - ReactDOM.render(, document.createElement('div')); - }).toErrorDev([]); -}); - -it('should warn when rendering in concurrent mode', () => { - expect(() => { - ReactDOM.createRoot(document.createElement('div')).render(); - }).toErrorDev( - 'In Concurrent or Sync modes, the "scheduler" module needs to be mocked ' + - 'to guarantee consistent behaviour across tests and browsers.', - {withoutStack: true}, - ); - // does not warn twice - expect(() => { - ReactDOM.createRoot(document.createElement('div')).render(); - }).toErrorDev([]); -}); diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsInternalAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsInternalAct.js index 3adc786eed5fa..6afe4af8aa49e 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsInternalAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsInternalAct.js @@ -20,7 +20,7 @@ const IsThisRendererActing = SecretInternals.IsThisRendererActing; const batchedUpdates = ReactDOM.unstable_batchedUpdates; -const {IsSomeRendererActing} = ReactSharedInternals; +const {IsSomeRendererActing, ReactCurrentActQueue} = ReactSharedInternals; // This version of `act` is only used by our tests. Unlike the public version // of `act`, it's designed to work identically in both production and @@ -28,6 +28,8 @@ const {IsSomeRendererActing} = ReactSharedInternals; // version, too, since our constraints in our test suite are not the same as // those of developers using React — we're testing React itself, as opposed to // building an app with React. +// TODO: Replace the internal "concurrent" implementations of `act` with a +// single shared module. let actingUpdatesScopeDepth = 0; @@ -50,8 +52,14 @@ export function unstable_concurrentAct(scope: () => Thenable | void) { IsSomeRendererActing.current = true; IsThisRendererActing.current = true; actingUpdatesScopeDepth++; + if (__DEV__ && actingUpdatesScopeDepth === 1) { + ReactCurrentActQueue.disableActWarning = true; + } const unwind = () => { + if (__DEV__ && actingUpdatesScopeDepth === 1) { + ReactCurrentActQueue.disableActWarning = false; + } actingUpdatesScopeDepth--; IsSomeRendererActing.current = previousIsSomeRendererActing; IsThisRendererActing.current = previousIsThisRendererActing; diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 9d2c87621bcef..59cea7cd5d971 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -31,7 +31,7 @@ import { import ReactSharedInternals from 'shared/ReactSharedInternals'; import enqueueTask from 'shared/enqueueTask'; -const {IsSomeRendererActing} = ReactSharedInternals; +const {IsSomeRendererActing, ReactCurrentActQueue} = ReactSharedInternals; type Container = { rootID: string, @@ -1048,6 +1048,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // version, too, since our constraints in our test suite are not the same as // those of developers using React — we're testing React itself, as opposed to // building an app with React. + // TODO: Replace the internal "concurrent" implementations of `act` with a + // single shared module. const {batchedUpdates, IsThisRendererActing} = NoopRenderer; let actingUpdatesScopeDepth = 0; @@ -1071,8 +1073,14 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { IsSomeRendererActing.current = true; IsThisRendererActing.current = true; actingUpdatesScopeDepth++; + if (__DEV__ && actingUpdatesScopeDepth === 1) { + ReactCurrentActQueue.disableActWarning = true; + } const unwind = () => { + if (__DEV__ && actingUpdatesScopeDepth === 1) { + ReactCurrentActQueue.disableActWarning = false; + } actingUpdatesScopeDepth--; IsSomeRendererActing.current = previousIsSomeRendererActing; IsThisRendererActing.current = previousIsThisRendererActing; diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 394aa983174c4..b195a0cb57488 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -79,7 +79,6 @@ import { requestEventTime, warnIfNotCurrentlyActingEffectsInDEV, warnIfNotCurrentlyActingUpdatesInDev, - warnIfNotScopedWithMatchingAct, markSkippedUpdateLanes, isInterleavedUpdate, } from './ReactFiberWorkLoop.new'; @@ -2011,7 +2010,6 @@ function dispatchAction( if (__DEV__) { // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests if ('undefined' !== typeof jest) { - warnIfNotScopedWithMatchingAct(fiber); warnIfNotCurrentlyActingUpdatesInDev(fiber); } } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index a5eac6836e283..7c51c43ad2e7e 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -79,7 +79,6 @@ import { requestEventTime, warnIfNotCurrentlyActingEffectsInDEV, warnIfNotCurrentlyActingUpdatesInDev, - warnIfNotScopedWithMatchingAct, markSkippedUpdateLanes, isInterleavedUpdate, } from './ReactFiberWorkLoop.old'; @@ -2011,7 +2010,6 @@ function dispatchAction( if (__DEV__) { // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests if ('undefined' !== typeof jest) { - warnIfNotScopedWithMatchingAct(fiber); warnIfNotCurrentlyActingUpdatesInDev(fiber); } } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index c53fabbb8e4ae..2c59cd62a812b 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -60,8 +60,6 @@ import { discreteUpdates, flushDiscreteUpdates, flushPassiveEffects, - warnIfNotScopedWithMatchingAct, - warnIfUnmockedScheduler, IsThisRendererActing, act, } from './ReactFiberWorkLoop.new'; @@ -272,13 +270,6 @@ export function updateContainer( } const current = container.current; const eventTime = requestEventTime(); - if (__DEV__) { - // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests - if ('undefined' !== typeof jest) { - warnIfUnmockedScheduler(current); - warnIfNotScopedWithMatchingAct(current); - } - } const lane = requestUpdateLane(current); if (enableSchedulingProfiler) { diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index 73d412bfcd1ae..614b9adfd8de3 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -60,8 +60,6 @@ import { discreteUpdates, flushDiscreteUpdates, flushPassiveEffects, - warnIfNotScopedWithMatchingAct, - warnIfUnmockedScheduler, IsThisRendererActing, act, } from './ReactFiberWorkLoop.old'; @@ -272,13 +270,6 @@ export function updateContainer( } const current = container.current; const eventTime = requestEventTime(); - if (__DEV__) { - // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests - if ('undefined' !== typeof jest) { - warnIfUnmockedScheduler(current); - warnIfNotScopedWithMatchingAct(current); - } - } const lane = requestUpdateLane(current); if (enableSchedulingProfiler) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index eb26348ca16e6..c38bfed01dc47 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -24,7 +24,6 @@ import { enableProfilerCommitHooks, enableProfilerNestedUpdatePhase, enableProfilerNestedUpdateScheduledHook, - warnAboutUnmockedScheduler, deferRenderPhaseUpdateToNextBatch, enableDebugTracing, enableSchedulingProfiler, @@ -37,7 +36,8 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; import { - scheduleCallback, + // Aliased because `act` will override and push to an internal queue + scheduleCallback as Scheduler_scheduleCallback, cancelCallback, shouldYield, requestPaint, @@ -79,9 +79,6 @@ import { markRenderStopped, } from './SchedulingProfiler'; -// The scheduler is imported here *only* to detect whether it's been mocked -import * as Scheduler from 'scheduler'; - import { resetAfterCommit, scheduleTimeout, @@ -247,7 +244,7 @@ const { ReactCurrentDispatcher, ReactCurrentOwner, ReactCurrentBatchConfig, - IsSomeRendererActing, + ReactCurrentActQueue, } = ReactSharedInternals; type ExecutionContext = number; @@ -2781,51 +2778,40 @@ export function restorePendingUpdaters(root: FiberRoot, lanes: Lanes): void { } } -export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void { +function scheduleCallback(priorityLevel, callback) { if (__DEV__) { - if ( - warnsIfNotActing === true && - IsSomeRendererActing.current === true && - IsThisRendererActing.current !== true - ) { - const previousFiber = ReactCurrentFiberCurrent; - try { - setCurrentDebugFiberInDEV(fiber); - console.error( - "It looks like you're using the wrong act() around your test interactions.\n" + - 'Be sure to use the matching version of act() corresponding to your renderer:\n\n' + - '// for react-dom:\n' + - // Break up imports to avoid accidentally parsing them as dependencies. - 'import {act} fr' + - "om 'react-dom/test-utils';\n" + - '// ...\n' + - 'act(() => ...);\n\n' + - '// for react-test-renderer:\n' + - // Break up imports to avoid accidentally parsing them as dependencies. - 'import TestRenderer fr' + - "om 'react-test-renderer';\n" + - 'const {act} = TestRenderer;\n' + - '// ...\n' + - 'act(() => ...);', - ); - } finally { - if (previousFiber) { - setCurrentDebugFiberInDEV(fiber); - } else { - resetCurrentDebugFiberInDEV(); - } - } + // If we're currently inside an `act` scope, bypass Scheduler and push to + // the `act` queue instead. + const actQueue = ReactCurrentActQueue.current; + if (actQueue !== null) { + actQueue.push(callback); + return null; + } else { + return Scheduler_scheduleCallback(priorityLevel, callback); } + } else { + // In production, always call Scheduler. This function will be stripped out. + return Scheduler_scheduleCallback(priorityLevel, callback); } } +function shouldForceFlushFallbacksInDEV() { + // Never force flush in production. This function should get stripped out. + return __DEV__ && ReactCurrentActQueue.current !== null; +} + export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void { if (__DEV__) { if ( warnsIfNotActing === true && (fiber.mode & StrictLegacyMode) !== NoMode && - IsSomeRendererActing.current === false && - IsThisRendererActing.current === false + ReactCurrentActQueue.current === null && + // Our internal tests use a custom implementation of `act` that works by + // mocking the Scheduler package. Disable the `act` warning. + // TODO: Maybe the warning should be disabled by default, and then turned + // on at the testing frameworks layer? Instead of what we do now, which + // is check if a `jest` global is defined. + ReactCurrentActQueue.disableActWarning === false ) { console.error( 'An update to %s ran an effect, but was not wrapped in act(...).\n\n' + @@ -2849,8 +2835,13 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { if ( warnsIfNotActing === true && executionContext === NoContext && - IsSomeRendererActing.current === false && - IsThisRendererActing.current === false + ReactCurrentActQueue.current === null && + // Our internal tests use a custom implementation of `act` that works by + // mocking the Scheduler package. Disable the `act` warning. + // TODO: Maybe the warning should be disabled by default, and then turned + // on at the testing frameworks layer? Instead of what we do now, which + // is check if a `jest` global is defined. + ReactCurrentActQueue.disableActWarning === false ) { const previousFiber = ReactCurrentFiberCurrent; try { @@ -2881,251 +2872,185 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { export const warnIfNotCurrentlyActingUpdatesInDev = warnIfNotCurrentlyActingUpdatesInDEV; -// In tests, we want to enforce a mocked scheduler. -let didWarnAboutUnmockedScheduler = false; -// TODO Before we release concurrent mode, revisit this and decide whether a mocked -// scheduler is the actual recommendation. The alternative could be a testing build, -// a new lib, or whatever; we dunno just yet. This message is for early adopters -// to get their tests right. +// Implementation of `act` testing API. +// TODO: Move to isomorphic module. +let actScopeDepth = 0; +let didWarnNoAwaitAct = false; -export function warnIfUnmockedScheduler(fiber: Fiber) { +export function act(callback: () => Thenable): Thenable { if (__DEV__) { - if ( - didWarnAboutUnmockedScheduler === false && - Scheduler.unstable_flushAllWithoutAsserting === undefined - ) { - if (fiber.mode & ConcurrentMode) { - didWarnAboutUnmockedScheduler = true; - console.error( - 'In Concurrent or Sync modes, the "scheduler" module needs to be mocked ' + - 'to guarantee consistent behaviour across tests and browsers. ' + - 'For example, with jest: \n' + - // Break up requires to avoid accidentally parsing them as dependencies. - "jest.mock('scheduler', () => require" + - "('scheduler/unstable_mock'));\n\n" + - 'For more info, visit https://reactjs.org/link/mock-scheduler', - ); - } else if (warnAboutUnmockedScheduler === true) { - didWarnAboutUnmockedScheduler = true; - console.error( - 'Starting from React v18, the "scheduler" module will need to be mocked ' + - 'to guarantee consistent behaviour across tests and browsers. ' + - 'For example, with jest: \n' + - // Break up requires to avoid accidentally parsing them as dependencies. - "jest.mock('scheduler', () => require" + - "('scheduler/unstable_mock'));\n\n" + - 'For more info, visit https://reactjs.org/link/mock-scheduler', - ); - } - } - } -} - -// `act` testing API -// -// 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. - -let isFlushingAct = false; -let isInsideThisAct = false; - -function shouldForceFlushFallbacksInDEV() { - // Never force flush in production. This function should get stripped out. - return __DEV__ && actingUpdatesScopeDepth > 0; -} - -const flushMockScheduler = Scheduler.unstable_flushAllWithoutAsserting; -const isSchedulerMocked = typeof flushMockScheduler === 'function'; + // `act` calls can be nested, so we track the depth. This represents the + // number of `act` scopes on the stack. + const prevActScopeDepth = actScopeDepth; + actScopeDepth++; -// 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; + if (ReactCurrentActQueue.current === null) { + // This is the outermost `act` scope. Initialize the queue. The reconciler + // will detect the queue and use it instead of Scheduler. + ReactCurrentActQueue.current = []; } - } 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; + + let result; try { - let didFlushWork = false; - while (flushPassiveEffects()) { - didFlushWork = true; - } - return didFlushWork; - } finally { - isFlushingAct = prevIsFlushing; + result = batchedUpdates(callback); + } catch (error) { + popActScope(prevActScopeDepth); + throw error; } - } -} -function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { - try { - flushActWork(); - enqueueTask(() => { - if (flushActWork()) { - flushWorkAndMicroTasks(onDone); + if ( + result !== null && + typeof result === 'object' && + typeof result.then === 'function' + ) { + // The callback is an async function (i.e. returned a promise). Wait + // for it to resolve before exiting the current scope. + let wasAwaited = false; + const thenable = { + then(resolve, reject) { + wasAwaited = true; + result.then( + () => { + popActScope(prevActScopeDepth); + if (actScopeDepth === 0) { + // We've exited the outermost act scope. Recursively flush the + // queue until there's no remaining work. + recursivelyFlushAsyncActWork(resolve, reject); + } else { + resolve(); + } + }, + error => { + // The callback threw an error. + popActScope(prevActScopeDepth); + reject(error); + }, + ); + }, + }; + + if (__DEV__) { + if (!didWarnNoAwaitAct && typeof Promise !== 'undefined') { + // eslint-disable-next-line no-undef + Promise.resolve() + .then(() => {}) + .then(() => { + if (!wasAwaited) { + didWarnNoAwaitAct = true; + console.error( + 'You called act(async () => ...) without await. ' + + 'This could lead to unexpected testing behaviour, ' + + 'interleaving multiple act calls and mixing their ' + + 'scopes. ' + + 'You should - await act(async () => ...);', + ); + } + }); + } + } + return thenable; + } else { + // The callback is not an async function. Exit the current scope + // immediately, without awaiting. + popActScope(prevActScopeDepth); + if (actScopeDepth === 0) { + // Exiting the outermost act scope. Flush the queue. + const queue = ReactCurrentActQueue.current; + if (queue !== null) { + flushActQueue(queue); + ReactCurrentActQueue.current = null; + } + // Return a thenable. If the user awaits it, we'll flush again in + // case additional work was scheduled by a microtask. + return { + then(resolve, reject) { + // Confirm we haven't re-entered another `act` scope, in case + // the user does something weird like await the thenable + // multiple times. + if (ReactCurrentActQueue.current === null) { + // Recursively flush the queue until there's no remaining work. + ReactCurrentActQueue.current = []; + recursivelyFlushAsyncActWork(resolve, reject); + } + }, + }; } else { - onDone(); + // Since we're inside a nested `act` scope, the returned thenable + // immediately resolves. The outer scope will flush the queue. + return { + then(resolve, reject) { + resolve(); + }, + }; } - }); - } catch (err) { - onDone(err); - } -} - -// we track the 'depth' of the act() calls with this counter, -// so we can tell if any async act() calls try to run in parallel. - -let actingUpdatesScopeDepth = 0; - -export function act(callback: () => Thenable): Thenable { - if (!__DEV__) { + } + } else { invariant( false, 'act(...) is not supported in production builds of React.', ); } +} - const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; - actingUpdatesScopeDepth++; - - 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 - console.error( - 'You seem to have overlapping act() calls, this is not supported. ' + - 'Be sure to await previous act() calls before making a new one. ', - ); - } +function popActScope(prevActScopeDepth) { + if (__DEV__) { + if (prevActScopeDepth !== actScopeDepth - 1) { + console.error( + 'You seem to have overlapping act() calls, this is not supported. ' + + 'Be sure to await previous act() calls before making a new one. ', + ); } + actScopeDepth = prevActScopeDepth; } +} - let result; - try { - result = batchedUpdates(callback); - } catch (error) { - // on sync errors, we still want to 'cleanup' and decrement actingUpdatesScopeDepth - onDone(); - throw error; - } - - if ( - result !== null && - typeof result === 'object' && - typeof result.then === 'function' - ) { - // setup a boolean that gets set to true only - // once this act() call is await-ed - let called = false; - if (__DEV__) { - if (typeof Promise !== 'undefined') { - //eslint-disable-next-line no-undef - Promise.resolve() - .then(() => {}) - .then(() => { - if (called === false) { - console.error( - 'You called act(async () => ...) without await. ' + - 'This could lead to unexpected testing behaviour, interleaving multiple act ' + - 'calls and mixing their scopes. You should - await act(async () => ...);', - ); - } - }); - } - } - - // in the async case, the returned thenable runs the callback, flushes - // effects and microtasks in a loop until flushPassiveEffects() === false, - // and cleans up - return { - then(resolve, reject) { - called = true; - result.then( - () => { - if ( - actingUpdatesScopeDepth > 1 || - (isSchedulerMocked === true && - previousIsSomeRendererActing === true) - ) { - onDone(); - resolve(); - return; - } - // we're about to exit the act() scope, - // now's the time to flush tasks/effects - flushWorkAndMicroTasks((err: ?Error) => { - onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); - }, - err => { - onDone(); - reject(err); - }, - ); - }, - }; - } else { - if (__DEV__) { - if (result !== undefined) { - console.error( - 'The callback passed to act(...) function ' + - 'must return undefined, or a Promise. You returned %s', - result, - ); +function recursivelyFlushAsyncActWork(resolve, reject) { + if (__DEV__) { + const queue = ReactCurrentActQueue.current; + if (queue !== null) { + try { + flushActQueue(queue); + enqueueTask(() => { + if (queue.length === 0) { + // No additional work was scheduled. Finish. + ReactCurrentActQueue.current = null; + resolve(); + } else { + // Keep flushing work until there's none left. + recursivelyFlushAsyncActWork(resolve, reject); + } + }); + } catch (error) { + reject(error); } + } else { + resolve(); } + } +} - // flush effects until none remain, and cleanup - try { - if ( - actingUpdatesScopeDepth === 1 && - (isSchedulerMocked === false || previousIsSomeRendererActing === false) - ) { - // we're about to exit the act() scope, - // now's the time to flush effects - flushActWork(); +let isFlushing = false; +function flushActQueue(queue) { + if (__DEV__) { + if (!isFlushing) { + // Prevent re-entrancy. + isFlushing = true; + let i = 0; + try { + for (; i < queue.length; i++) { + let callback = queue[i]; + do { + callback = callback(true); + } while (callback !== null); + } + queue.length = 0; + } catch (error) { + // If something throws, leave the remaining callbacks on the queue. + queue = queue.slice(i + 1); + throw error; + } finally { + isFlushing = false; } - onDone(); - } catch (err) { - onDone(); - throw err; } - - // in the sync case, the returned thenable only warns *if* await-ed - return { - then(resolve) { - if (__DEV__) { - console.error( - 'Do not await the result of calling act(...) with sync logic, it is not a Promise.', - ); - } - resolve(); - }, - }; } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 0b34957078535..59a0a9c6908b2 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -24,7 +24,6 @@ import { enableProfilerCommitHooks, enableProfilerNestedUpdatePhase, enableProfilerNestedUpdateScheduledHook, - warnAboutUnmockedScheduler, deferRenderPhaseUpdateToNextBatch, enableDebugTracing, enableSchedulingProfiler, @@ -37,7 +36,8 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; import { - scheduleCallback, + // Aliased because `act` will override and push to an internal queue + scheduleCallback as Scheduler_scheduleCallback, cancelCallback, shouldYield, requestPaint, @@ -79,9 +79,6 @@ import { markRenderStopped, } from './SchedulingProfiler'; -// The scheduler is imported here *only* to detect whether it's been mocked -import * as Scheduler from 'scheduler'; - import { resetAfterCommit, scheduleTimeout, @@ -247,7 +244,7 @@ const { ReactCurrentDispatcher, ReactCurrentOwner, ReactCurrentBatchConfig, - IsSomeRendererActing, + ReactCurrentActQueue, } = ReactSharedInternals; type ExecutionContext = number; @@ -2781,51 +2778,40 @@ export function restorePendingUpdaters(root: FiberRoot, lanes: Lanes): void { } } -export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void { +function scheduleCallback(priorityLevel, callback) { if (__DEV__) { - if ( - warnsIfNotActing === true && - IsSomeRendererActing.current === true && - IsThisRendererActing.current !== true - ) { - const previousFiber = ReactCurrentFiberCurrent; - try { - setCurrentDebugFiberInDEV(fiber); - console.error( - "It looks like you're using the wrong act() around your test interactions.\n" + - 'Be sure to use the matching version of act() corresponding to your renderer:\n\n' + - '// for react-dom:\n' + - // Break up imports to avoid accidentally parsing them as dependencies. - 'import {act} fr' + - "om 'react-dom/test-utils';\n" + - '// ...\n' + - 'act(() => ...);\n\n' + - '// for react-test-renderer:\n' + - // Break up imports to avoid accidentally parsing them as dependencies. - 'import TestRenderer fr' + - "om 'react-test-renderer';\n" + - 'const {act} = TestRenderer;\n' + - '// ...\n' + - 'act(() => ...);', - ); - } finally { - if (previousFiber) { - setCurrentDebugFiberInDEV(fiber); - } else { - resetCurrentDebugFiberInDEV(); - } - } + // If we're currently inside an `act` scope, bypass Scheduler and push to + // the `act` queue instead. + const actQueue = ReactCurrentActQueue.current; + if (actQueue !== null) { + actQueue.push(callback); + return null; + } else { + return Scheduler_scheduleCallback(priorityLevel, callback); } + } else { + // In production, always call Scheduler. This function will be stripped out. + return Scheduler_scheduleCallback(priorityLevel, callback); } } +function shouldForceFlushFallbacksInDEV() { + // Never force flush in production. This function should get stripped out. + return __DEV__ && ReactCurrentActQueue.current !== null; +} + export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void { if (__DEV__) { if ( warnsIfNotActing === true && (fiber.mode & StrictLegacyMode) !== NoMode && - IsSomeRendererActing.current === false && - IsThisRendererActing.current === false + ReactCurrentActQueue.current === null && + // Our internal tests use a custom implementation of `act` that works by + // mocking the Scheduler package. Disable the `act` warning. + // TODO: Maybe the warning should be disabled by default, and then turned + // on at the testing frameworks layer? Instead of what we do now, which + // is check if a `jest` global is defined. + ReactCurrentActQueue.disableActWarning === false ) { console.error( 'An update to %s ran an effect, but was not wrapped in act(...).\n\n' + @@ -2849,8 +2835,13 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { if ( warnsIfNotActing === true && executionContext === NoContext && - IsSomeRendererActing.current === false && - IsThisRendererActing.current === false + ReactCurrentActQueue.current === null && + // Our internal tests use a custom implementation of `act` that works by + // mocking the Scheduler package. Disable the `act` warning. + // TODO: Maybe the warning should be disabled by default, and then turned + // on at the testing frameworks layer? Instead of what we do now, which + // is check if a `jest` global is defined. + ReactCurrentActQueue.disableActWarning === false ) { const previousFiber = ReactCurrentFiberCurrent; try { @@ -2881,251 +2872,185 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { export const warnIfNotCurrentlyActingUpdatesInDev = warnIfNotCurrentlyActingUpdatesInDEV; -// In tests, we want to enforce a mocked scheduler. -let didWarnAboutUnmockedScheduler = false; -// TODO Before we release concurrent mode, revisit this and decide whether a mocked -// scheduler is the actual recommendation. The alternative could be a testing build, -// a new lib, or whatever; we dunno just yet. This message is for early adopters -// to get their tests right. +// Implementation of `act` testing API. +// TODO: Move to isomorphic module. +let actScopeDepth = 0; +let didWarnNoAwaitAct = false; -export function warnIfUnmockedScheduler(fiber: Fiber) { +export function act(callback: () => Thenable): Thenable { if (__DEV__) { - if ( - didWarnAboutUnmockedScheduler === false && - Scheduler.unstable_flushAllWithoutAsserting === undefined - ) { - if (fiber.mode & ConcurrentMode) { - didWarnAboutUnmockedScheduler = true; - console.error( - 'In Concurrent or Sync modes, the "scheduler" module needs to be mocked ' + - 'to guarantee consistent behaviour across tests and browsers. ' + - 'For example, with jest: \n' + - // Break up requires to avoid accidentally parsing them as dependencies. - "jest.mock('scheduler', () => require" + - "('scheduler/unstable_mock'));\n\n" + - 'For more info, visit https://reactjs.org/link/mock-scheduler', - ); - } else if (warnAboutUnmockedScheduler === true) { - didWarnAboutUnmockedScheduler = true; - console.error( - 'Starting from React v18, the "scheduler" module will need to be mocked ' + - 'to guarantee consistent behaviour across tests and browsers. ' + - 'For example, with jest: \n' + - // Break up requires to avoid accidentally parsing them as dependencies. - "jest.mock('scheduler', () => require" + - "('scheduler/unstable_mock'));\n\n" + - 'For more info, visit https://reactjs.org/link/mock-scheduler', - ); - } - } - } -} - -// `act` testing API -// -// 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. - -let isFlushingAct = false; -let isInsideThisAct = false; - -function shouldForceFlushFallbacksInDEV() { - // Never force flush in production. This function should get stripped out. - return __DEV__ && actingUpdatesScopeDepth > 0; -} - -const flushMockScheduler = Scheduler.unstable_flushAllWithoutAsserting; -const isSchedulerMocked = typeof flushMockScheduler === 'function'; + // `act` calls can be nested, so we track the depth. This represents the + // number of `act` scopes on the stack. + const prevActScopeDepth = actScopeDepth; + actScopeDepth++; -// 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; + if (ReactCurrentActQueue.current === null) { + // This is the outermost `act` scope. Initialize the queue. The reconciler + // will detect the queue and use it instead of Scheduler. + ReactCurrentActQueue.current = []; } - } 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; + + let result; try { - let didFlushWork = false; - while (flushPassiveEffects()) { - didFlushWork = true; - } - return didFlushWork; - } finally { - isFlushingAct = prevIsFlushing; + result = batchedUpdates(callback); + } catch (error) { + popActScope(prevActScopeDepth); + throw error; } - } -} -function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { - try { - flushActWork(); - enqueueTask(() => { - if (flushActWork()) { - flushWorkAndMicroTasks(onDone); + if ( + result !== null && + typeof result === 'object' && + typeof result.then === 'function' + ) { + // The callback is an async function (i.e. returned a promise). Wait + // for it to resolve before exiting the current scope. + let wasAwaited = false; + const thenable = { + then(resolve, reject) { + wasAwaited = true; + result.then( + () => { + popActScope(prevActScopeDepth); + if (actScopeDepth === 0) { + // We've exited the outermost act scope. Recursively flush the + // queue until there's no remaining work. + recursivelyFlushAsyncActWork(resolve, reject); + } else { + resolve(); + } + }, + error => { + // The callback threw an error. + popActScope(prevActScopeDepth); + reject(error); + }, + ); + }, + }; + + if (__DEV__) { + if (!didWarnNoAwaitAct && typeof Promise !== 'undefined') { + // eslint-disable-next-line no-undef + Promise.resolve() + .then(() => {}) + .then(() => { + if (!wasAwaited) { + didWarnNoAwaitAct = true; + console.error( + 'You called act(async () => ...) without await. ' + + 'This could lead to unexpected testing behaviour, ' + + 'interleaving multiple act calls and mixing their ' + + 'scopes. ' + + 'You should - await act(async () => ...);', + ); + } + }); + } + } + return thenable; + } else { + // The callback is not an async function. Exit the current scope + // immediately, without awaiting. + popActScope(prevActScopeDepth); + if (actScopeDepth === 0) { + // Exiting the outermost act scope. Flush the queue. + const queue = ReactCurrentActQueue.current; + if (queue !== null) { + flushActQueue(queue); + ReactCurrentActQueue.current = null; + } + // Return a thenable. If the user awaits it, we'll flush again in + // case additional work was scheduled by a microtask. + return { + then(resolve, reject) { + // Confirm we haven't re-entered another `act` scope, in case + // the user does something weird like await the thenable + // multiple times. + if (ReactCurrentActQueue.current === null) { + // Recursively flush the queue until there's no remaining work. + ReactCurrentActQueue.current = []; + recursivelyFlushAsyncActWork(resolve, reject); + } + }, + }; } else { - onDone(); + // Since we're inside a nested `act` scope, the returned thenable + // immediately resolves. The outer scope will flush the queue. + return { + then(resolve, reject) { + resolve(); + }, + }; } - }); - } catch (err) { - onDone(err); - } -} - -// we track the 'depth' of the act() calls with this counter, -// so we can tell if any async act() calls try to run in parallel. - -let actingUpdatesScopeDepth = 0; - -export function act(callback: () => Thenable): Thenable { - if (!__DEV__) { + } + } else { invariant( false, 'act(...) is not supported in production builds of React.', ); } +} - const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; - actingUpdatesScopeDepth++; - - 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 - console.error( - 'You seem to have overlapping act() calls, this is not supported. ' + - 'Be sure to await previous act() calls before making a new one. ', - ); - } +function popActScope(prevActScopeDepth) { + if (__DEV__) { + if (prevActScopeDepth !== actScopeDepth - 1) { + console.error( + 'You seem to have overlapping act() calls, this is not supported. ' + + 'Be sure to await previous act() calls before making a new one. ', + ); } + actScopeDepth = prevActScopeDepth; } +} - let result; - try { - result = batchedUpdates(callback); - } catch (error) { - // on sync errors, we still want to 'cleanup' and decrement actingUpdatesScopeDepth - onDone(); - throw error; - } - - if ( - result !== null && - typeof result === 'object' && - typeof result.then === 'function' - ) { - // setup a boolean that gets set to true only - // once this act() call is await-ed - let called = false; - if (__DEV__) { - if (typeof Promise !== 'undefined') { - //eslint-disable-next-line no-undef - Promise.resolve() - .then(() => {}) - .then(() => { - if (called === false) { - console.error( - 'You called act(async () => ...) without await. ' + - 'This could lead to unexpected testing behaviour, interleaving multiple act ' + - 'calls and mixing their scopes. You should - await act(async () => ...);', - ); - } - }); - } - } - - // in the async case, the returned thenable runs the callback, flushes - // effects and microtasks in a loop until flushPassiveEffects() === false, - // and cleans up - return { - then(resolve, reject) { - called = true; - result.then( - () => { - if ( - actingUpdatesScopeDepth > 1 || - (isSchedulerMocked === true && - previousIsSomeRendererActing === true) - ) { - onDone(); - resolve(); - return; - } - // we're about to exit the act() scope, - // now's the time to flush tasks/effects - flushWorkAndMicroTasks((err: ?Error) => { - onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); - }, - err => { - onDone(); - reject(err); - }, - ); - }, - }; - } else { - if (__DEV__) { - if (result !== undefined) { - console.error( - 'The callback passed to act(...) function ' + - 'must return undefined, or a Promise. You returned %s', - result, - ); +function recursivelyFlushAsyncActWork(resolve, reject) { + if (__DEV__) { + const queue = ReactCurrentActQueue.current; + if (queue !== null) { + try { + flushActQueue(queue); + enqueueTask(() => { + if (queue.length === 0) { + // No additional work was scheduled. Finish. + ReactCurrentActQueue.current = null; + resolve(); + } else { + // Keep flushing work until there's none left. + recursivelyFlushAsyncActWork(resolve, reject); + } + }); + } catch (error) { + reject(error); } + } else { + resolve(); } + } +} - // flush effects until none remain, and cleanup - try { - if ( - actingUpdatesScopeDepth === 1 && - (isSchedulerMocked === false || previousIsSomeRendererActing === false) - ) { - // we're about to exit the act() scope, - // now's the time to flush effects - flushActWork(); +let isFlushing = false; +function flushActQueue(queue) { + if (__DEV__) { + if (!isFlushing) { + // Prevent re-entrancy. + isFlushing = true; + let i = 0; + try { + for (; i < queue.length; i++) { + let callback = queue[i]; + do { + callback = callback(true); + } while (callback !== null); + } + queue.length = 0; + } catch (error) { + // If something throws, leave the remaining callbacks on the queue. + queue = queue.slice(i + 1); + throw error; + } finally { + isFlushing = false; } - onDone(); - } catch (err) { - onDone(); - throw err; } - - // in the sync case, the returned thenable only warns *if* await-ed - return { - then(resolve) { - if (__DEV__) { - console.error( - 'Do not await the result of calling act(...) with sync logic, it is not a Promise.', - ); - } - resolve(); - }, - }; } } diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index 246790ead3a46..e414fbcd15102 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -53,7 +53,7 @@ import {getPublicInstance} from './ReactTestHostConfig'; import {ConcurrentRoot, LegacyRoot} from 'react-reconciler/src/ReactRootTags'; import {allowConcurrentByDefault} from 'shared/ReactFeatureFlags'; -const {IsSomeRendererActing} = ReactSharedInternals; +const {IsSomeRendererActing, ReactCurrentActQueue} = ReactSharedInternals; type TestRendererOptions = { createNodeMock: (element: React$Element) => any, @@ -604,6 +604,8 @@ let actingUpdatesScopeDepth = 0; // building an app with React. // TODO: Migrate our tests to use ReactNoop. Although we would need to figure // out a solution for Relay, which has some Concurrent Mode tests. +// TODO: Replace the internal "concurrent" implementations of `act` with a +// single shared module. function unstable_concurrentAct(scope: () => Thenable | void) { if (Scheduler.unstable_flushAllWithoutAsserting === undefined) { throw Error( @@ -623,8 +625,14 @@ function unstable_concurrentAct(scope: () => Thenable | void) { IsSomeRendererActing.current = true; IsThisRendererActing.current = true; actingUpdatesScopeDepth++; + if (__DEV__ && actingUpdatesScopeDepth === 1) { + ReactCurrentActQueue.disableActWarning = true; + } const unwind = () => { + if (__DEV__ && actingUpdatesScopeDepth === 1) { + ReactCurrentActQueue.disableActWarning = false; + } actingUpdatesScopeDepth--; IsSomeRendererActing.current = previousIsSomeRendererActing; IsThisRendererActing.current = previousIsThisRendererActing; diff --git a/packages/react-test-renderer/src/__tests__/ReactTestRendererAct-test.js b/packages/react-test-renderer/src/__tests__/ReactTestRendererAct-test.js index 28cc0062e8a7b..4fe1afea6d62f 100644 --- a/packages/react-test-renderer/src/__tests__/ReactTestRendererAct-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactTestRendererAct-test.js @@ -40,23 +40,6 @@ describe('ReactTestRenderer.act()', () => { expect(root.toJSON()).toEqual('1'); }); - it("warns if you don't use .act", () => { - let setCtr; - function App(props) { - const [ctr, _setCtr] = React.useState(0); - setCtr = _setCtr; - return ctr; - } - - ReactTestRenderer.create(); - - expect(() => { - setCtr(1); - }).toErrorDev([ - 'An update to App inside a test was not wrapped in act(...)', - ]); - }); - describe('async', () => { // @gate __DEV__ it('should work with async/await', async () => { diff --git a/packages/react/src/ReactCurrentActQueue.js b/packages/react/src/ReactCurrentActQueue.js new file mode 100644 index 0000000000000..1b7966337cad5 --- /dev/null +++ b/packages/react/src/ReactCurrentActQueue.js @@ -0,0 +1,22 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +type RendererTask = boolean => RendererTask | null; + +const ReactCurrentActQueue = { + current: (null: null | Array), + // Our internal tests use a custom implementation of `act` that works by + // mocking the Scheduler package. Use this field to disable the `act` warning. + // TODO: Maybe the warning should be disabled by default, and then turned + // on at the testing frameworks layer? Instead of what we do now, which + // is check if a `jest` global is defined. + disableActWarning: (false: boolean), +}; + +export default ReactCurrentActQueue; diff --git a/packages/react/src/ReactSharedInternals.js b/packages/react/src/ReactSharedInternals.js index 791d41adf8044..e95ea15848751 100644 --- a/packages/react/src/ReactSharedInternals.js +++ b/packages/react/src/ReactSharedInternals.js @@ -8,6 +8,7 @@ import assign from 'object-assign'; import ReactCurrentDispatcher from './ReactCurrentDispatcher'; import ReactCurrentBatchConfig from './ReactCurrentBatchConfig'; +import ReactCurrentActQueue from './ReactCurrentActQueue'; import ReactCurrentOwner from './ReactCurrentOwner'; import ReactDebugCurrentFrame from './ReactDebugCurrentFrame'; import IsSomeRendererActing from './IsSomeRendererActing'; @@ -23,6 +24,7 @@ const ReactSharedInternals = { if (__DEV__) { ReactSharedInternals.ReactDebugCurrentFrame = ReactDebugCurrentFrame; + ReactSharedInternals.ReactCurrentActQueue = ReactCurrentActQueue; } export default ReactSharedInternals; diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 60efb97e42017..d966c9ec54c9d 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -78,7 +78,6 @@ export const enableCreateEventHandleAPI = false; // We will enforce mocking scheduler with scheduler/unstable_mock at some point. (v18?) // Till then, we warn about the missing mock, but still fallback to a legacy mode compatible version -export const warnAboutUnmockedScheduler = false; // Add a callback property to suspense to notify which promises are currently // in the update queue. This allows reporting and tracing of what is causing diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index a58e9e543895f..76cfa7d18f3ea 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -30,7 +30,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; export const warnAboutDeprecatedLifecycles = true; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; -export const warnAboutUnmockedScheduler = true; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; export const warnAboutStringRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 1fc0969a3fa90..f8595c01b9274 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -29,7 +29,6 @@ export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; -export const warnAboutUnmockedScheduler = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; export const warnAboutStringRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index fe3abe658bdd4..528f42ddc3cc0 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -29,7 +29,6 @@ export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; -export const warnAboutUnmockedScheduler = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; export const warnAboutStringRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index bdf3607a1ee82..6a70e11a4c557 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -29,7 +29,6 @@ export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; -export const warnAboutUnmockedScheduler = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; export const warnAboutStringRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index a4f209a6b9f30..ef095eef7e7bd 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -29,7 +29,6 @@ export const disableJavaScriptURLs = false; export const disableInputAttributeSyncing = false; export const enableScopeAPI = true; export const enableCreateEventHandleAPI = false; -export const warnAboutUnmockedScheduler = true; export const enableSuspenseCallback = true; export const warnAboutDefaultPropsOnFunctionComponents = false; export const warnAboutStringRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 5c8baa8a12049..58c6400b0aeef 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -29,7 +29,6 @@ export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; -export const warnAboutUnmockedScheduler = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; export const warnAboutStringRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 30cf572ddce21..0c3d15890b277 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -29,7 +29,6 @@ export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = true; export const enableCreateEventHandleAPI = true; -export const warnAboutUnmockedScheduler = true; export const enableSuspenseCallback = true; export const warnAboutDefaultPropsOnFunctionComponents = false; export const warnAboutStringRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 04abc20de6464..e028c9ddb8ef9 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -75,8 +75,6 @@ export const enableCreateEventHandleAPI = true; export const enableScopeAPI = true; -export const warnAboutUnmockedScheduler = true; - export const enableSuspenseCallback = true; export const enableComponentStackLocations = true; diff --git a/scripts/jest/setupTests.www.js b/scripts/jest/setupTests.www.js index 3812d8f1354b5..e303d9fee22a5 100644 --- a/scripts/jest/setupTests.www.js +++ b/scripts/jest/setupTests.www.js @@ -14,7 +14,6 @@ jest.mock('shared/ReactFeatureFlags', () => { // www configuration. Update those tests so that they work against the www // configuration, too. Then remove these overrides. wwwFlags.disableLegacyContext = defaultFlags.disableLegacyContext; - wwwFlags.warnAboutUnmockedScheduler = defaultFlags.warnAboutUnmockedScheduler; wwwFlags.disableJavaScriptURLs = defaultFlags.disableJavaScriptURLs; return wwwFlags;