From d17086c7c813402a550d15a2f56dc43f1dbd1735 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 8 Sep 2020 10:11:45 -0500 Subject: [PATCH] Decouple public, internal act implementation (#19745) 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 | 27 +- .../src/test-utils/ReactTestUtilsAct.js | 257 +++++++----------- .../__tests__/useFocusWithin-test.internal.js | 2 +- .../src/createReactNoop.js | 122 ++++++++- .../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 | 120 ++++++++ .../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, 468 insertions(+), 301 deletions(-) diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js index 01461dfc16..4dfe0bac0b 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 29e8e379e4..a68de20ab0 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 341e06b04d..1a5be03f09 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 d6baddbe45..5e783489bb 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 a66a868667..6cb57a8560 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 1e6a912651..9a897789b4 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 3a948a0651..6b543805ea 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 cc3b563d0f..6c48b36bd1 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 02da1e84b2..44f6aeebe6 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 204c67ba52..46ace25fd7 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 2bf80a2090..f876158b67 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 33fd350cb6..b81a749837 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 5daeef8181..78c920ec44 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 9638722378..1a125dc260 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 9f80e7329c..041231cac3 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 f281438e12..4a6feaaf5c 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -18,26 +18,24 @@ 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, } from 'shared/ReactErrorUtils'; // Keep in sync with ReactDOM.js, and ReactTestUtilsAct.js: -const [ - getInstanceFromNode, - /* eslint-disable no-unused-vars */ - getNodeFromInstance, - getFiberCurrentPropsFromNode, - /* eslint-enable no-unused-vars */ - enqueueStateRestore, - restoreStateIfNeeded, - /* eslint-disable no-unused-vars */ - flushPassiveEffects, - IsThisRendererActing, - /* eslint-enable no-unused-vars */ -] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; +const EventInternals = + ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; +const getInstanceFromNode = EventInternals[0]; +const getNodeFromInstance = EventInternals[1]; +const getFiberCurrentPropsFromNode = EventInternals[2]; +const enqueueStateRestore = EventInternals[3]; +const restoreStateIfNeeded = EventInternals[4]; +// const flushPassiveEffects = EventInternals[5]; +// TODO: These are related to `act`, not events. Move to separate key? +// const IsThisRendererActing = EventInternals[6]; +const act = EventInternals[7]; function Event(suffix) {} @@ -728,4 +726,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 363826ca15..369981f18a 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -15,200 +15,129 @@ import enqueueTask from 'shared/enqueueTask'; import * as Scheduler from 'scheduler'; // Keep in sync with ReactDOM.js, and ReactTestUtils.js: -const [ - /* eslint-disable no-unused-vars */ - getInstanceFromNode, - getNodeFromInstance, - getFiberCurrentPropsFromNode, - enqueueStateRestore, - restoreStateIfNeeded, - /* eslint-enable no-unused-vars */ - flushPassiveEffects, - IsThisRendererActing, -] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; +const EventInternals = + ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; +// const getInstanceFromNode = EventInternals[0]; +// const getNodeFromInstance = EventInternals[1]; +// const getFiberCurrentPropsFromNode = EventInternals[2]; +// const enqueueStateRestore = EventInternals[3]; +// const restoreStateIfNeeded = EventInternals[4]; +// const flushPassiveEffects = EventInternals[5]; +const IsThisRendererActing = EventInternals[6]; 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: () => Thenable | 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, - ); - } - } - - // 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 - flushWork(); - } - 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.', + }, + error => { + unwind(); + reject(error); + }, ); - } - resolve(); - }, - }; + }, + }; + } 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; } } -export default act; +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); + } + }); +} 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 1b9c47224a..f1e8f57ac0 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 cf9552fb2f..f04c5479da 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -16,7 +16,7 @@ import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; import type {UpdateQueue} from 'react-reconciler/src/ReactUpdateQueue'; -import type {ReactNodeList} from 'shared/ReactTypes'; +import type {ReactNodeList, Thenable} from 'shared/ReactTypes'; import type {RootTag} from 'react-reconciler/src/ReactRootTags'; import * as Scheduler from 'scheduler/unstable_mock'; @@ -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,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { flushPassiveEffects: NoopRenderer.flushPassiveEffects, - act: NoopRenderer.act, + act: noopAct, // Logs the current state of the tree. dumpTree(rootID: string = DEFAULT_ROOT_ID) { @@ -1073,6 +1077,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: () => Thenable | 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: () => void, reject: (error: mixed) => void) { + 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 1e386c3309..eb835a6850 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 e1cccf912c..49de7469ed 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 606a902520..f73ae2f899 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 4c2a39718c..bc37abbd32 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 ca5d786f3f..6fad734ad5 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 e806070e9c..54806dba47 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 b6ed0d2be4..13d38e992c 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 9fc02c7bdc..55f8d3617c 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 d21f955bc9..2cd6881c94 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 4c3c932c90..3b6f69b13e 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 4f3709a677..f3538670e3 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 ffd0eaaf09..616e38faaa 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -7,6 +7,7 @@ * @flow */ +import type {Thenable} from 'shared/ReactTypes'; import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes'; import type {Instance, TextInstance} from './ReactTestHostConfig'; @@ -20,6 +21,7 @@ import { injectIntoDevTools, batchedUpdates, act, + IsThisRendererActing, } from 'react-reconciler/src/ReactFiberReconciler'; import {findCurrentFiberUsingSlowPath} from 'react-reconciler/src/ReactFiberTreeReflection'; import { @@ -44,10 +46,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 +587,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: () => Thenable | 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: () => void, reject: (error: mixed) => void) { + 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 756f1f5467..7109a7270a 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 72aa615260..ecd71cea82 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 66f548f9c3..e2c6bbe8f5 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 193f3d1ac7..c6022c9253 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 52e15fd428..6e0a9e5045 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;