Skip to content

Reimplement act without mock Scheduler #21714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrote to not use a mock scheduler API

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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a new feature. There was no inherent reason this wasn't already supported. This is nice because usually an act scope doesn't need to be async, but if the effects trigger microtasks you might still want to await that.

});
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