From 11bdace1be2db1b562065bd2c1fd1dee771a6d93 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 1 Sep 2020 22:22:54 -0500 Subject: [PATCH] Decouple public, internal act implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the next major release, we intend to drop support for using the `act` testing helper in production. (It already fires a warning.) The rationale is that, in order for `act` to work, you must either mock the testing environment or add extra logic at runtime. Mocking the testing environment isn't ideal because it requires extra set up for the user. Extra logic at runtime is fine only in development mode — we don't want to slow down the production builds. Since most people only run their tests in development mode, dropping support for production should be fine; if there's demand, we can add it back later using a special testing build that is identical to the production build except for the additional testing logic. One blocker for removing production support is that we currently use `act` to test React itself. We must test React in both development and production modes. So, the solution is to fork `act` into separate public and internal implementations: - *public implementation of `act`* – exposed to users, only works in development mode, uses special runtime logic, does not support partial rendering - *internal implementation of `act`* – private, works in both development and productionm modes, only used by the React Core test suite, uses no special runtime logic, supports partial rendering (i.e. `toFlushAndYieldThrough`) The internal implementation should mostly match the public implementation's behavior, but since it's a private API, it doesn't have to match exactly. It works by mocking the test environment: it uses a mock build of Scheduler to flush rendering tasks, and Jest's mock timers to flush Suspense placeholders. --- In this first commit, I've added the internal forks of `act` and migrated our tests to use them. The public `act` implementation is unaffected for now; I will leave refactoring/clean-up for a later step. --- .../ReactHooksInspectionIntegration-test.js | 2 +- .../__tests__/inspectedElementContext-test.js | 45 ++-- .../__tests__/storeComponentFilters-test.js | 2 +- .../src/__tests__/ReactDOMFiberAsync-test.js | 2 +- .../ReactDOMServerIntegrationHooks-test.js | 28 ++- ...DOMServerPartialHydration-test.internal.js | 2 +- ...MServerSelectiveHydration-test.internal.js | 4 +- .../ReactDOMServerSuspense-test.internal.js | 4 +- .../ReactDOMSuspensePlaceholder-test.js | 2 +- .../ReactErrorBoundaries-test.internal.js | 2 +- .../src/__tests__/ReactUpdates-test.js | 2 +- .../ReactDOMServerIntegrationTestUtils.js | 4 +- packages/react-dom/src/client/ReactDOM.js | 3 + .../DOMPluginEventSystem-test.internal.js | 10 +- .../__tests__/ChangeEventPlugin-test.js | 2 +- .../src/test-utils/ReactTestUtils.js | 5 +- .../src/test-utils/ReactTestUtilsAct.js | 235 ++++++------------ .../__tests__/useFocusWithin-test.internal.js | 2 +- .../src/createReactNoop.js | 121 ++++++++- .../src/ReactFiberWorkLoop.new.js | 15 +- .../src/ReactFiberWorkLoop.old.js | 15 +- .../src/SchedulerWithReactIntegration.new.js | 7 +- .../src/__tests__/ReactHooks-test.internal.js | 10 +- .../__tests__/ReactNoopRendererAct-test.js | 1 - .../__tests__/ReactSuspense-test.internal.js | 4 +- .../ReactSuspenseWithNoopRenderer-test.js | 3 +- .../SchedulingProfiler-test.internal.js | 4 +- .../useMutableSourceHydration-test.js | 2 +- .../src/__tests__/ReactFresh-test.js | 4 +- .../__tests__/ReactFreshIntegration-test.js | 2 +- .../src/ReactTestRenderer.js | 119 +++++++++ .../ReactFlightDOMRelay-test.internal.js | 2 +- .../src/__tests__/ReactFlightDOM-test.js | 2 +- .../ReactDOMTracing-test.internal.js | 15 +- .../__tests__/ReactProfiler-test.internal.js | 39 +-- .../src/__tests__/useSubscription-test.js | 2 +- 36 files changed, 447 insertions(+), 276 deletions(-) diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js index 01461dfc1648e..4dfe0bac0b1f1 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js @@ -22,7 +22,7 @@ describe('ReactHooksInspectionIntegration', () => { React = require('react'); ReactTestRenderer = require('react-test-renderer'); Scheduler = require('scheduler'); - act = ReactTestRenderer.act; + act = ReactTestRenderer.unstable_concurrentAct; ReactDebugTools = require('react-debug-tools'); }); diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js index 29e8e379e4e43..a68de20ab0ee1 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js @@ -33,6 +33,9 @@ describe('InspectedElementContext', () => { let TestUtils; let TreeContextController; + let TestUtilsAct; + let TestRendererAct; + beforeEach(() => { utils = require('./utils'); utils.beforeEachProfiling(); @@ -47,7 +50,9 @@ describe('InspectedElementContext', () => { ReactDOM = require('react-dom'); PropTypes = require('prop-types'); TestUtils = require('react-dom/test-utils'); + TestUtilsAct = TestUtils.unstable_concurrentAct; TestRenderer = utils.requireTestRenderer(); + TestRendererAct = TestUtils.unstable_concurrentAct; BridgeContext = require('react-devtools-shared/src/devtools/views/context') .BridgeContext; @@ -999,8 +1004,8 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); inspectedElement = null; - TestUtils.act(() => { - TestRenderer.act(() => { + TestUtilsAct(() => { + TestRendererAct(() => { getInspectedElementPath(id, ['props', 'nestedObject', 'a']); jest.runOnlyPendingTimers(); }); @@ -1009,8 +1014,8 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); inspectedElement = null; - TestUtils.act(() => { - TestRenderer.act(() => { + TestUtilsAct(() => { + TestRendererAct(() => { getInspectedElementPath(id, ['props', 'nestedObject', 'a', 'b', 'c']); jest.runOnlyPendingTimers(); }); @@ -1021,8 +1026,8 @@ describe('InspectedElementContext', () => { ); inspectedElement = null; - TestUtils.act(() => { - TestRenderer.act(() => { + TestUtilsAct(() => { + TestRendererAct(() => { getInspectedElementPath(id, [ 'props', 'nestedObject', @@ -1041,8 +1046,8 @@ describe('InspectedElementContext', () => { ); inspectedElement = null; - TestUtils.act(() => { - TestRenderer.act(() => { + TestUtilsAct(() => { + TestRendererAct(() => { getInspectedElementPath(id, ['hooks', 0, 'value']); jest.runOnlyPendingTimers(); }); @@ -1051,8 +1056,8 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toMatchSnapshot('5: Inspect hooks.0.value'); inspectedElement = null; - TestUtils.act(() => { - TestRenderer.act(() => { + TestUtilsAct(() => { + TestRendererAct(() => { getInspectedElementPath(id, ['hooks', 0, 'value', 'foo', 'bar']); jest.runOnlyPendingTimers(); }); @@ -1108,8 +1113,8 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); inspectedElement = null; - TestUtils.act(() => { - TestRenderer.act(() => { + TestUtilsAct(() => { + TestRendererAct(() => { getInspectedElementPath(id, ['props', 'set_of_sets', 0]); jest.runOnlyPendingTimers(); }); @@ -1179,7 +1184,7 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); inspectedElement = null; - TestRenderer.act(() => { + TestRendererAct(() => { getInspectedElementPath(id, ['props', 'nestedObject', 'a']); jest.runOnlyPendingTimers(); }); @@ -1187,15 +1192,15 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); inspectedElement = null; - TestRenderer.act(() => { + TestRendererAct(() => { getInspectedElementPath(id, ['props', 'nestedObject', 'c']); jest.runOnlyPendingTimers(); }); expect(inspectedElement).not.toBeNull(); expect(inspectedElement).toMatchSnapshot('3: Inspect props.nestedObject.c'); - TestRenderer.act(() => { - TestUtils.act(() => { + TestRendererAct(() => { + TestUtilsAct(() => { ReactDOM.render( { }); }); - TestRenderer.act(() => { + TestRendererAct(() => { inspectedElement = null; jest.advanceTimersByTime(1000); }); @@ -1281,7 +1286,7 @@ describe('InspectedElementContext', () => { expect(inspectedElement).not.toBeNull(); expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); - TestUtils.act(() => { + TestUtilsAct(() => { ReactDOM.render( { inspectedElement = null; - TestRenderer.act(() => { - TestUtils.act(() => { + TestRendererAct(() => { + TestUtilsAct(() => { getInspectedElementPath(id, ['props', 'nestedObject', 'a']); jest.runOnlyPendingTimers(); }); diff --git a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js index 341e06b04d57e..1a5be03f09aad 100644 --- a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js +++ b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js @@ -20,7 +20,7 @@ describe('Store component filters', () => { let utils; const act = (callback: Function) => { - TestUtils.act(() => { + TestUtils.unstable_concurrentAct(() => { callback(); }); jest.runAllTimers(); // Flush Bridge operations diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index d6baddbe45c9e..5e783489bb932 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -28,7 +28,7 @@ describe('ReactDOMFiberAsync', () => { container = document.createElement('div'); React = require('react'); ReactDOM = require('react-dom'); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; Scheduler = require('scheduler'); document.body.appendChild(container); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index a66a86866776f..6cb57a8560b73 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -17,6 +17,7 @@ let React; let ReactDOM; let ReactDOMServer; let ReactTestUtils; +let act; let Scheduler; let useState; let useReducer; @@ -43,6 +44,7 @@ function initModules() { ReactDOMServer = require('react-dom/server'); ReactTestUtils = require('react-dom/test-utils'); Scheduler = require('scheduler'); + act = ReactTestUtils.unstable_concurrentAct; useState = React.useState; useReducer = React.useReducer; useEffect = React.useEffect; @@ -1063,7 +1065,7 @@ describe('ReactDOMServerHooks', () => { expect(domNode.children.length).toEqual(1); expect(oldClientId).not.toBeNull(); - await ReactTestUtils.act(async () => _setShowId(true)); + await act(async () => _setShowId(true)); expect(domNode.children.length).toEqual(2); expect(domNode.children[0].getAttribute('aria-labelledby')).toEqual( @@ -1281,7 +1283,7 @@ describe('ReactDOMServerHooks', () => { const oldServerId = container.children[0].children[0].getAttribute('id'); expect(oldServerId).not.toBeNull(); - await ReactTestUtils.act(async () => { + await act(async () => { _setShowDiv(true); }); expect(container.children[0].children.length).toEqual(2); @@ -1322,7 +1324,7 @@ describe('ReactDOMServerHooks', () => { const oldServerId = container.children[0].children[0].getAttribute('id'); expect(oldServerId).not.toBeNull(); - await ReactTestUtils.act(async () => { + await act(async () => { _setShowDiv(true); }); expect(container.children[0].children.length).toEqual(2); @@ -1356,12 +1358,12 @@ describe('ReactDOMServerHooks', () => { document.body.append(container); container.innerHTML = ReactDOMServer.renderToString(); const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); - ReactTestUtils.act(() => { + act(() => { root.render(); }); expect(Scheduler).toHaveYielded(['App', 'App']); // The ID goes from not being used to being added to the page - ReactTestUtils.act(() => { + act(() => { _setShow(true); }); expect(Scheduler).toHaveYielded(['App', 'App']); @@ -1391,7 +1393,7 @@ describe('ReactDOMServerHooks', () => { ReactDOM.hydrate(, container); expect(Scheduler).toHaveYielded(['App', 'App']); // The ID goes from not being used to being added to the page - ReactTestUtils.act(() => { + act(() => { _setShow(true); }); expect(Scheduler).toHaveYielded(['App']); @@ -1418,12 +1420,12 @@ describe('ReactDOMServerHooks', () => { document.body.append(container); container.innerHTML = ReactDOMServer.renderToString(); const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); - ReactTestUtils.act(() => { + act(() => { root.render(); }); // The ID goes from not being used to being added to the page - ReactTestUtils.act(() => { + act(() => { ReactDOM.flushSync(() => { _setShow(true); }); @@ -1518,7 +1520,7 @@ describe('ReactDOMServerHooks', () => { expect(child1Ref.current).toBe(null); expect(Scheduler).toHaveYielded([]); - ReactTestUtils.act(() => { + act(() => { _setShow(true); // State update should trigger the ID to update, which changes the props @@ -1603,7 +1605,7 @@ describe('ReactDOMServerHooks', () => { suspend = true; const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); - await ReactTestUtils.act(async () => { + await act(async () => { root.render(); }); jest.runAllTimers(); @@ -1616,7 +1618,7 @@ describe('ReactDOMServerHooks', () => { container.children[0].children[0].getAttribute('id'), ).not.toBeNull(); - await ReactTestUtils.act(async () => { + await act(async () => { suspend = false; resolve(); await promise; @@ -1703,7 +1705,7 @@ describe('ReactDOMServerHooks', () => { suspend = false; const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); - await ReactTestUtils.act(async () => { + await act(async () => { root.render(); }); jest.runAllTimers(); @@ -1968,7 +1970,7 @@ describe('ReactDOMServerHooks', () => { expect(Scheduler).toHaveYielded([]); expect(Scheduler).toFlushAndYield([]); - ReactTestUtils.act(() => { + act(() => { _setShow(false); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 1e6a912651832..9a897789b47ca 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -79,7 +79,7 @@ describe('ReactDOMServerPartialHydration', () => { React = require('react'); ReactDOM = require('react-dom'); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; ReactDOMServer = require('react-dom/server'); Scheduler = require('scheduler'); Suspense = React.Suspense; diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 3a948a0651b1d..6b543805ea102 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -17,6 +17,7 @@ let ReactDOMServer; let ReactTestUtils; let Scheduler; let Suspense; +let act; function dispatchMouseHoverEvent(to, from) { if (!to) { @@ -101,6 +102,7 @@ describe('ReactDOMServerSelectiveHydration', () => { ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); ReactTestUtils = require('react-dom/test-utils'); + act = ReactTestUtils.unstable_concurrentAct; Scheduler = require('scheduler'); Suspense = React.Suspense; }); @@ -880,7 +882,7 @@ describe('ReactDOMServerSelectiveHydration', () => { const spanC = container.getElementsByTagName('span')[4]; const root = ReactDOM.createRoot(container, {hydrate: true}); - ReactTestUtils.act(() => { + act(() => { root.render(); // Hydrate the shell. diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js index cc3b563d0ff43..6c48b36bd107d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js @@ -15,6 +15,7 @@ let React; let ReactDOM; let ReactDOMServer; let ReactTestUtils; +let act; function initModules() { // Reset warning cache. @@ -24,6 +25,7 @@ function initModules() { ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); ReactTestUtils = require('react-dom/test-utils'); + act = ReactTestUtils.unstable_concurrentAct; // Make them available to the helpers. return { @@ -124,7 +126,7 @@ describe('ReactDOMServerSuspense', () => { expect(divB.tagName).toBe('DIV'); expect(divB.textContent).toBe('B'); - ReactTestUtils.act(() => { + act(() => { const root = ReactDOM.createBlockingRoot(parent, {hydrate: true}); root.render(example); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js index 02da1e84b202c..44f6aeebe6711 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js @@ -28,7 +28,7 @@ describe('ReactDOMSuspensePlaceholder', () => { ReactCache = require('react-cache'); ReactTestUtils = require('react-dom/test-utils'); Scheduler = require('scheduler'); - act = ReactTestUtils.act; + act = ReactTestUtils.unstable_concurrentAct; Suspense = React.Suspense; container = document.createElement('div'); document.body.appendChild(container); diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 204c67ba52846..46ace25fd7a16 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -44,7 +44,7 @@ describe('ReactErrorBoundaries', () => { ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; ReactDOM = require('react-dom'); React = require('react'); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; Scheduler = require('scheduler'); BrokenConstructor = class extends React.Component { diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 2bf80a2090c47..f876158b6718e 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -21,7 +21,7 @@ describe('ReactUpdates', () => { React = require('react'); ReactDOM = require('react-dom'); ReactTestUtils = require('react-dom/test-utils'); - act = ReactTestUtils.act; + act = ReactTestUtils.unstable_concurrentAct; Scheduler = require('scheduler'); }); diff --git a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js index 33fd350cb6e6f..b81a749837802 100644 --- a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js +++ b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js @@ -49,11 +49,11 @@ module.exports = function(initModules) { function asyncReactDOMRender(reactElement, domElement, forceHydrate) { return new Promise(resolve => { if (forceHydrate) { - ReactTestUtils.act(() => { + ReactTestUtils.unstable_concurrentAct(() => { ReactDOM.hydrate(reactElement, domElement); }); } else { - ReactTestUtils.act(() => { + ReactTestUtils.unstable_concurrentAct(() => { ReactDOM.render(reactElement, domElement); }); } diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 5daeef818161f..78c920ec44afb 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -37,6 +37,7 @@ import { attemptHydrationAtCurrentPriority, runWithPriority, getCurrentUpdateLanePriority, + act, } from 'react-reconciler/src/ReactFiberReconciler'; import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -183,7 +184,9 @@ const Internals = { enqueueStateRestore, restoreStateIfNeeded, flushPassiveEffects, + // TODO: These are related to `act`, not events. Move to separate key? IsThisRendererActing, + act, ], }; diff --git a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js index 96387223787a3..1a125dc260628 100644 --- a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js @@ -17,6 +17,7 @@ let ReactDOM; let ReactDOMServer; let Scheduler; let ReactTestUtils; +let act; function dispatchEvent(element, type) { const event = document.createEvent('Event'); @@ -1236,6 +1237,7 @@ describe('DOMPluginEventSystem', () => { Scheduler = require('scheduler'); ReactDOMServer = require('react-dom/server'); ReactTestUtils = require('react-dom/test-utils'); + act = ReactTestUtils.unstable_concurrentAct; }); // @gate experimental @@ -2674,7 +2676,7 @@ describe('DOMPluginEventSystem', () => { const root = ReactDOM.createRoot(container2); - ReactTestUtils.act(() => { + act(() => { root.render(); }); jest.runAllTimers(); @@ -2686,7 +2688,7 @@ describe('DOMPluginEventSystem', () => { expect(onAfterBlur).toHaveBeenCalledTimes(0); suspend = true; - ReactTestUtils.act(() => { + act(() => { root.render(); }); jest.runAllTimers(); @@ -2746,7 +2748,7 @@ describe('DOMPluginEventSystem', () => { document.body.appendChild(container2); const root = ReactDOM.createRoot(container2); - ReactTestUtils.act(() => { + act(() => { root.render(); }); @@ -2757,7 +2759,7 @@ describe('DOMPluginEventSystem', () => { // Suspend. This hides the input node, causing it to lose focus. suspend = true; - ReactTestUtils.act(() => { + act(() => { root.render(); }); diff --git a/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js index 9f80e7329c510..041231cac30cd 100644 --- a/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js @@ -736,7 +736,7 @@ describe('ChangeEventPlugin', () => { // @gate experimental it('mouse enter/leave should be user-blocking but not discrete', async () => { - const {act} = TestUtils; + const {unstable_concurrentAct: act} = TestUtils; const {useState} = React; const root = ReactDOM.unstable_createRoot(container); diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index f281438e12288..ded943e77e07d 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -18,7 +18,7 @@ import { import {SyntheticEvent} from '../events/SyntheticEvent'; import invariant from 'shared/invariant'; import {ELEMENT_NODE} from '../shared/HTMLNodeType'; -import act from './ReactTestUtilsAct'; +import {unstable_concurrentAct} from './ReactTestUtilsAct'; import { rethrowCaughtError, invokeGuardedCallbackAndCatchFirstError, @@ -35,7 +35,9 @@ const [ restoreStateIfNeeded, /* eslint-disable no-unused-vars */ flushPassiveEffects, + // TODO: These are related to `act`, not events. Move to separate key? IsThisRendererActing, + act, /* eslint-enable no-unused-vars */ ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; @@ -728,4 +730,5 @@ export { nativeTouchData, Simulate, act, + unstable_concurrentAct, }; diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index 363826ca15342..306fbdda823d0 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -7,8 +7,6 @@ * @flow */ -import type {Thenable} from 'shared/ReactTypes'; - import * as ReactDOM from 'react-dom'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import enqueueTask from 'shared/enqueueTask'; @@ -22,8 +20,8 @@ const [ getFiberCurrentPropsFromNode, enqueueStateRestore, restoreStateIfNeeded, - /* eslint-enable no-unused-vars */ flushPassiveEffects, + /* eslint-enable no-unused-vars */ IsThisRendererActing, ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; @@ -31,184 +29,115 @@ const batchedUpdates = ReactDOM.unstable_batchedUpdates; const {IsSomeRendererActing} = ReactSharedInternals; -// this implementation should be exactly the same in -// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js - -const isSchedulerMocked = - typeof Scheduler.unstable_flushAllWithoutAsserting === 'function'; -const flushWork = - Scheduler.unstable_flushAllWithoutAsserting || - function() { - let didFlushWork = false; - while (flushPassiveEffects()) { - didFlushWork = true; - } - - return didFlushWork; - }; - -function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) { - try { - flushWork(); - enqueueTask(() => { - if (flushWork()) { - flushWorkAndMicroTasks(onDone); - } else { - onDone(); - } - }); - } 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. +// 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 +// development. It may have slightly different behavior from the public +// 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. let actingUpdatesScopeDepth = 0; -let didWarnAboutUsingActInProd = false; -function act(callback: () => Thenable): Thenable { - if (!__DEV__) { - if (didWarnAboutUsingActInProd === false) { - didWarnAboutUsingActInProd = true; - // eslint-disable-next-line react-internal/no-production-logging - console.error( - 'act(...) is not supported in production builds of React, and might not behave as expected.', - ); - } +export function unstable_concurrentAct(scope: () => void) { + if (Scheduler.unstable_flushAllWithoutAsserting === undefined) { + throw Error( + 'This version of `act` requires a special mock build of Scheduler.', + ); + } + if (setTimeout._isMockFunction !== true) { + throw Error( + "This version of `act` requires Jest's timer mocks " + + '(i.e. jest.useFakeTimers).', + ); } - const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; - actingUpdatesScopeDepth++; + const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; const previousIsSomeRendererActing = IsSomeRendererActing.current; const previousIsThisRendererActing = IsThisRendererActing.current; IsSomeRendererActing.current = true; IsThisRendererActing.current = true; + actingUpdatesScopeDepth++; - function onDone() { + const unwind = () => { actingUpdatesScopeDepth--; IsSomeRendererActing.current = previousIsSomeRendererActing; IsThisRendererActing.current = previousIsThisRendererActing; + if (__DEV__) { if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) { - // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned + // 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. ', ); } } - } + }; - let result; + // TODO: This would be way simpler if 1) we required a promise to be + // returned and 2) we could use async/await. Since it's only our used in + // our test suite, we should be able to. 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 () => ...);', + const thenable = batchedUpdates(scope); + if ( + typeof thenable === 'object' && + thenable !== null && + typeof thenable.then === 'function' + ) { + return { + then(resolve: () => void, reject: (error: mixed) => void) { + thenable.then( + () => { + flushActWork( + () => { + unwind(); + resolve(); + }, + error => { + unwind(); + reject(error); + }, ); - } - }); - } - } - - // 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, - ); + }, + error => { + unwind(); + reject(error); + }, + ); + }, + }; + } else { + try { + // TODO: Let's not support non-async scopes at all in our tests. Need to + // migrate existing tests. + let didFlushWork; + do { + didFlushWork = Scheduler.unstable_flushAllWithoutAsserting(); + } while (didFlushWork); + } finally { + unwind(); } } + } catch (error) { + unwind(); + throw error; + } +} - // flush effects until none remain, and cleanup +function flushActWork(resolve, reject) { + // TODO: Run timers to flush suspended fallbacks + // jest.runOnlyPendingTimers(); + enqueueTask(() => { try { - if ( - actingUpdatesScopeDepth === 1 && - (isSchedulerMocked === false || previousIsSomeRendererActing === false) - ) { - // we're about to exit the act() scope, - // now's the time to flush effects - flushWork(); + const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting(); + if (didFlushWork) { + flushActWork(resolve, reject); + } else { + resolve(); } - onDone(); - } catch (err) { - onDone(); - throw err; + } catch (error) { + reject(error); } - - // 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(); - }, - }; - } + }); } - -export default act; diff --git a/packages/react-interactions/events/src/dom/create-event-handle/__tests__/useFocusWithin-test.internal.js b/packages/react-interactions/events/src/dom/create-event-handle/__tests__/useFocusWithin-test.internal.js index 1b9c47224a97e..f1e8f57ac08d5 100644 --- a/packages/react-interactions/events/src/dom/create-event-handle/__tests__/useFocusWithin-test.internal.js +++ b/packages/react-interactions/events/src/dom/create-event-handle/__tests__/useFocusWithin-test.internal.js @@ -29,7 +29,7 @@ function initializeModules(hasPointerEvents) { ReactDOM = require('react-dom'); ReactTestRenderer = require('react-test-renderer'); Scheduler = require('scheduler'); - act = ReactTestRenderer.act; + act = ReactTestRenderer.unstable_concurrentAct; // TODO: This import throws outside of experimental mode. Figure out better // strategy for gated imports. diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index cf9552fb2f9b9..9611abe68b844 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -27,6 +27,10 @@ import { LegacyRoot, } from 'react-reconciler/src/ReactRootTags'; +import ReactSharedInternals from 'shared/ReactSharedInternals'; +import enqueueTask from 'shared/enqueueTask'; +const {IsSomeRendererActing} = ReactSharedInternals; + type Container = { rootID: string, children: Array, @@ -958,7 +962,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { flushPassiveEffects: NoopRenderer.flushPassiveEffects, - act: NoopRenderer.act, + act: noopAct, + // act: NoopRenderer.act, // Logs the current state of the tree. dumpTree(rootID: string = DEFAULT_ROOT_ID) { @@ -1073,6 +1078,120 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }, }; + // 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 + // development. It may have slightly different behavior from the public + // 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. + + const {batchedUpdates, IsThisRendererActing} = NoopRenderer; + let actingUpdatesScopeDepth = 0; + + function noopAct(scope: () => void) { + if (Scheduler.unstable_flushAllWithoutAsserting === undefined) { + throw Error( + 'This version of `act` requires a special mock build of Scheduler.', + ); + } + if (setTimeout._isMockFunction !== true) { + throw Error( + "This version of `act` requires Jest's timer mocks " + + '(i.e. jest.useFakeTimers).', + ); + } + + const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + const previousIsSomeRendererActing = IsSomeRendererActing.current; + const previousIsThisRendererActing = IsThisRendererActing.current; + IsSomeRendererActing.current = true; + IsThisRendererActing.current = true; + actingUpdatesScopeDepth++; + + const unwind = () => { + actingUpdatesScopeDepth--; + IsSomeRendererActing.current = previousIsSomeRendererActing; + IsThisRendererActing.current = previousIsThisRendererActing; + + 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. ', + ); + } + } + }; + + // TODO: This would be way simpler if 1) we required a promise to be + // returned and 2) we could use async/await. Since it's only our used in + // our test suite, we should be able to. + try { + const thenable = batchedUpdates(scope); + if ( + typeof thenable === 'object' && + thenable !== null && + typeof thenable.then === 'function' + ) { + return { + then(resolve, reject) { + thenable.then( + () => { + flushActWork( + () => { + unwind(); + resolve(); + }, + error => { + unwind(); + reject(error); + }, + ); + }, + error => { + unwind(); + reject(error); + }, + ); + }, + }; + } else { + try { + // TODO: Let's not support non-async scopes at all in our tests. Need to + // migrate existing tests. + let didFlushWork; + do { + didFlushWork = Scheduler.unstable_flushAllWithoutAsserting(); + } while (didFlushWork); + } finally { + unwind(); + } + } + } catch (error) { + unwind(); + throw error; + } + } + + function flushActWork(resolve, reject) { + // TODO: Run timers to flush suspended fallbacks + // jest.runOnlyPendingTimers(); + enqueueTask(() => { + try { + const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting(); + if (didFlushWork) { + flushActWork(resolve, reject); + } else { + resolve(); + } + } catch (error) { + reject(error); + } + }); + } + return ReactNoop; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 1e386c3309548..eb835a685078b 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -3622,20 +3622,9 @@ function finishPendingInteractions(root, committedLanes) { 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; + // Never force flush in production. This function should get stripped out. + return __DEV__ && actingUpdatesScopeDepth > 0; } const flushMockScheduler = Scheduler.unstable_flushAllWithoutAsserting; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index e1cccf912c73d..49de7469eda68 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -3579,20 +3579,9 @@ function finishPendingInteractions(root, committedLanes) { 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; + // Never force flush in production. This function should get stripped out. + return __DEV__ && actingUpdatesScopeDepth > 0; } const flushMockScheduler = Scheduler.unstable_flushAllWithoutAsserting; diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js index 606a90252077e..f73ae2f8998f4 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js @@ -165,13 +165,13 @@ export function cancelCallback(callbackNode: mixed) { } } -export function flushSyncCallbackQueue() { +export function flushSyncCallbackQueue(): boolean { if (immediateQueueCallbackNode !== null) { const node = immediateQueueCallbackNode; immediateQueueCallbackNode = null; Scheduler_cancelCallback(node); } - flushSyncCallbackQueueImpl(); + return flushSyncCallbackQueueImpl(); } function flushSyncCallbackQueueImpl() { @@ -237,5 +237,8 @@ function flushSyncCallbackQueueImpl() { isFlushingSyncQueue = false; } } + return true; + } else { + return false; } } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 4c2a39718c9e4..bc37abbd322f4 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -31,7 +31,7 @@ describe('ReactHooks', () => { ReactTestRenderer = require('react-test-renderer'); Scheduler = require('scheduler'); ReactDOMServer = require('react-dom/server'); - act = ReactTestRenderer.act; + act = ReactTestRenderer.unstable_concurrentAct; }); if (__DEV__) { @@ -782,7 +782,7 @@ describe('ReactHooks', () => { } const root = ReactTestRenderer.create(null); - ReactTestRenderer.act(() => { + act(() => { root.update(); }); expect(root).toMatchRenderedOutput('4'); @@ -806,7 +806,7 @@ describe('ReactHooks', () => { } const root = ReactTestRenderer.create(null); - ReactTestRenderer.act(() => { + act(() => { root.update(); }); expect(root).toMatchRenderedOutput('4'); @@ -829,7 +829,7 @@ describe('ReactHooks', () => { } const root = ReactTestRenderer.create(null); - ReactTestRenderer.act(() => { + act(() => { root.update(); }); expect(root).toMatchRenderedOutput('4'); @@ -1830,7 +1830,7 @@ describe('ReactHooks', () => { return null; } - ReactTestRenderer.act(() => { + act(() => { ReactTestRenderer.create(); }); diff --git a/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js b/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js index ca5d786f3fad4..6fad734ad5781 100644 --- a/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js +++ b/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js @@ -9,7 +9,6 @@ // sanity tests for ReactNoop.act() -jest.useRealTimers(); const React = require('react'); const ReactNoop = require('react-noop-renderer'); const Scheduler = require('scheduler'); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index e806070e9cca1..54806dba475e9 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -21,7 +21,7 @@ describe('ReactSuspense', () => { ReactFeatureFlags.enableSchedulerTracing = true; React = require('react'); ReactTestRenderer = require('react-test-renderer'); - act = ReactTestRenderer.act; + act = ReactTestRenderer.unstable_concurrentAct; Scheduler = require('scheduler'); SchedulerTracing = require('scheduler/tracing'); ReactCache = require('react-cache'); @@ -440,7 +440,7 @@ describe('ReactSuspense', () => { unstable_isConcurrent: true, }); - await ReactTestRenderer.act(async () => { + await act(async () => { root.update(); expect(Scheduler).toFlushAndYield([ 'shouldHideInParent: false', diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index b6ed0d2be4d40..13d38e992c5e0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -3846,8 +3846,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { await ReactNoop.act(async () => { await resolveText('b'); }); - expect(Scheduler).toHaveYielded(['Promise resolved [b]']); - expect(Scheduler).toFlushAndYield(['b']); + expect(Scheduler).toHaveYielded(['Promise resolved [b]', 'b']); // The bug was that the pending state got stuck forever. expect(root).toMatchRenderedOutput(); }); diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js index 9fc02c7bdc101..55f8d3617c0de 100644 --- a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js +++ b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js @@ -447,7 +447,7 @@ describe('SchedulingProfiler', () => { return didMount; } - ReactTestRenderer.act(() => { + ReactTestRenderer.unstable_concurrentAct(() => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); }); @@ -480,7 +480,7 @@ describe('SchedulingProfiler', () => { return didRender; } - ReactTestRenderer.act(() => { + ReactTestRenderer.unstable_concurrentAct(() => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); }); diff --git a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js index d21f955bc945c..2cd6881c94f73 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js +++ b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js @@ -26,7 +26,7 @@ describe('useMutableSourceHydration', () => { ReactDOMServer = require('react-dom/server'); Scheduler = require('scheduler'); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; createMutableSource = React.unstable_createMutableSource; useMutableSource = React.unstable_useMutableSource; }); diff --git a/packages/react-refresh/src/__tests__/ReactFresh-test.js b/packages/react-refresh/src/__tests__/ReactFresh-test.js index 4c3c932c90ac0..3b6f69b13e318 100644 --- a/packages/react-refresh/src/__tests__/ReactFresh-test.js +++ b/packages/react-refresh/src/__tests__/ReactFresh-test.js @@ -29,7 +29,7 @@ describe('ReactFresh', () => { ReactFreshRuntime.injectIntoGlobalHook(global); ReactDOM = require('react-dom'); Scheduler = require('scheduler'); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; createReactClass = require('create-react-class/factory')( React.Component, React.isValidElement, @@ -3748,7 +3748,7 @@ describe('ReactFresh', () => { React = require('react'); ReactDOM = require('react-dom'); Scheduler = require('scheduler'); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; // Important! Inject into the global hook *after* ReactDOM runs: ReactFreshRuntime = require('react-refresh/runtime'); diff --git a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js index 4f3709a677ab8..f3538670e3717 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js @@ -30,7 +30,7 @@ describe('ReactFreshIntegration', () => { ReactFreshRuntime = require('react-refresh/runtime'); ReactFreshRuntime.injectIntoGlobalHook(global); ReactDOM = require('react-dom'); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; container = document.createElement('div'); document.body.appendChild(container); exportsObj = undefined; diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index ffd0eaaf09c9a..388c8c796a259 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -20,6 +20,7 @@ import { injectIntoDevTools, batchedUpdates, act, + IsThisRendererActing, } from 'react-reconciler/src/ReactFiberReconciler'; import {findCurrentFiberUsingSlowPath} from 'react-reconciler/src/ReactFiberTreeReflection'; import { @@ -44,10 +45,14 @@ import { import invariant from 'shared/invariant'; import getComponentName from 'shared/getComponentName'; import ReactVersion from 'shared/ReactVersion'; +import ReactSharedInternals from 'shared/ReactSharedInternals'; +import enqueueTask from 'shared/enqueueTask'; import {getPublicInstance} from './ReactTestHostConfig'; import {ConcurrentRoot, LegacyRoot} from 'react-reconciler/src/ReactRootTags'; +const {IsSomeRendererActing} = ReactSharedInternals; + type TestRendererOptions = { createNodeMock: (element: React$Element) => any, unstable_isConcurrent: boolean, @@ -581,10 +586,124 @@ injectIntoDevTools({ rendererPackageName: 'react-test-renderer', }); +let actingUpdatesScopeDepth = 0; + +// 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 +// development. It may have slightly different behavior from the public +// 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: Migrate our tests to use ReactNoop. Although we would need to figure +// out a solution for Relay, which has some Concurrent Mode tests. +function unstable_concurrentAct(scope: () => void) { + if (Scheduler.unstable_flushAllWithoutAsserting === undefined) { + throw Error( + 'This version of `act` requires a special mock build of Scheduler.', + ); + } + if (setTimeout._isMockFunction !== true) { + throw Error( + "This version of `act` requires Jest's timer mocks " + + '(i.e. jest.useFakeTimers).', + ); + } + + const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + const previousIsSomeRendererActing = IsSomeRendererActing.current; + const previousIsThisRendererActing = IsThisRendererActing.current; + IsSomeRendererActing.current = true; + IsThisRendererActing.current = true; + actingUpdatesScopeDepth++; + + const unwind = () => { + actingUpdatesScopeDepth--; + IsSomeRendererActing.current = previousIsSomeRendererActing; + IsThisRendererActing.current = previousIsThisRendererActing; + 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. ', + ); + } + } + }; + + // TODO: This would be way simpler if 1) we required a promise to be + // returned and 2) we could use async/await. Since it's only our used in + // our test suite, we should be able to. + try { + const thenable = batchedUpdates(scope); + if ( + typeof thenable === 'object' && + thenable !== null && + typeof thenable.then === 'function' + ) { + return { + then(resolve, reject) { + thenable.then( + () => { + flushActWork( + () => { + unwind(); + resolve(); + }, + error => { + unwind(); + reject(error); + }, + ); + }, + error => { + unwind(); + reject(error); + }, + ); + }, + }; + } else { + try { + // TODO: Let's not support non-async scopes at all in our tests. Need to + // migrate existing tests. + let didFlushWork; + do { + didFlushWork = Scheduler.unstable_flushAllWithoutAsserting(); + } while (didFlushWork); + } finally { + unwind(); + } + } + } catch (error) { + unwind(); + throw error; + } +} + +function flushActWork(resolve, reject) { + // TODO: Run timers to flush suspended fallbacks + // jest.runOnlyPendingTimers(); + enqueueTask(() => { + try { + const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting(); + if (didFlushWork) { + flushActWork(resolve, reject); + } else { + resolve(); + } + } catch (error) { + reject(error); + } + }); +} + export { Scheduler as _Scheduler, create, /* eslint-disable-next-line camelcase */ batchedUpdates as unstable_batchedUpdates, act, + unstable_concurrentAct, }; diff --git a/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js b/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js index 67db8e74da9dd..31d4707248c74 100644 --- a/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js +++ b/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js @@ -18,7 +18,7 @@ describe('ReactFlightDOMRelay', () => { beforeEach(() => { jest.resetModules(); - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; React = require('react'); ReactDOM = require('react-dom'); ReactDOMFlightRelayServer = require('react-transport-dom-relay/server'); diff --git a/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 72aa615260f27..ecd71cea82d9f 100644 --- a/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -37,7 +37,7 @@ describe('ReactFlightDOM', () => { jest.resetModules(); webpackModules = {}; webpackMap = {}; - act = require('react-dom/test-utils').act; + act = require('react-dom/test-utils').unstable_concurrentAct; Stream = require('stream'); React = require('react'); ReactDOM = require('react-dom'); diff --git a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js index 66f548f9c39d5..e2c6bbe8f59dd 100644 --- a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js +++ b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js @@ -16,6 +16,7 @@ let ReactFeatureFlags; let Scheduler; let SchedulerTracing; let TestUtils; +let act; let onInteractionScheduledWorkCompleted; let onInteractionTraced; let onWorkCanceled; @@ -42,6 +43,8 @@ function loadModules() { SchedulerTracing = require('scheduler/tracing'); TestUtils = require('react-dom/test-utils'); + act = TestUtils.unstable_concurrentAct; + onInteractionScheduledWorkCompleted = jest.fn(); onInteractionTraced = jest.fn(); onWorkCanceled = jest.fn(); @@ -118,7 +121,7 @@ describe('ReactDOMTracing', () => { const root = ReactDOM.createRoot(container); SchedulerTracing.unstable_trace('initialization', 0, () => { interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0]; - TestUtils.act(() => { + act(() => { root.render( @@ -190,7 +193,7 @@ describe('ReactDOMTracing', () => { SchedulerTracing.unstable_trace('initialization', 0, () => { interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0]; - TestUtils.act(() => { + act(() => { root.render( @@ -269,7 +272,7 @@ describe('ReactDOMTracing', () => { const root = ReactDOM.createRoot(container); SchedulerTracing.unstable_trace('initialization', 0, () => { interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0]; - TestUtils.act(() => { + act(() => { root.render( @@ -364,7 +367,7 @@ describe('ReactDOMTracing', () => { const root = ReactDOM.createRoot(container); // Schedule some idle work without any interactions. - TestUtils.act(() => { + act(() => { root.render( @@ -468,7 +471,7 @@ describe('ReactDOMTracing', () => { const container = document.createElement('div'); const root = ReactDOM.createRoot(container); - TestUtils.act(() => { + act(() => { root.render( @@ -568,7 +571,7 @@ describe('ReactDOMTracing', () => { let interaction; - TestUtils.act(() => { + act(() => { SchedulerTracing.unstable_trace('initialization', 0, () => { interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0]; // This render is only CPU bound. Nothing suspends. diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 193f3d1ac7f15..c6022c92531c6 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -16,6 +16,7 @@ let ReactNoop; let Scheduler; let ReactCache; let ReactTestRenderer; +let ReactTestRendererAct; let SchedulerTracing; let AdvanceTime; let AsyncText; @@ -45,9 +46,11 @@ function loadModules({ if (useNoopRenderer) { ReactNoop = require('react-noop-renderer'); ReactTestRenderer = null; + ReactTestRendererAct = null; } else { ReactNoop = null; ReactTestRenderer = require('react-test-renderer'); + ReactTestRendererAct = ReactTestRenderer.unstable_concurrentAct; } AdvanceTime = class extends React.Component { @@ -374,7 +377,7 @@ describe('Profiler', () => { Scheduler.unstable_advanceTime(20); // 30 -> 50 // Updating a sibling should not report a re-render. - ReactTestRenderer.act(updateProfilerSibling); + ReactTestRendererAct(updateProfilerSibling); expect(callback).not.toHaveBeenCalled(); }); @@ -1495,7 +1498,7 @@ describe('Profiler', () => { const setCountRef = React.createRef(null); let renderer = null; - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer = ReactTestRenderer.create( @@ -1523,7 +1526,7 @@ describe('Profiler', () => { expect(call[3]).toBe(2); // commit start time (before mutations or effects) expect(call[4]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events - ReactTestRenderer.act(() => setCountRef.current(count => count + 1)); + ReactTestRendererAct(() => setCountRef.current(count => count + 1)); expect(callback).toHaveBeenCalledTimes(2); @@ -1536,7 +1539,7 @@ describe('Profiler', () => { expect(call[3]).toBe(1013); // commit start time (before mutations or effects) expect(call[4]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer.update( @@ -1596,7 +1599,7 @@ describe('Profiler', () => { // Test an error that happens during an effect - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { ReactTestRenderer.create( { let renderer = null; - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer = ReactTestRenderer.create( { // Test an error that happens during an cleanup function - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer.update( { Scheduler.unstable_advanceTime(1); let renderer; - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer = ReactTestRenderer.create( @@ -1897,7 +1900,7 @@ describe('Profiler', () => { Scheduler.unstable_advanceTime(1); - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer.update( @@ -1919,7 +1922,7 @@ describe('Profiler', () => { Scheduler.unstable_advanceTime(1); - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer.update( , ); @@ -1961,7 +1964,7 @@ describe('Profiler', () => { Scheduler.unstable_advanceTime(1); - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { ReactTestRenderer.create( @@ -2015,7 +2018,7 @@ describe('Profiler', () => { const setCountRef = React.createRef(null); let renderer = null; - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer = ReactTestRenderer.create( @@ -2043,7 +2046,7 @@ describe('Profiler', () => { expect(call[3]).toBe(2); // commit start time (before mutations or effects) expect(call[4]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events - ReactTestRenderer.act(() => setCountRef.current(count => count + 1)); + ReactTestRendererAct(() => setCountRef.current(count => count + 1)); expect(callback).toHaveBeenCalledTimes(2); @@ -2056,7 +2059,7 @@ describe('Profiler', () => { expect(call[3]).toBe(1013); // commit start time (before mutations or effects) expect(call[4]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer.update( @@ -2116,7 +2119,7 @@ describe('Profiler', () => { // Test an error that happens during an effect - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { ReactTestRenderer.create( { let renderer = null; - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer = ReactTestRenderer.create( { // Test an error that happens during an cleanup function - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { renderer.update( { Scheduler.unstable_advanceTime(1); - ReactTestRenderer.act(() => { + ReactTestRendererAct(() => { SchedulerTracing.unstable_trace( interaction.name, interaction.timestamp, diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.js b/packages/use-subscription/src/__tests__/useSubscription-test.js index 52e15fd428229..6e0a9e5045661 100644 --- a/packages/use-subscription/src/__tests__/useSubscription-test.js +++ b/packages/use-subscription/src/__tests__/useSubscription-test.js @@ -27,7 +27,7 @@ describe('useSubscription', () => { ReactTestRenderer = require('react-test-renderer'); Scheduler = require('scheduler'); - act = ReactTestRenderer.act; + act = ReactTestRenderer.unstable_concurrentAct; BehaviorSubject = require('rxjs').BehaviorSubject; ReplaySubject = require('rxjs').ReplaySubject;