Skip to content

Commit

Permalink
warn if you try to use act() in prod (#16282)
Browse files Browse the repository at this point in the history
We have behaviour divergence for act() between prod and dev (specifically, act() + concurrent mode does not flush fallbacks in prod. This doesn't affect anyone in OSS yet)

We also don't have a good story for writing tests in prod (and what from what I gather, nobody really writes tests in prod mode).

We could have wiped out act() in prod builds, except that _we_ ourselves use act() for our tests when we run them in prod mode.

This PR is a compromise to all of this. We will log a warning if you try to use act() in prod mode, and we silence it in our test suites.
  • Loading branch information
threepointone authored and acdlite committed Aug 5, 2019
1 parent dc232e6 commit a1dbb85
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 2 deletions.
25 changes: 23 additions & 2 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function sleep(period) {

describe('ReactTestUtils.act()', () => {
// first we run all the tests with concurrent mode
let concurrentRoot;
let concurrentRoot = null;
function renderConcurrent(el, dom) {
concurrentRoot = ReactDOM.unstable_createRoot(dom);
concurrentRoot.render(el);
Expand Down Expand Up @@ -71,7 +71,7 @@ describe('ReactTestUtils.act()', () => {
runActTests('legacy sync mode', renderSync, unmountSync, rerenderSync);

// and then in batched mode
let batchedRoot;
let batchedRoot = null;
function renderBatched(el, dom) {
batchedRoot = ReactDOM.unstable_createSyncRoot(dom);
batchedRoot.render(el);
Expand Down Expand Up @@ -791,5 +791,26 @@ function runActTests(label, render, unmount, rerender) {
});
}
});
describe('warn in prod mode', () => {
it('warns if you try to use act() in prod mode', () => {
const spy = spyOnDevAndProd(console, 'error');

act(() => {});

if (!__DEV__) {
expect(console.error).toHaveBeenCalledTimes(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'act(...) is not supported in production builds of React',
);
} else {
expect(console.error).toHaveBeenCalledTimes(0);
}

spy.calls.reset();
// does not warn twice
act(() => {});
expect(console.error).toHaveBeenCalledTimes(0);
});
});
});
}
9 changes: 9 additions & 0 deletions packages/react-dom/src/test-utils/ReactTestUtilsAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,17 @@ function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
// so we can tell if any async act() calls try to run in parallel.

let actingUpdatesScopeDepth = 0;
let didWarnAboutUsingActInProd = false;

function act(callback: () => Thenable) {
if (!__DEV__) {
if (didWarnAboutUsingActInProd === false) {
didWarnAboutUsingActInProd = true;
console.error(
'act(...) is not supported in production builds of React, and might not behave as expected.',
);
}
}
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousIsSomeRendererActing;
let previousIsThisRendererActing;
Expand Down
9 changes: 9 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,17 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
// so we can tell if any async act() calls try to run in parallel.

let actingUpdatesScopeDepth = 0;
let didWarnAboutUsingActInProd = false;

function act(callback: () => Thenable) {
if (!__DEV__) {
if (didWarnAboutUsingActInProd === false) {
didWarnAboutUsingActInProd = true;
console.error(
'act(...) is not supported in production builds of React, and might not behave as expected.',
);
}
}
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousIsSomeRendererActing;
let previousIsThisRendererActing;
Expand Down
9 changes: 9 additions & 0 deletions packages/react-test-renderer/src/ReactTestRendererAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,17 @@ function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
// so we can tell if any async act() calls try to run in parallel.

let actingUpdatesScopeDepth = 0;
let didWarnAboutUsingActInProd = false;

function act(callback: () => Thenable) {
if (!__DEV__) {
if (didWarnAboutUsingActInProd === false) {
didWarnAboutUsingActInProd = true;
console.error(
'act(...) is not supported in production builds of React, and might not behave as expected.',
);
}
}
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousIsSomeRendererActing;
let previousIsThisRendererActing;
Expand Down
10 changes: 10 additions & 0 deletions scripts/jest/shouldIgnoreConsoleError.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ module.exports = function shouldIgnoreConsoleError(format, args) {
// They are noisy too so we'll try to ignore them.
return true;
}
if (
format.indexOf(
'act(...) is not supported in production builds of React'
) === 0
) {
// We don't yet support act() for prod builds, and warn for it.
// But we'd like to use act() ourselves for prod builds.
// Let's ignore the warning and #yolo.
return true;
}
}
// Looks legit
return false;
Expand Down

0 comments on commit a1dbb85

Please sign in to comment.