-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: Prevent "missing act" warning for queued microtasks #1137
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,73 +1,164 @@ | ||
import * as React from 'react' | ||
import {render, waitForElementToBeRemoved, screen, waitFor} from '../' | ||
|
||
const fetchAMessage = () => | ||
new Promise(resolve => { | ||
// we are using random timeout here to simulate a real-time example | ||
// of an async operation calling a callback at a non-deterministic time | ||
const randomTimeout = Math.floor(Math.random() * 100) | ||
setTimeout(() => { | ||
resolve({returnedMessage: 'Hello World'}) | ||
}, randomTimeout) | ||
}) | ||
|
||
function ComponentWithLoader() { | ||
const [state, setState] = React.useState({data: undefined, loading: true}) | ||
React.useEffect(() => { | ||
let cancelled = false | ||
fetchAMessage().then(data => { | ||
if (!cancelled) { | ||
setState({data, loading: false}) | ||
} | ||
describe.each([ | ||
['real timers', () => jest.useRealTimers()], | ||
['fake legacy timers', () => jest.useFakeTimers('legacy')], | ||
['fake modern timers', () => jest.useFakeTimers('modern')], | ||
])( | ||
'it waits for the data to be loaded in a macrotask using %s', | ||
(label, useTimers) => { | ||
beforeEach(() => { | ||
useTimers() | ||
}) | ||
|
||
afterEach(() => { | ||
jest.useRealTimers() | ||
}) | ||
|
||
return () => { | ||
cancelled = true | ||
const fetchAMessageInAMacrotask = () => | ||
new Promise(resolve => { | ||
// we are using random timeout here to simulate a real-time example | ||
// of an async operation calling a callback at a non-deterministic time | ||
const randomTimeout = Math.floor(Math.random() * 100) | ||
setTimeout(() => { | ||
resolve({returnedMessage: 'Hello World'}) | ||
}, randomTimeout) | ||
}) | ||
|
||
function ComponentWithMacrotaskLoader() { | ||
const [state, setState] = React.useState({data: undefined, loading: true}) | ||
React.useEffect(() => { | ||
let cancelled = false | ||
fetchAMessageInAMacrotask().then(data => { | ||
if (!cancelled) { | ||
setState({data, loading: false}) | ||
} | ||
}) | ||
|
||
return () => { | ||
cancelled = true | ||
} | ||
}, []) | ||
|
||
if (state.loading) { | ||
return <div>Loading...</div> | ||
} | ||
|
||
return ( | ||
<div data-testid="message"> | ||
Loaded this message: {state.data.returnedMessage}! | ||
</div> | ||
) | ||
} | ||
}, []) | ||
|
||
if (state.loading) { | ||
return <div>Loading...</div> | ||
} | ||
test('waitForElementToBeRemoved', async () => { | ||
render(<ComponentWithMacrotaskLoader />) | ||
const loading = () => screen.getByText('Loading...') | ||
await waitForElementToBeRemoved(loading) | ||
expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) | ||
}) | ||
|
||
test('waitFor', async () => { | ||
render(<ComponentWithMacrotaskLoader />) | ||
await waitFor(() => screen.getByText(/Loading../)) | ||
await waitFor(() => screen.getByText(/Loaded this message:/)) | ||
expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) | ||
}) | ||
|
||
return ( | ||
<div data-testid="message"> | ||
Loaded this message: {state.data.returnedMessage}! | ||
</div> | ||
) | ||
} | ||
test('findBy', async () => { | ||
render(<ComponentWithMacrotaskLoader />) | ||
await expect(screen.findByTestId('message')).resolves.toHaveTextContent( | ||
/Hello World/, | ||
) | ||
}) | ||
}, | ||
) | ||
|
||
describe.each([ | ||
['real timers', () => jest.useRealTimers()], | ||
['fake legacy timers', () => jest.useFakeTimers('legacy')], | ||
['fake modern timers', () => jest.useFakeTimers('modern')], | ||
])('it waits for the data to be loaded using %s', (label, useTimers) => { | ||
beforeEach(() => { | ||
useTimers() | ||
}) | ||
|
||
afterEach(() => { | ||
jest.useRealTimers() | ||
}) | ||
|
||
test('waitForElementToBeRemoved', async () => { | ||
render(<ComponentWithLoader />) | ||
const loading = () => screen.getByText('Loading...') | ||
await waitForElementToBeRemoved(loading) | ||
expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) | ||
}) | ||
|
||
test('waitFor', async () => { | ||
render(<ComponentWithLoader />) | ||
const message = () => screen.getByText(/Loaded this message:/) | ||
await waitFor(message) | ||
expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) | ||
}) | ||
|
||
test('findBy', async () => { | ||
render(<ComponentWithLoader />) | ||
await expect(screen.findByTestId('message')).resolves.toHaveTextContent( | ||
/Hello World/, | ||
) | ||
}) | ||
}) | ||
])( | ||
'it waits for the data to be loaded in a microtask using %s', | ||
(label, useTimers) => { | ||
beforeEach(() => { | ||
useTimers() | ||
}) | ||
|
||
afterEach(() => { | ||
jest.useRealTimers() | ||
}) | ||
|
||
const fetchAMessageInAMicrotask = () => | ||
Promise.resolve({ | ||
status: 200, | ||
json: () => Promise.resolve({title: 'Hello World'}), | ||
}) | ||
|
||
function ComponentWithMicrotaskLoader() { | ||
const [fetchState, setFetchState] = React.useState({fetching: true}) | ||
|
||
React.useEffect(() => { | ||
if (fetchState.fetching) { | ||
fetchAMessageInAMicrotask().then(res => { | ||
return ( | ||
res | ||
.json() | ||
// By spec, the runtime can only yield back to the event loop once | ||
// the microtask queue is empty. | ||
// So we ensure that we actually wait for that as well before yielding back from `waitFor`. | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => data) | ||
.then(data => { | ||
setFetchState({todo: data.title, fetching: false}) | ||
}) | ||
) | ||
}) | ||
} | ||
}, [fetchState]) | ||
|
||
if (fetchState.fetching) { | ||
return <p>Loading..</p> | ||
} | ||
|
||
return ( | ||
<div data-testid="message">Loaded this message: {fetchState.todo}</div> | ||
) | ||
} | ||
|
||
test('waitForElementToBeRemoved', async () => { | ||
render(<ComponentWithMicrotaskLoader />) | ||
const loading = () => screen.getByText('Loading..') | ||
await waitForElementToBeRemoved(loading) | ||
expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) | ||
}) | ||
|
||
test('waitFor', async () => { | ||
render(<ComponentWithMicrotaskLoader />) | ||
await waitFor(() => { | ||
screen.getByText('Loading..') | ||
}) | ||
await waitFor(() => { | ||
screen.getByText(/Loaded this message:/) | ||
}) | ||
expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) | ||
}) | ||
|
||
test('findBy', async () => { | ||
render(<ComponentWithMicrotaskLoader />) | ||
await expect(screen.findByTestId('message')).resolves.toHaveTextContent( | ||
/Hello World/, | ||
) | ||
}) | ||
}, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,20 @@ import act, { | |
} from './act-compat' | ||
import {fireEvent} from './fire-event' | ||
|
||
function jestFakeTimersAreEnabled() { | ||
/* istanbul ignore else */ | ||
if (typeof jest !== 'undefined' && jest !== null) { | ||
return ( | ||
// legacy timers | ||
setTimeout._isMockFunction === true || // modern timers | ||
// eslint-disable-next-line prefer-object-has-own -- No Object.hasOwn in all target environments we support. | ||
Object.prototype.hasOwnProperty.call(setTimeout, 'clock') | ||
) | ||
} // istanbul ignore next | ||
|
||
return false | ||
} | ||
|
||
configureDTL({ | ||
unstable_advanceTimersWrapper: cb => { | ||
return act(cb) | ||
|
@@ -23,7 +37,21 @@ configureDTL({ | |
const previousActEnvironment = getIsReactActEnvironment() | ||
setReactActEnvironment(false) | ||
try { | ||
return await cb() | ||
const result = await cb() | ||
// Drain microtask queue. | ||
// Otherwise we'll restore the previous act() environment, before we resolve the `waitFor` call. | ||
// The caller would have no chance to wrap the in-flight Promises in `act()` | ||
await new Promise(resolve => { | ||
setTimeout(() => { | ||
resolve() | ||
}, 0) | ||
|
||
if (jestFakeTimersAreEnabled()) { | ||
jest.advanceTimersByTime(0) | ||
} | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An observation: if I comment out this new Seems the tests would need to be amended somehow to really reproduce the bug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added tests in #1215 |
||
|
||
return result | ||
} finally { | ||
setReactActEnvironment(previousActEnvironment) | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.