Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support disabling spurious act warnings with a global environment flag #22561

Merged
merged 2 commits into from
Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
35 changes: 35 additions & 0 deletions packages/react-reconciler/src/ReactFiberAct.new.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* 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.
*
* @flow
*/

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

export function isActEnvironment(fiber: Fiber) {
if (__DEV__) {
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 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
);
}
return false;
}
35 changes: 35 additions & 0 deletions packages/react-reconciler/src/ReactFiberAct.old.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* 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.
*
* @flow
*/

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

export function isActEnvironment(fiber: Fiber) {
if (__DEV__) {
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 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
);
}
return false;
}
13 changes: 5 additions & 8 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ import {
} from './ReactUpdateQueue.new';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new';
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
import {isActEnvironment} from './ReactFiberAct.new';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -1678,8 +1679,7 @@ function mountEffect(
deps: Array<mixed> | void | null,
): void {
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(currentlyRenderingFiber)) {
warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
}
}
Expand Down Expand Up @@ -1709,8 +1709,7 @@ function updateEffect(
deps: Array<mixed> | void | null,
): void {
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(currentlyRenderingFiber)) {
warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
}
}
Expand Down Expand Up @@ -2193,8 +2192,7 @@ function dispatchReducerAction<S, A>(
enqueueUpdate(fiber, queue, update, lane);

if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(fiber)) {
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}
Expand Down Expand Up @@ -2279,8 +2277,7 @@ function dispatchSetState<S, A>(
}
}
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(fiber)) {
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}
Expand Down
13 changes: 5 additions & 8 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ import {
} from './ReactUpdateQueue.old';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.old';
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
import {isActEnvironment} from './ReactFiberAct.old';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -1678,8 +1679,7 @@ function mountEffect(
deps: Array<mixed> | void | null,
): void {
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(currentlyRenderingFiber)) {
warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
}
}
Expand Down Expand Up @@ -1709,8 +1709,7 @@ function updateEffect(
deps: Array<mixed> | void | null,
): void {
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(currentlyRenderingFiber)) {
warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
}
}
Expand Down Expand Up @@ -2193,8 +2192,7 @@ function dispatchReducerAction<S, A>(
enqueueUpdate(fiber, queue, update, lane);

if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(fiber)) {
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}
Expand Down Expand Up @@ -2279,8 +2277,7 @@ function dispatchSetState<S, A>(
}
}
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
if (isActEnvironment(fiber)) {
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}
Expand Down
19 changes: 2 additions & 17 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ import {
scheduleTimeout,
cancelTimeout,
noTimeout,
warnsIfNotActing,
afterActiveInstanceBlur,
clearContainer,
getCurrentEventPriority,
Expand Down Expand Up @@ -2816,15 +2815,8 @@ function shouldForceFlushFallbacksInDEV() {
export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
warnsIfNotActing === true &&
(fiber.mode & StrictLegacyMode) !== NoMode &&
ReactCurrentActQueue.current === null &&
// Our internal tests use a custom implementation of `act` that works by
// mocking the Scheduler package. 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.
ReactCurrentActQueue.disableActWarning === false
ReactCurrentActQueue.current === null
) {
console.error(
'An update to %s ran an effect, but was not wrapped in act(...).\n\n' +
Expand All @@ -2846,15 +2838,8 @@ export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
warnsIfNotActing === true &&
executionContext === NoContext &&
ReactCurrentActQueue.current === null &&
// Our internal tests use a custom implementation of `act` that works by
// mocking the Scheduler package. 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.
ReactCurrentActQueue.disableActWarning === false
ReactCurrentActQueue.current === null
) {
const previousFiber = ReactCurrentFiberCurrent;
try {
Expand Down
19 changes: 2 additions & 17 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ import {
scheduleTimeout,
cancelTimeout,
noTimeout,
warnsIfNotActing,
afterActiveInstanceBlur,
clearContainer,
getCurrentEventPriority,
Expand Down Expand Up @@ -2816,15 +2815,8 @@ function shouldForceFlushFallbacksInDEV() {
export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
warnsIfNotActing === true &&
(fiber.mode & StrictLegacyMode) !== NoMode &&
ReactCurrentActQueue.current === null &&
// Our internal tests use a custom implementation of `act` that works by
// mocking the Scheduler package. 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.
ReactCurrentActQueue.disableActWarning === false
ReactCurrentActQueue.current === null
) {
console.error(
'An update to %s ran an effect, but was not wrapped in act(...).\n\n' +
Expand All @@ -2846,15 +2838,8 @@ export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
warnsIfNotActing === true &&
executionContext === NoContext &&
ReactCurrentActQueue.current === null &&
// Our internal tests use a custom implementation of `act` that works by
// mocking the Scheduler package. 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.
ReactCurrentActQueue.disableActWarning === false
ReactCurrentActQueue.current === null
) {
const previousFiber = ReactCurrentFiberCurrent;
try {
Expand Down
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
Loading