Skip to content

Commit

Permalink
Throw when act is used in production (#21686)
Browse files Browse the repository at this point in the history 
Upgrades the deprecation warning to a runtime error.

I did it this way instead of removing the export so the type is the same
in both builds. It will get dead code eliminated regardless.
  • Loading branch information
@acdlite
acdlite committed Jun 16, 2021
1 parent a0d2d1e commit bd0a963
Show file tree
Hide file tree
Showing 15 changed files with 87 additions and 92 deletions.
10 changes: 0 additions & 10 deletions fixtures/legacy-jsx-runtimes/setupTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,6 @@ 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
46 changes: 28 additions & 18 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ function runActTests(label, render, unmount, rerender) {
});

describe('sync', () => {
// @gate __DEV__
it('can use act to flush effects', () => {
function App() {
React.useEffect(() => {
Expand All @@ -150,6 +151,7 @@ function runActTests(label, render, unmount, rerender) {
expect(Scheduler).toHaveYielded([100]);
});

// @gate __DEV__
it('flushes effects on every call', async () => {
function App() {
const [ctr, setCtr] = React.useState(0);
Expand Down Expand Up @@ -186,6 +188,7 @@ function runActTests(label, render, unmount, rerender) {
expect(button.innerHTML).toBe('5');
});

// @gate __DEV__
it("should keep flushing effects until they're done", () => {
function App() {
const [ctr, setCtr] = React.useState(0);
Expand All @@ -204,6 +207,7 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toBe('5');
});

// @gate __DEV__
it('should flush effects only on exiting the outermost act', () => {
function App() {
React.useEffect(() => {
Expand All @@ -224,6 +228,7 @@ function runActTests(label, render, unmount, rerender) {
expect(Scheduler).toHaveYielded([0]);
});

// @gate __DEV__
it('warns if a setState is called outside of act(...)', () => {
let setValue = null;
function App() {
Expand All @@ -250,6 +255,7 @@ function runActTests(label, render, unmount, rerender) {
jest.useRealTimers();
});

// @gate __DEV__
it('lets a ticker update', () => {
function App() {
const [toggle, setToggle] = React.useState(0);
Expand All @@ -272,6 +278,7 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toBe('1');
});

// @gate __DEV__
it('can use the async version to catch microtasks', async () => {
function App() {
const [toggle, setToggle] = React.useState(0);
Expand All @@ -294,6 +301,7 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toBe('1');
});

// @gate __DEV__
it('can handle cascading promises with fake timers', async () => {
// this component triggers an effect, that waits a tick,
// then sets state. repeats this 5 times.
Expand All @@ -317,6 +325,7 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toBe('5');
});

// @gate __DEV__
it('flushes immediate re-renders with act', () => {
function App() {
const [ctr, setCtr] = React.useState(0);
Expand Down Expand Up @@ -346,6 +355,7 @@ function runActTests(label, render, unmount, rerender) {
});
});

// @gate __DEV__
it('warns if you return a value inside act', () => {
expect(() => act(() => null)).toErrorDev(
[
Expand All @@ -361,6 +371,7 @@ function runActTests(label, render, unmount, rerender) {
);
});

// @gate __DEV__
it('warns if you try to await a sync .act call', () => {
expect(() => act(() => {}).then(() => {})).toErrorDev(
[
Expand All @@ -372,6 +383,7 @@ function runActTests(label, render, unmount, rerender) {
});

describe('asynchronous tests', () => {
// @gate __DEV__
it('works with timeouts', async () => {
function App() {
const [ctr, setCtr] = React.useState(0);
Expand All @@ -396,6 +408,7 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toBe('1');
});

// @gate __DEV__
it('flushes microtasks before exiting', async () => {
function App() {
const [ctr, setCtr] = React.useState(0);
Expand All @@ -418,6 +431,7 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toEqual('1');
});

// @gate __DEV__
it('warns if you do not await an act call', async () => {
spyOnDevAndProd(console, 'error');
act(async () => {});
Expand All @@ -431,6 +445,7 @@ function runActTests(label, render, unmount, rerender) {
}
});

// @gate __DEV__
it('warns if you try to interleave multiple act calls', async () => {
spyOnDevAndProd(console, 'error');
// let's try to cheat and spin off a 'thread' with an act call
Expand All @@ -450,6 +465,7 @@ function runActTests(label, render, unmount, rerender) {
}
});

// @gate __DEV__
it('async commits and effects are guaranteed to be flushed', async () => {
function App() {
const [state, setState] = React.useState(0);
Expand All @@ -475,6 +491,7 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toBe('1');
});

// @gate __DEV__
it('can handle cascading promises', async () => {
// this component triggers an effect, that waits a tick,
// then sets state. repeats this 5 times.
Expand All @@ -501,6 +518,7 @@ function runActTests(label, render, unmount, rerender) {
});

describe('error propagation', () => {
// @gate __DEV__
it('propagates errors - sync', () => {
let err;
try {
Expand All @@ -515,6 +533,7 @@ function runActTests(label, render, unmount, rerender) {
}
});

// @gate __DEV__
it('should propagate errors from effects - sync', () => {
function App() {
React.useEffect(() => {
Expand All @@ -536,6 +555,7 @@ function runActTests(label, render, unmount, rerender) {
}
});

// @gate __DEV__
it('propagates errors - async', async () => {
let err;
try {
Expand All @@ -551,6 +571,7 @@ function runActTests(label, render, unmount, rerender) {
}
});

// @gate __DEV__
it('should cleanup after errors - sync', () => {
function App() {
React.useEffect(() => {
Expand All @@ -576,6 +597,7 @@ function runActTests(label, render, unmount, rerender) {
}
});

// @gate __DEV__
it('should cleanup after errors - async', async () => {
function App() {
async function somethingAsync() {
Expand Down Expand Up @@ -611,6 +633,7 @@ function runActTests(label, render, unmount, rerender) {
if (__DEV__ && __EXPERIMENTAL__) {
// todo - remove __DEV__ check once we start using testing builds

// @gate __DEV__
it('triggers fallbacks if available', async () => {
if (label !== 'legacy mode') {
// FIXME: Support for Concurrent Root intentionally removed
Expand Down Expand Up @@ -691,25 +714,12 @@ function runActTests(label, render, unmount, rerender) {
});
}
});
describe('warn in prod mode', () => {
describe('throw in prod mode', () => {
// @gate !__DEV__
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);
expect(() => act(() => {})).toThrow(
'act(...) is not supported in production builds of React',
);
});
});
});
Expand Down
5 changes: 5 additions & 0 deletions packages/react-dom/src/__tests__/ReactTestUtilsActUnmockedScheduler-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ afterEach(() => {
document.body.removeChild(container);
});

// @gate __DEV__
it('can use act to flush effects', () => {
function App() {
React.useEffect(() => {
Expand All @@ -62,6 +63,7 @@ it('can use act to flush effects', () => {
expect(clearYields()).toEqual([100]);
});

// @gate __DEV__
it('flushes effects on every call', () => {
function App() {
const [ctr, setCtr] = React.useState(0);
Expand Down Expand Up @@ -100,6 +102,7 @@ it('flushes effects on every call', () => {
expect(button.innerHTML).toEqual('5');
});

// @gate __DEV__
it("should keep flushing effects until they're done", () => {
function App() {
const [ctr, setCtr] = React.useState(0);
Expand All @@ -118,6 +121,7 @@ it("should keep flushing effects until they're done", () => {
expect(container.innerHTML).toEqual('5');
});

// @gate __DEV__
it('should flush effects only on exiting the outermost act', () => {
function App() {
React.useEffect(() => {
Expand All @@ -138,6 +142,7 @@ it('should flush effects only on exiting the outermost act', () => {
expect(clearYields()).toEqual([0]);
});

// @gate __DEV__
it('can handle cascading promises', async () => {
// this component triggers an effect, that waits a tick,
// then sets state. repeats this 5 times.
Expand Down
13 changes: 5 additions & 8 deletions packages/react-dom/src/test-utils/ReactTestUtilsPublicAct.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import * as ReactDOM from 'react-dom';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import enqueueTask from 'shared/enqueueTask';
import * as Scheduler from 'scheduler';
import invariant from 'shared/invariant';

// Keep in sync with ReactDOM.js, and ReactTestUtils.js:
const EventInternals =
Expand Down Expand Up @@ -71,17 +72,13 @@ 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;

export function act(callback: () => Thenable<mixed>): Thenable<void> {
if (!__DEV__) {
if (didWarnAboutUsingActInProd === false) {
didWarnAboutUsingActInProd = true;
// eslint-disable-next-line react-internal/no-production-logging
console.error(
'act(...) is not supported in production builds of React, and might not behave as expected.',
);
}
invariant(
false,
'act(...) is not supported in production builds of React.',
);
}
const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
Expand Down
12 changes: 4 additions & 8 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2985,17 +2985,13 @@ 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;

export function act(callback: () => Thenable<mixed>): Thenable<void> {
if (!__DEV__) {
if (didWarnAboutUsingActInProd === false) {
didWarnAboutUsingActInProd = true;
// eslint-disable-next-line react-internal/no-production-logging
console.error(
'act(...) is not supported in production builds of React, and might not behave as expected.',
);
}
invariant(
false,
'act(...) is not supported in production builds of React.',
);
}

const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
Expand Down
12 changes: 4 additions & 8 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2985,17 +2985,13 @@ 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;

export function act(callback: () => Thenable<mixed>): Thenable<void> {
if (!__DEV__) {
if (didWarnAboutUsingActInProd === false) {
didWarnAboutUsingActInProd = true;
// eslint-disable-next-line react-internal/no-production-logging
console.error(
'act(...) is not supported in production builds of React, and might not behave as expected.',
);
}
invariant(
false,
'act(...) is not supported in production builds of React.',
);
}

const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('ReactFiberHostContext', () => {
.DefaultEventPriority;
});

// @gate __DEV__
it('works with null host context', async () => {
let creates = 0;
const Renderer = ReactFiberReconciler({
Expand Down Expand Up @@ -83,6 +84,7 @@ describe('ReactFiberHostContext', () => {
expect(creates).toBe(2);
});

// @gate __DEV__
it('should send the context to prepareForCommit and resetAfterCommit', () => {
const rootContext = {};
const Renderer = ReactFiberReconciler({
Expand Down
Loading

0 comments on commit bd0a963

Please sign in to comment.