Skip to content

Commit e83ad98

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 e83ad98

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+422
-796
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('ReactDOMTestSelectors', () => {
3333
React = require('react');
3434

3535
const ReactDOM = require('react-dom/testing');
36-
act = ReactDOM.act;
36+
act = React.unstable_act;
3737
createComponentSelector = ReactDOM.createComponentSelector;
3838
createHasPseudoClassSelector = ReactDOM.createHasPseudoClassSelector;
3939
createRoleSelector = ReactDOM.createRoleSelector;

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/client/ReactDOM.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import {
2929
flushControlled,
3030
injectIntoDevTools,
3131
IsThisRendererActing,
32-
act,
3332
attemptSynchronousHydration,
3433
attemptDiscreteHydration,
3534
attemptContinuousHydration,
@@ -163,8 +162,8 @@ const Internals = {
163162
getFiberCurrentPropsFromNode,
164163
enqueueStateRestore,
165164
restoreStateIfNeeded,
165+
batchedUpdates,
166166
],
167-
act,
168167
// TODO: Temporary. Only used by our internal version of `act. Will remove.
169168
IsThisRendererActing,
170169
};

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,14 @@ const getNodeFromInstance = EventInternals[1];
3434
const getFiberCurrentPropsFromNode = EventInternals[2];
3535
const enqueueStateRestore = EventInternals[3];
3636
const restoreStateIfNeeded = EventInternals[4];
37+
const batchedUpdates = EventInternals[5];
3738

38-
const act = SecretInternals.act;
39+
const act_notBatchedInLegacyMode = React.unstable_act;
40+
function act(callback) {
41+
return act_notBatchedInLegacyMode(() => {
42+
return batchedUpdates(callback);
43+
});
44+
}
3945

4046
function Event(suffix) {}
4147

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-dom/testing.classic.fb.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
export * from './index.classic.fb.js';
1111
export {
12-
act,
1312
createComponentSelector,
1413
createHasPseudoClassSelector,
1514
createRoleSelector,

packages/react-dom/testing.experimental.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,3 @@
88
*/
99

1010
export * from './index.experimental.js';
11-
export {act} from 'react-reconciler/src/ReactFiberReconciler';

packages/react-dom/testing.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
export * from './index.js';
1111
export {
12-
act,
1312
createComponentSelector,
1413
createHasPseudoClassSelector,
1514
createRoleSelector,

0 commit comments

Comments
 (0)