Skip to content

Commit

Permalink
Replace global jest heuristic with IS_REACT_ACT_ENVIRONMENT (face…
Browse files Browse the repository at this point in the history
…book#22562)

* Remove `jest` global check in concurrent roots

In concurrent mode, instead of checking `jest`, we check the new
`IS_REACT_ACT_ENVIRONMENT` global. The default behavior is `false`.

Legacy mode behavior is unchanged.

React's own internal test suite use a custom version of `act` that works
by mocking the Scheduler — rather than the "real" act used publicly. So
we don't enable the flag in our repo.

* Warn if `act` is called in wrong environment

Adds a warning if `act` is called but `IS_REACT_ACT_ENVIRONMENT` is not
enabled. The goal is to prompt users to correctly configure their
testing environment, so that if they forget to use `act` in a different
test, we can detect and warn about.

It's expected that the environment flag will be configured by the
testing framework. For example, a Jest plugin. We will link to the
relevant documentation page, once it exists.

The warning only fires in concurrent mode. Legacy roots will keep the
existing behavior.
  • Loading branch information
acdlite authored and KamranAsif committed Nov 4, 2021
1 parent b81e99b commit 9e456c6
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {
} from '../../constants';
import REACT_VERSION from 'shared/ReactVersion';

global.IS_REACT_ACT_ENVIRONMENT = true;

describe('getLanesFromTransportDecimalBitmask', () => {
it('should return array of lane numbers from bitmask string', () => {
expect(getLanesFromTransportDecimalBitmask('1')).toEqual([0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ describe('InspectedElement', () => {
let ErrorBoundary;
let errorBoundaryInstance;

global.IS_REACT_ACT_ENVIRONMENT = true;

beforeEach(() => {
utils = require('./utils');
utils.beforeEachProfiling();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
// Dispatch a click event on the Disable-button.
const firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
expect(() =>
dispatchAndSetCurrentEvent(disableButton, firstEvent),
).toErrorDev(['An update to Form inside a test was not wrapped in act']);
dispatchAndSetCurrentEvent(disableButton, firstEvent);

// Discrete events should be flushed in a microtask.
// Verify that the second button was removed.
Expand Down Expand Up @@ -134,9 +132,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
// Dispatch a click event on the Disable-button.
const firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
expect(() => {
dispatchAndSetCurrentEvent(disableButton, firstEvent);
}).toErrorDev(['An update to Form inside a test was not wrapped in act']);
dispatchAndSetCurrentEvent(disableButton, firstEvent);

// There should now be a pending update to disable the form.
// This should not have flushed yet since it's in concurrent mode.
Expand Down Expand Up @@ -196,9 +192,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
// Dispatch a click event on the Enable-button.
const firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
expect(() => {
dispatchAndSetCurrentEvent(enableButton, firstEvent);
}).toErrorDev(['An update to Form inside a test was not wrapped in act']);
dispatchAndSetCurrentEvent(enableButton, firstEvent);

// There should now be a pending update to enable the form.
// This should not have flushed yet since it's in concurrent mode.
Expand Down Expand Up @@ -344,9 +338,6 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
});
expect(container.textContent).toEqual('Count: 0');

// Ignore act warning. We can't use act because it forces batched updates.
spyOnDev(console, 'error');

const pressEvent = document.createEvent('Event');
pressEvent.initEvent('click', true, true);
dispatchAndSetCurrentEvent(target.current, pressEvent);
Expand All @@ -355,17 +346,6 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
await null;
// If this is 2, that means the `setCount` calls were not batched.
expect(container.textContent).toEqual('Count: 1');

// Assert that the `act` warnings were the only ones that fired.
if (__DEV__) {
expect(console.error).toHaveBeenCalledTimes(2);
expect(console.error.calls.argsFor(0)[0]).toContain(
'was not wrapped in act',
);
expect(console.error.calls.argsFor(1)[0]).toContain(
'was not wrapped in act',
);
}
});

it('should not flush discrete events at the end of outermost batchedUpdates', async () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ let container;

jest.useRealTimers();

global.IS_REACT_ACT_ENVIRONMENT = true;

function sleep(period) {
return new Promise(resolve => {
setTimeout(() => {
Expand Down
43 changes: 31 additions & 12 deletions packages/react-reconciler/src/ReactFiberAct.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
*/

import type {Fiber} from './ReactFiber.new';

import ReactSharedInternals from 'shared/ReactSharedInternals';

import {warnsIfNotActing} from './ReactFiberHostConfig';
import {ConcurrentMode} from './ReactTypeOfMode';

const {ReactCurrentActQueue} = ReactSharedInternals;

export function isActEnvironment(fiber: Fiber) {
if (__DEV__) {
Expand All @@ -18,18 +24,31 @@ export function isActEnvironment(fiber: Fiber) {
? IS_REACT_ACT_ENVIRONMENT
: undefined;

// TODO: Only check `jest` in legacy mode. In concurrent mode, this
// heuristic is replaced by IS_REACT_ACT_ENVIRONMENT.
// $FlowExpectedError - Flow doesn't know about jest
const jestIsDefined = typeof jest !== 'undefined';
return (
warnsIfNotActing &&
jestIsDefined &&
// Legacy mode assumes an act environment whenever `jest` is defined, but
// you can still turn off spurious warnings by setting
// IS_REACT_ACT_ENVIRONMENT explicitly to false.
isReactActEnvironmentGlobal !== false
);
if (fiber.mode & ConcurrentMode) {
if (
!isReactActEnvironmentGlobal &&
ReactCurrentActQueue.current !== null
) {
// TODO: Include link to relevant documentation page.
console.error(
'The current testing environment is not configured to support ' +
'act(...)',
);
}
return isReactActEnvironmentGlobal;
} else {
// Legacy mode. We preserve the behavior of React 17's act. It assumes an
// act environment whenever `jest` is defined, but you can still turn off
// spurious warnings by setting IS_REACT_ACT_ENVIRONMENT explicitly
// to false.
// $FlowExpectedError - Flow doesn't know about jest
const jestIsDefined = typeof jest !== 'undefined';
return (
warnsIfNotActing &&
jestIsDefined &&
isReactActEnvironmentGlobal !== false
);
}
}
return false;
}
43 changes: 31 additions & 12 deletions packages/react-reconciler/src/ReactFiberAct.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
*/

import type {Fiber} from './ReactFiber.old';

import ReactSharedInternals from 'shared/ReactSharedInternals';

import {warnsIfNotActing} from './ReactFiberHostConfig';
import {ConcurrentMode} from './ReactTypeOfMode';

const {ReactCurrentActQueue} = ReactSharedInternals;

export function isActEnvironment(fiber: Fiber) {
if (__DEV__) {
Expand All @@ -18,18 +24,31 @@ export function isActEnvironment(fiber: Fiber) {
? IS_REACT_ACT_ENVIRONMENT
: undefined;

// TODO: Only check `jest` in legacy mode. In concurrent mode, this
// heuristic is replaced by IS_REACT_ACT_ENVIRONMENT.
// $FlowExpectedError - Flow doesn't know about jest
const jestIsDefined = typeof jest !== 'undefined';
return (
warnsIfNotActing &&
jestIsDefined &&
// Legacy mode assumes an act environment whenever `jest` is defined, but
// you can still turn off spurious warnings by setting
// IS_REACT_ACT_ENVIRONMENT explicitly to false.
isReactActEnvironmentGlobal !== false
);
if (fiber.mode & ConcurrentMode) {
if (
!isReactActEnvironmentGlobal &&
ReactCurrentActQueue.current !== null
) {
// TODO: Include link to relevant documentation page.
console.error(
'The current testing environment is not configured to support ' +
'act(...)',
);
}
return isReactActEnvironmentGlobal;
} else {
// Legacy mode. We preserve the behavior of React 17's act. It assumes an
// act environment whenever `jest` is defined, but you can still turn off
// spurious warnings by setting IS_REACT_ACT_ENVIRONMENT explicitly
// to false.
// $FlowExpectedError - Flow doesn't know about jest
const jestIsDefined = typeof jest !== 'undefined';
return (
warnsIfNotActing &&
jestIsDefined &&
isReactActEnvironmentGlobal !== false
);
}
}
return false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ describe('DebugTracing', () => {
const DEFAULT_LANE_STRING = '0b0000000000000000000000000010000';
const RETRY_LANE_STRING = '0b0000000010000000000000000000000';

global.IS_REACT_ACT_ENVIRONMENT = true;

beforeEach(() => {
jest.resetModules();

Expand Down
130 changes: 130 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactActWarnings-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @jest-environment node
*/

let React;
let Scheduler;
let ReactNoop;
let useState;
let act;

// These tests are mostly concerned with concurrent roots. The legacy root
// behavior is covered by other older test suites and is unchanged from
// React 17.
describe('act warnings', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
Scheduler = require('scheduler');
ReactNoop = require('react-noop-renderer');
act = React.unstable_act;
useState = React.useState;
});

function Text(props) {
Scheduler.unstable_yieldValue(props.text);
return props.text;
}

function withActEnvironment(value, scope) {
const prevValue = global.IS_REACT_ACT_ENVIRONMENT;
global.IS_REACT_ACT_ENVIRONMENT = value;
try {
return scope();
} finally {
global.IS_REACT_ACT_ENVIRONMENT = prevValue;
}
}

test('warns about unwrapped updates only if environment flag is enabled', () => {
let setState;
function App() {
const [state, _setState] = useState(0);
setState = _setState;
return <Text text={state} />;
}

const root = ReactNoop.createRoot();
root.render(<App />);
expect(Scheduler).toFlushAndYield([0]);
expect(root).toMatchRenderedOutput('0');

// Default behavior. Flag is undefined. No warning.
expect(global.IS_REACT_ACT_ENVIRONMENT).toBe(undefined);
setState(1);
expect(Scheduler).toFlushAndYield([1]);
expect(root).toMatchRenderedOutput('1');

// Flag is true. Warn.
withActEnvironment(true, () => {
expect(() => setState(2)).toErrorDev(
'An update to App inside a test was not wrapped in act',
);
expect(Scheduler).toFlushAndYield([2]);
expect(root).toMatchRenderedOutput('2');
});

// Flag is false. No warning.
withActEnvironment(false, () => {
setState(3);
expect(Scheduler).toFlushAndYield([3]);
expect(root).toMatchRenderedOutput('3');
});
});

// @gate __DEV__
test('act warns if the environment flag is not enabled', () => {
let setState;
function App() {
const [state, _setState] = useState(0);
setState = _setState;
return <Text text={state} />;
}

const root = ReactNoop.createRoot();
root.render(<App />);
expect(Scheduler).toFlushAndYield([0]);
expect(root).toMatchRenderedOutput('0');

// Default behavior. Flag is undefined. Warn.
expect(global.IS_REACT_ACT_ENVIRONMENT).toBe(undefined);
expect(() => {
act(() => {
setState(1);
});
}).toErrorDev(
'The current testing environment is not configured to support act(...)',
{withoutStack: true},
);
expect(Scheduler).toHaveYielded([1]);
expect(root).toMatchRenderedOutput('1');

// Flag is true. Don't warn.
withActEnvironment(true, () => {
act(() => {
setState(2);
});
expect(Scheduler).toHaveYielded([2]);
expect(root).toMatchRenderedOutput('2');
});

// Flag is false. Warn.
withActEnvironment(false, () => {
expect(() => {
act(() => {
setState(1);
});
}).toErrorDev(
'The current testing environment is not configured to support act(...)',
{withoutStack: true},
);
expect(Scheduler).toHaveYielded([1]);
expect(root).toMatchRenderedOutput('1');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ describe('SchedulingProfiler labels', () => {
let featureDetectionMarkName = null;
let marks;

global.IS_REACT_ACT_ENVIRONMENT = true;

function polyfillJSDomUserTiming() {
featureDetectionMarkName = null;

Expand Down

0 comments on commit 9e456c6

Please sign in to comment.