Skip to content

Commit 06f7b4f

Browse files
authored
act should work without mock Scheduler (facebook#21714)
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 422e0bb commit 06f7b4f

Some content is hidden

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

44 files changed

+423
-1004
lines changed

fixtures/dom/src/__tests__/nested-act-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('unmocked scheduler', () => {
4848
TestAct(() => {
4949
TestRenderer.create(<Effecty />);
5050
});
51-
expect(log).toEqual(['called']);
51+
expect(log).toEqual([]);
5252
});
5353
expect(log).toEqual(['called']);
5454
});

fixtures/dom/src/__tests__/wrong-act-test.js

-207
This file was deleted.

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

+1-1
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

+37-30
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

-56
This file was deleted.

0 commit comments

Comments
 (0)