Skip to content

Commit

Permalink
Remove internal act builds from public modules (#21721)
Browse files Browse the repository at this point in the history
* Move internal version of act to shared module

No reason to have three different copies of this anymore.

I've left the the renderer-specific `act` entry points because legacy
mode tests need to also be wrapped in `batchedUpdates`. Next, I'll update
the tests to use `batchedUpdates` manually when needed.

* Migrates tests to use internal module directly

Instead of the `unstable_concurrentAct` exports. Now we can drop those
from the public builds.

I put it in the jest-react package since that's where we put our other
testing utilities (like `toFlushAndYield`). Not so much so it can be
consumed publicly (nobody uses that package except us), but so it works
with our build tests.

* Remove unused internal fields

These were used by the old act implementation. No longer needed.
  • Loading branch information
acdlite authored Jun 22, 2021
1 parent 06f7b4f commit d7dce57
Show file tree
Hide file tree
Showing 81 changed files with 574 additions and 842 deletions.
2 changes: 2 additions & 0 deletions packages/jest-react/src/JestReact.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';
import invariant from 'shared/invariant';
import isArray from 'shared/isArray';

export {act} from './internalAct';

function captureAssertion(fn) {
// Trick to use a Jest matcher inside another Jest matcher. `fn` contains an
// assertion; if it throws, we capture the error and return it, so the stack
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,27 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
* @flow strict
*/

import type {Thenable} from 'shared/ReactTypes';

import * as ReactDOM from 'react-dom';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import enqueueTask from 'shared/enqueueTask';
import * as Scheduler from 'scheduler';

const SecretInternals =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;
const IsThisRendererActing = SecretInternals.IsThisRendererActing;

const batchedUpdates = ReactDOM.unstable_batchedUpdates;

const {IsSomeRendererActing, ReactCurrentActQueue} = ReactSharedInternals;

// This version of `act` is only used by our tests. Unlike the public version
// of `act`, it's designed to work identically in both production and
// 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: Replace the internal "concurrent" implementations of `act` with a
// single shared module.

import type {Thenable} from 'shared/ReactTypes';

import * as Scheduler from 'scheduler/unstable_mock';

import ReactSharedInternals from 'shared/ReactSharedInternals';
import enqueueTask from 'shared/enqueueTask';
const {ReactCurrentActQueue} = ReactSharedInternals;

let actingUpdatesScopeDepth = 0;

export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
export function act(scope: () => Thenable<mixed> | void) {
if (Scheduler.unstable_flushAllWithoutAsserting === undefined) {
throw Error(
'This version of `act` requires a special mock build of Scheduler.',
Expand All @@ -47,10 +38,6 @@ export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
}

const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
const previousIsSomeRendererActing = IsSomeRendererActing.current;
const previousIsThisRendererActing = IsThisRendererActing.current;
IsSomeRendererActing.current = true;
IsThisRendererActing.current = true;
actingUpdatesScopeDepth++;
if (__DEV__ && actingUpdatesScopeDepth === 1) {
ReactCurrentActQueue.disableActWarning = true;
Expand All @@ -61,8 +48,6 @@ export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
ReactCurrentActQueue.disableActWarning = false;
}
actingUpdatesScopeDepth--;
IsSomeRendererActing.current = previousIsSomeRendererActing;
IsThisRendererActing.current = previousIsThisRendererActing;

if (__DEV__) {
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
Expand All @@ -80,7 +65,7 @@ export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
// 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);
const thenable = scope();
if (
typeof thenable === 'object' &&
thenable !== null &&
Expand Down
2 changes: 1 addition & 1 deletion packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('ReactFlight', () => {
ReactNoop = require('react-noop-renderer');
ReactNoopFlightServer = require('react-noop-renderer/flight-server');
ReactNoopFlightClient = require('react-noop-renderer/flight-client');
act = ReactNoop.act;
act = require('jest-react').act;

ErrorBoundary = class extends React.Component {
state = {hasError: false, error: null};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('ReactHooksInspectionIntegration', () => {
React = require('react');
ReactTestRenderer = require('react-test-renderer');
Scheduler = require('scheduler');
act = ReactTestRenderer.unstable_concurrentAct;
act = require('jest-react').act;
ReactDebugTools = require('react-debug-tools');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ describe('InspectedElement', () => {
let InspectedElementContext;
let InspectedElementContextController;
let StoreContext;
let TestUtils;
let TreeContextController;

let TestUtilsAct;
Expand All @@ -45,10 +44,9 @@ describe('InspectedElement', () => {
React = require('react');
ReactDOM = require('react-dom');
PropTypes = require('prop-types');
TestUtils = require('react-dom/test-utils');
TestUtilsAct = TestUtils.unstable_concurrentAct;
TestUtilsAct = require('jest-react').act;
TestRenderer = utils.requireTestRenderer();
TestRendererAct = TestUtils.unstable_concurrentAct;
TestRendererAct = require('jest-react').act;

BridgeContext = require('react-devtools-shared/src/devtools/views/context')
.BridgeContext;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ import type Store from 'react-devtools-shared/src/devtools/store';
describe('Store component filters', () => {
let React;
let ReactDOM;
let TestUtils;
let Types;
let bridge: FrontendBridge;
let store: Store;
let utils;
let internalAct;

const act = (callback: Function) => {
TestUtils.unstable_concurrentAct(() => {
internalAct(() => {
callback();
});
jest.runAllTimers(); // Flush Bridge operations
Expand All @@ -35,9 +35,9 @@ describe('Store component filters', () => {

React = require('react');
ReactDOM = require('react-dom');
TestUtils = require('react-dom/test-utils');
Types = require('react-devtools-shared/src/types');
utils = require('./utils');
internalAct = require('jest-react').act;
});

it('should throw if filters are updated while profiling', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('ReactDOMFiberAsync', () => {
container = document.createElement('div');
React = require('react');
ReactDOM = require('react-dom');
act = require('react-dom/test-utils').unstable_concurrentAct;
act = require('jest-react').act;
Scheduler = require('scheduler');

document.body.appendChild(container);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactDOMHooks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('ReactDOMHooks', () => {
React = require('react');
ReactDOM = require('react-dom');
Scheduler = require('scheduler');
act = require('react-dom/test-utils').unstable_concurrentAct;
act = require('jest-react').act;

container = document.createElement('div');
document.body.appendChild(container);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
React = require('react');
ReactDOM = require('react-dom');
Scheduler = require('scheduler');
act = require('react-dom/test-utils').unstable_concurrentAct;
act = require('jest-react').act;

document.body.appendChild(container);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ describe('ReactDOMNestedEvents', () => {
let React;
let ReactDOM;
let Scheduler;
let TestUtils;
let act;
let useState;

Expand All @@ -22,8 +21,7 @@ describe('ReactDOMNestedEvents', () => {
React = require('react');
ReactDOM = require('react-dom');
Scheduler = require('scheduler');
TestUtils = require('react-dom/test-utils');
act = TestUtils.unstable_concurrentAct;
act = require('jest-react').act;
useState = React.useState;
});

Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('ReactDOMRoot', () => {
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
Scheduler = require('scheduler');
act = require('react-dom/test-utils').unstable_concurrentAct;
act = require('jest-react').act;
});

it('renders children', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function initModules() {
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
Scheduler = require('scheduler');
act = ReactTestUtils.unstable_concurrentAct;
act = require('jest-react').act;
useState = React.useState;
useReducer = React.useReducer;
useEffect = React.useEffect;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('ReactDOMServerPartialHydration', () => {

React = require('react');
ReactDOM = require('react-dom');
act = require('react-dom/test-utils').unstable_concurrentAct;
act = require('jest-react').act;
ReactDOMServer = require('react-dom/server');
Scheduler = require('scheduler');
Suspense = React.Suspense;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {createEventTarget} from 'dom-event-testing-library';
let React;
let ReactDOM;
let ReactDOMServer;
let ReactTestUtils;
let Scheduler;
let Suspense;
let act;
Expand Down Expand Up @@ -118,8 +117,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
act = ReactTestUtils.unstable_concurrentAct;
act = require('jest-react').act;
Scheduler = require('scheduler');
Suspense = React.Suspense;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function initModules() {
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
act = ReactTestUtils.unstable_concurrentAct;
act = require('jest-react').act;

// Make them available to the helpers.
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ let React;
let ReactDOM;
let Suspense;
let ReactCache;
let ReactTestUtils;
let Scheduler;
let TextResource;
let act;
Expand All @@ -26,9 +25,8 @@ describe('ReactDOMSuspensePlaceholder', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactCache = require('react-cache');
ReactTestUtils = require('react-dom/test-utils');
Scheduler = require('scheduler');
act = ReactTestUtils.unstable_concurrentAct;
act = require('jest-react').act;
Suspense = React.Suspense;
container = document.createElement('div');
document.body.appendChild(container);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('ReactErrorBoundaries', () => {
ReactFeatureFlags.skipUnmountedBoundaries = true;
ReactDOM = require('react-dom');
React = require('react');
act = require('react-dom/test-utils').unstable_concurrentAct;
act = require('jest-react').act;
Scheduler = require('scheduler');

BrokenConstructor = class extends React.Component {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactUpdates-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('ReactUpdates', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactTestUtils = require('react-dom/test-utils');
act = ReactTestUtils.unstable_concurrentAct;
act = require('jest-react').act;
Scheduler = require('scheduler');
});

Expand Down
12 changes: 7 additions & 5 deletions packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
let React;
let ReactNoop;
let Scheduler;
let act;
let Suspense;
let SuspenseList;
let getCacheForType;
Expand All @@ -20,6 +21,7 @@ beforeEach(() => {
React = require('react');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');
act = require('jest-react').act;

Suspense = React.Suspense;
SuspenseList = React.SuspenseList;
Expand Down Expand Up @@ -180,12 +182,12 @@ test('warns in DEV if return pointer is inconsistent', async () => {
}

const root = ReactNoop.createRoot();
await ReactNoop.act(async () => {
await act(async () => {
root.render(<App text="A" />);
});

spyOnDev(console, 'error');
await ReactNoop.act(async () => {
await act(async () => {
root.render(<App text="B" />);
});
expect(console.error.calls.count()).toBe(1);
Expand Down Expand Up @@ -225,19 +227,19 @@ test('regression (#20932): return pointer is correct before entering deleted tre
}

const root = ReactNoop.createRoot();
await ReactNoop.act(async () => {
await act(async () => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded([
'Suspend! [0]',
'Loading Async...',
'Loading Tail...',
]);
await ReactNoop.act(async () => {
await act(async () => {
resolveText(0);
});
expect(Scheduler).toHaveYielded([0, 'Tail']);
await ReactNoop.act(async () => {
await act(async () => {
setAsyncText(x => x + 1);
});
expect(Scheduler).toHaveYielded([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ const shouldIgnoreConsoleError = require('../../../../../scripts/jest/shouldIgno
module.exports = function(initModules) {
let ReactDOM;
let ReactDOMServer;
let ReactTestUtils;
let act;

function resetModules() {
({ReactDOM, ReactDOMServer, ReactTestUtils} = initModules());
({ReactDOM, ReactDOMServer} = initModules());
act = require('jest-react').act;
}

function shouldUseDocument(reactElement) {
Expand Down Expand Up @@ -50,11 +51,11 @@ module.exports = function(initModules) {
function asyncReactDOMRender(reactElement, domElement, forceHydrate) {
return new Promise(resolve => {
if (forceHydrate) {
ReactTestUtils.unstable_concurrentAct(() => {
act(() => {
ReactDOM.hydrate(reactElement, domElement);
});
} else {
ReactTestUtils.unstable_concurrentAct(() => {
act(() => {
ReactDOM.render(reactElement, domElement);
});
}
Expand Down
3 changes: 0 additions & 3 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
flushSync,
flushControlled,
injectIntoDevTools,
IsThisRendererActing,
attemptSynchronousHydration,
attemptDiscreteHydration,
attemptContinuousHydration,
Expand Down Expand Up @@ -164,8 +163,6 @@ const Internals = {
restoreStateIfNeeded,
batchedUpdates,
],
// TODO: Temporary. Only used by our internal version of `act. Will remove.
IsThisRendererActing,
};

export {
Expand Down
Loading

0 comments on commit d7dce57

Please sign in to comment.