Skip to content

Commit

Permalink
Use IS_REACT_ACT_ENVIRONMENT to disable warnings
Browse files Browse the repository at this point in the history
If `IS_REACT_ACT_ENVIRONMENT` is set to `false`, we will suppress
any `act` warnings. Otherwise, the behavior of `act` is the same as in
React 17: if `jest` is defined, it warns.

In concurrent mode, the plan is to remove the `jest` check and only warn
if `IS_REACT_ACT_ENVIRONMENT` is true. I have not implemented that
part yet.
  • Loading branch information
acdlite committed Oct 14, 2021
1 parent 34c3c35 commit d5cc3c6
Show file tree
Hide file tree
Showing 12 changed files with 90 additions and 22 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,5 +279,6 @@ module.exports = {
__VARIANT__: true,
gate: true,
trustedTypes: true,
IS_REACT_ACT_ENVIRONMENT: true,
},
};
9 changes: 5 additions & 4 deletions packages/jest-react/src/internalAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ 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;

Expand All @@ -37,15 +35,18 @@ export function act(scope: () => Thenable<mixed> | void) {
);
}

const previousIsActEnvironment = global.IS_REACT_ACT_ENVIRONMENT;
const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
if (__DEV__ && actingUpdatesScopeDepth === 1) {
ReactCurrentActQueue.disableActWarning = true;
// Because this is not the "real" `act`, we set this to `false` so React
// knows not to fire `act` warnings.
global.IS_REACT_ACT_ENVIRONMENT = false;
}

const unwind = () => {
if (__DEV__ && actingUpdatesScopeDepth === 1) {
ReactCurrentActQueue.disableActWarning = false;
global.IS_REACT_ACT_ENVIRONMENT = previousIsActEnvironment;
}
actingUpdatesScopeDepth--;

Expand Down
34 changes: 34 additions & 0 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,40 @@ function runActTests(label, render, unmount, rerender) {
]);
});

// @gate __DEV__
it('does not warn if IS_REACT_ACT_ENVIRONMENT is set to false', () => {
let setState;
function App() {
const [state, _setState] = React.useState(0);
setState = _setState;
return state;
}

act(() => {
render(<App />, container);
});

// First show that it does warn
expect(() => setState(1)).toErrorDev(
'An update to App inside a test was not wrapped in act(...)',
);

// Now do the same thing again, but disable with the environment flag
const prevIsActEnvironment = global.IS_REACT_ACT_ENVIRONMENT;
global.IS_REACT_ACT_ENVIRONMENT = false;
try {
setState(2);
} finally {
global.IS_REACT_ACT_ENVIRONMENT = prevIsActEnvironment;
}

// When the flag is restored to its previous value, it should start
// warning again. This shows that React reads the flag each time.
expect(() => setState(3)).toErrorDev(
'An update to App inside a test was not wrapped in act(...)',
);
});

describe('fake timers', () => {
beforeEach(() => {
jest.useFakeTimers();
Expand Down
22 changes: 16 additions & 6 deletions packages/react-reconciler/src/ReactFiberAct.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,23 @@
import type {Fiber} from './ReactFiber.new';
import {warnsIfNotActing} from './ReactFiberHostConfig';

import ReactSharedInternals from 'shared/ReactSharedInternals';

const {ReactCurrentActQueue} = ReactSharedInternals;

export function isActEnvironment(fiber: Fiber) {
const disableActWarning = ReactCurrentActQueue.disableActWarning;
const isReactActEnvironmentGlobal =
// $FlowExpectedError – Flow doesn't know about IS_REACT_ACT_ENVIRONMENT global
typeof IS_REACT_ACT_ENVIRONMENT !== 'undefined'
? IS_REACT_ACT_ENVIRONMENT
: undefined;

// TODO: Only check `jest` in legacy mode. In concurrent mode, this heuristic
// is repalced by IS_REACT_ACT_ENVIRONMENT.
// $FlowExpectedError - Flow doesn't know about jest
const jestIsDefined = typeof jest !== 'undefined';
return warnsIfNotActing && jestIsDefined && !disableActWarning;
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
);
}
22 changes: 16 additions & 6 deletions packages/react-reconciler/src/ReactFiberAct.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,23 @@
import type {Fiber} from './ReactFiber.old';
import {warnsIfNotActing} from './ReactFiberHostConfig';

import ReactSharedInternals from 'shared/ReactSharedInternals';

const {ReactCurrentActQueue} = ReactSharedInternals;

export function isActEnvironment(fiber: Fiber) {
const disableActWarning = ReactCurrentActQueue.disableActWarning;
const isReactActEnvironmentGlobal =
// $FlowExpectedError – Flow doesn't know about IS_REACT_ACT_ENVIRONMENT global
typeof IS_REACT_ACT_ENVIRONMENT !== 'undefined'
? IS_REACT_ACT_ENVIRONMENT
: undefined;

// TODO: Only check `jest` in legacy mode. In concurrent mode, this heuristic
// is repalced by IS_REACT_ACT_ENVIRONMENT.
// $FlowExpectedError - Flow doesn't know about jest
const jestIsDefined = typeof jest !== 'undefined';
return warnsIfNotActing && jestIsDefined && !disableActWarning;
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
);
}
6 changes: 0 additions & 6 deletions packages/react/src/ReactCurrentActQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ type RendererTask = boolean => RendererTask | null;

const ReactCurrentActQueue = {
current: (null: null | Array<RendererTask>),
// Our internal tests use a custom implementation of `act` that works by
// mocking the Scheduler package. Use this field to disable the `act` warning.
// TODO: Maybe the warning should be disabled by default, and then turned
// on at the testing frameworks layer? Instead of what we do now, which
// is check if a `jest` global is defined.
disableActWarning: (false: boolean),

// Used to reproduce behavior of `batchedUpdates` in legacy mode.
isBatchingLegacy: false,
Expand Down
3 changes: 3 additions & 0 deletions scripts/rollup/validate/eslintrc.cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ module.exports = {
// jest
expect: true,
jest: true,

// act
IS_REACT_ACT_ENVIRONMENT: true,
},
parserOptions: {
ecmaVersion: 5,
Expand Down
3 changes: 3 additions & 0 deletions scripts/rollup/validate/eslintrc.cjs2015.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ module.exports = {
// jest
expect: true,
jest: true,

// act
IS_REACT_ACT_ENVIRONMENT: true,
},
parserOptions: {
ecmaVersion: 2015,
Expand Down
3 changes: 3 additions & 0 deletions scripts/rollup/validate/eslintrc.esm.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ module.exports = {
// jest
expect: true,
jest: true,

// act
IS_REACT_ACT_ENVIRONMENT: true,
},
parserOptions: {
ecmaVersion: 2017,
Expand Down
3 changes: 3 additions & 0 deletions scripts/rollup/validate/eslintrc.fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ module.exports = {

// jest
jest: true,

// act
IS_REACT_ACT_ENVIRONMENT: true,
},
parserOptions: {
ecmaVersion: 5,
Expand Down
3 changes: 3 additions & 0 deletions scripts/rollup/validate/eslintrc.rn.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ module.exports = {

// jest
jest: true,

// act
IS_REACT_ACT_ENVIRONMENT: true,
},
parserOptions: {
ecmaVersion: 5,
Expand Down
3 changes: 3 additions & 0 deletions scripts/rollup/validate/eslintrc.umd.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ module.exports = {

// jest
jest: true,

// act
IS_REACT_ACT_ENVIRONMENT: true,
},
parserOptions: {
ecmaVersion: 5,
Expand Down

0 comments on commit d5cc3c6

Please sign in to comment.