Skip to content

Commit

Permalink
act should work without mock Scheduler
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed Jun 22, 2021
1 parent b5debd5 commit 6c91e94
Show file tree
Hide file tree
Showing 44 changed files with 423 additions and 1,004 deletions.
2 changes: 1 addition & 1 deletion fixtures/dom/src/__tests__/nested-act-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('unmocked scheduler', () => {
TestAct(() => {
TestRenderer.create(<Effecty />);
});
expect(log).toEqual(['called']);
expect(log).toEqual([]);
});
expect(log).toEqual(['called']);
});
Expand Down
207 changes: 0 additions & 207 deletions fixtures/dom/src/__tests__/wrong-act-test.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('ReactDOMTestSelectors', () => {
React = require('react');

const ReactDOM = require('react-dom/testing');
act = ReactDOM.act;
act = React.unstable_act;
createComponentSelector = ReactDOM.createComponentSelector;
createHasPseudoClassSelector = ReactDOM.createHasPseudoClassSelector;
createRoleSelector = ReactDOM.createRoleSelector;
Expand Down
67 changes: 37 additions & 30 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,32 +354,6 @@ function runActTests(label, render, unmount, rerender) {
expect(container.innerHTML).toBe('2');
});
});

// @gate __DEV__
it('warns if you return a value inside act', () => {
expect(() => act(() => null)).toErrorDev(
[
'The callback passed to act(...) function must return undefined, or a Promise.',
],
{withoutStack: true},
);
expect(() => act(() => 123)).toErrorDev(
[
'The callback passed to act(...) function must return undefined, or a Promise.',
],
{withoutStack: true},
);
});

// @gate __DEV__
it('warns if you try to await a sync .act call', () => {
expect(() => act(() => {}).then(() => {})).toErrorDev(
[
'Do not await the result of calling act(...) with sync logic, it is not a Promise.',
],
{withoutStack: true},
);
});
});

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

await act(async () => {
render(<App />, container);
// flush a little to start the timer
expect(Scheduler).toFlushAndYield([]);
});
expect(container.innerHTML).toBe('0');
// Flush the pending timers
await act(async () => {
await sleep(100);
});
expect(container.innerHTML).toBe('1');
});

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

// @gate __DEV__
it('flushes microtasks before exiting (sync function)', async () => {
// Same as previous test, but the callback passed to `act` is not itself
// an async function.
function App() {
const [ctr, setCtr] = React.useState(0);
async function someAsyncFunction() {
// queue a bunch of promises to be sure they all flush
await null;
await null;
await null;
setCtr(1);
}
React.useEffect(() => {
someAsyncFunction();
}, []);
return ctr;
}

await act(() => {
render(<App />, container);
});
expect(container.innerHTML).toEqual('1');
});

// @gate __DEV__
it('warns if you do not await an act call', async () => {
spyOnDevAndProd(console, 'error');
Expand Down Expand Up @@ -461,7 +462,13 @@ function runActTests(label, render, unmount, rerender) {

await sleep(150);
if (__DEV__) {
expect(console.error).toHaveBeenCalledTimes(1);
expect(console.error).toHaveBeenCalledTimes(2);
expect(console.error.calls.argsFor(0)[0]).toMatch(
'You seem to have overlapping act() calls',
);
expect(console.error.calls.argsFor(1)[0]).toMatch(
'You seem to have overlapping act() calls',
);
}
});

Expand Down

This file was deleted.

Loading

0 comments on commit 6c91e94

Please sign in to comment.