Skip to content

Commit 3400ee6

Browse files
committed
act should work without mock Scheduler
Currently, in a React 18 root, `act` only works if you mock the Scheduler package. This was because we didn't want to add additional checks at runtime. But now that the `act` testing API is dev-only, we can simplify its implementation. Now when an update is wrapped with `act`, React will bypass Scheduler entirely and push its tasks onto a special internal queue. Then, when the outermost `act` scope exists, we'll flush that queue. I also removed the "wrong act" warning, because the plan is to move `act` to an isomorphic entry point, simlar to `startTransition`. That's not directly related to this PR, but I didn't want to bother re-implementing that warning only to immediately remove it. I'll add the isomorphic API in a follow up. Note that the internal version of `act` that we use in our own tests still depends on mocking the Scheduler package, because it needs to work in production. I'm planning to move that implementation to a shared (internal) module, too.
1 parent b5debd5 commit 3400ee6

25 files changed

+470
-713
lines changed

packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -354,32 +354,6 @@ function runActTests(label, render, unmount, rerender) {
354354
expect(container.innerHTML).toBe('2');
355355
});
356356
});
357-
358-
// @gate __DEV__
359-
it('warns if you return a value inside act', () => {
360-
expect(() => act(() => null)).toErrorDev(
361-
[
362-
'The callback passed to act(...) function must return undefined, or a Promise.',
363-
],
364-
{withoutStack: true},
365-
);
366-
expect(() => act(() => 123)).toErrorDev(
367-
[
368-
'The callback passed to act(...) function must return undefined, or a Promise.',
369-
],
370-
{withoutStack: true},
371-
);
372-
});
373-
374-
// @gate __DEV__
375-
it('warns if you try to await a sync .act call', () => {
376-
expect(() => act(() => {}).then(() => {})).toErrorDev(
377-
[
378-
'Do not await the result of calling act(...) with sync logic, it is not a Promise.',
379-
],
380-
{withoutStack: true},
381-
);
382-
});
383357
});
384358

385359
describe('asynchronous tests', () => {
@@ -401,15 +375,17 @@ function runActTests(label, render, unmount, rerender) {
401375

402376
await act(async () => {
403377
render(<App />, container);
404-
// flush a little to start the timer
405-
expect(Scheduler).toFlushAndYield([]);
378+
});
379+
expect(container.innerHTML).toBe('0');
380+
// Flush the pending timers
381+
await act(async () => {
406382
await sleep(100);
407383
});
408384
expect(container.innerHTML).toBe('1');
409385
});
410386

411387
// @gate __DEV__
412-
it('flushes microtasks before exiting', async () => {
388+
it('flushes microtasks before exiting (async function)', async () => {
413389
function App() {
414390
const [ctr, setCtr] = React.useState(0);
415391
async function someAsyncFunction() {
@@ -431,6 +407,31 @@ function runActTests(label, render, unmount, rerender) {
431407
expect(container.innerHTML).toEqual('1');
432408
});
433409

410+
// @gate __DEV__
411+
it('flushes microtasks before exiting (sync function)', async () => {
412+
// Same as previous test, but the callback passed to `act` is not itself
413+
// an async function.
414+
function App() {
415+
const [ctr, setCtr] = React.useState(0);
416+
async function someAsyncFunction() {
417+
// queue a bunch of promises to be sure they all flush
418+
await null;
419+
await null;
420+
await null;
421+
setCtr(1);
422+
}
423+
React.useEffect(() => {
424+
someAsyncFunction();
425+
}, []);
426+
return ctr;
427+
}
428+
429+
await act(() => {
430+
render(<App />, container);
431+
});
432+
expect(container.innerHTML).toEqual('1');
433+
});
434+
434435
// @gate __DEV__
435436
it('warns if you do not await an act call', async () => {
436437
spyOnDevAndProd(console, 'error');
@@ -461,7 +462,13 @@ function runActTests(label, render, unmount, rerender) {
461462

462463
await sleep(150);
463464
if (__DEV__) {
464-
expect(console.error).toHaveBeenCalledTimes(1);
465+
expect(console.error).toHaveBeenCalledTimes(2);
466+
expect(console.error.calls.argsFor(0)[0]).toMatch(
467+
'You seem to have overlapping act() calls',
468+
);
469+
expect(console.error.calls.argsFor(1)[0]).toMatch(
470+
'You seem to have overlapping act() calls',
471+
);
465472
}
466473
});
467474

packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.internal.js

Lines changed: 0 additions & 56 deletions
This file was deleted.

packages/react-dom/src/__tests__/ReactUnmockedSchedulerWarning-test.js

Lines changed: 0 additions & 42 deletions
This file was deleted.

packages/react-dom/src/test-utils/ReactTestUtilsInternalAct.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@ const IsThisRendererActing = SecretInternals.IsThisRendererActing;
2020

2121
const batchedUpdates = ReactDOM.unstable_batchedUpdates;
2222

23-
const {IsSomeRendererActing} = ReactSharedInternals;
23+
const {IsSomeRendererActing, ReactCurrentActQueue} = ReactSharedInternals;
2424

2525
// This version of `act` is only used by our tests. Unlike the public version
2626
// of `act`, it's designed to work identically in both production and
2727
// development. It may have slightly different behavior from the public
2828
// version, too, since our constraints in our test suite are not the same as
2929
// those of developers using React — we're testing React itself, as opposed to
3030
// building an app with React.
31+
// TODO: Replace the internal "concurrent" implementations of `act` with a
32+
// single shared module.
3133

3234
let actingUpdatesScopeDepth = 0;
3335

@@ -50,8 +52,14 @@ export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
5052
IsSomeRendererActing.current = true;
5153
IsThisRendererActing.current = true;
5254
actingUpdatesScopeDepth++;
55+
if (__DEV__ && actingUpdatesScopeDepth === 1) {
56+
ReactCurrentActQueue.disableActWarning = true;
57+
}
5358

5459
const unwind = () => {
60+
if (__DEV__ && actingUpdatesScopeDepth === 1) {
61+
ReactCurrentActQueue.disableActWarning = false;
62+
}
5563
actingUpdatesScopeDepth--;
5664
IsSomeRendererActing.current = previousIsSomeRendererActing;
5765
IsThisRendererActing.current = previousIsThisRendererActing;

packages/react-noop-renderer/src/createReactNoop.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131

3232
import ReactSharedInternals from 'shared/ReactSharedInternals';
3333
import enqueueTask from 'shared/enqueueTask';
34-
const {IsSomeRendererActing} = ReactSharedInternals;
34+
const {IsSomeRendererActing, ReactCurrentActQueue} = ReactSharedInternals;
3535

3636
type Container = {
3737
rootID: string,
@@ -1048,6 +1048,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
10481048
// version, too, since our constraints in our test suite are not the same as
10491049
// those of developers using React — we're testing React itself, as opposed to
10501050
// building an app with React.
1051+
// TODO: Replace the internal "concurrent" implementations of `act` with a
1052+
// single shared module.
10511053

10521054
const {batchedUpdates, IsThisRendererActing} = NoopRenderer;
10531055
let actingUpdatesScopeDepth = 0;
@@ -1071,8 +1073,14 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
10711073
IsSomeRendererActing.current = true;
10721074
IsThisRendererActing.current = true;
10731075
actingUpdatesScopeDepth++;
1076+
if (__DEV__ && actingUpdatesScopeDepth === 1) {
1077+
ReactCurrentActQueue.disableActWarning = true;
1078+
}
10741079

10751080
const unwind = () => {
1081+
if (__DEV__ && actingUpdatesScopeDepth === 1) {
1082+
ReactCurrentActQueue.disableActWarning = false;
1083+
}
10761084
actingUpdatesScopeDepth--;
10771085
IsSomeRendererActing.current = previousIsSomeRendererActing;
10781086
IsThisRendererActing.current = previousIsThisRendererActing;

packages/react-reconciler/src/ReactFiberHooks.new.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ import {
7979
requestEventTime,
8080
warnIfNotCurrentlyActingEffectsInDEV,
8181
warnIfNotCurrentlyActingUpdatesInDev,
82-
warnIfNotScopedWithMatchingAct,
8382
markSkippedUpdateLanes,
8483
isInterleavedUpdate,
8584
} from './ReactFiberWorkLoop.new';
@@ -2011,7 +2010,6 @@ function dispatchAction<S, A>(
20112010
if (__DEV__) {
20122011
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
20132012
if ('undefined' !== typeof jest) {
2014-
warnIfNotScopedWithMatchingAct(fiber);
20152013
warnIfNotCurrentlyActingUpdatesInDev(fiber);
20162014
}
20172015
}

packages/react-reconciler/src/ReactFiberHooks.old.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ import {
7979
requestEventTime,
8080
warnIfNotCurrentlyActingEffectsInDEV,
8181
warnIfNotCurrentlyActingUpdatesInDev,
82-
warnIfNotScopedWithMatchingAct,
8382
markSkippedUpdateLanes,
8483
isInterleavedUpdate,
8584
} from './ReactFiberWorkLoop.old';
@@ -2011,7 +2010,6 @@ function dispatchAction<S, A>(
20112010
if (__DEV__) {
20122011
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
20132012
if ('undefined' !== typeof jest) {
2014-
warnIfNotScopedWithMatchingAct(fiber);
20152013
warnIfNotCurrentlyActingUpdatesInDev(fiber);
20162014
}
20172015
}

packages/react-reconciler/src/ReactFiberReconciler.new.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ import {
6060
discreteUpdates,
6161
flushDiscreteUpdates,
6262
flushPassiveEffects,
63-
warnIfNotScopedWithMatchingAct,
64-
warnIfUnmockedScheduler,
6563
IsThisRendererActing,
6664
act,
6765
} from './ReactFiberWorkLoop.new';
@@ -272,13 +270,6 @@ export function updateContainer(
272270
}
273271
const current = container.current;
274272
const eventTime = requestEventTime();
275-
if (__DEV__) {
276-
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
277-
if ('undefined' !== typeof jest) {
278-
warnIfUnmockedScheduler(current);
279-
warnIfNotScopedWithMatchingAct(current);
280-
}
281-
}
282273
const lane = requestUpdateLane(current);
283274

284275
if (enableSchedulingProfiler) {

packages/react-reconciler/src/ReactFiberReconciler.old.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ import {
6060
discreteUpdates,
6161
flushDiscreteUpdates,
6262
flushPassiveEffects,
63-
warnIfNotScopedWithMatchingAct,
64-
warnIfUnmockedScheduler,
6563
IsThisRendererActing,
6664
act,
6765
} from './ReactFiberWorkLoop.old';
@@ -272,13 +270,6 @@ export function updateContainer(
272270
}
273271
const current = container.current;
274272
const eventTime = requestEventTime();
275-
if (__DEV__) {
276-
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
277-
if ('undefined' !== typeof jest) {
278-
warnIfUnmockedScheduler(current);
279-
warnIfNotScopedWithMatchingAct(current);
280-
}
281-
}
282273
const lane = requestUpdateLane(current);
283274

284275
if (enableSchedulingProfiler) {

0 commit comments

Comments
 (0)