From f78839bf4147a777a823e33a429bcf5de9562f9e Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 16 Feb 2023 23:46:50 +0100 Subject: [PATCH] fix: Prevent "missing act" warning for queued microtasks (#1137) * Add intended behavior * fix: Prevent "missing act" warning for in-flight promises * Disable TL lint rules in tests * Implementation without macrotask * Now I member --- package.json | 2 + src/__tests__/end-to-end.js | 211 ++++++++++++++++++++++++++---------- src/pure.js | 30 ++++- 3 files changed, 182 insertions(+), 61 deletions(-) diff --git a/package.json b/package.json index f9061345..70aebdad 100644 --- a/package.json +++ b/package.json @@ -82,6 +82,8 @@ "testing-library/no-debugging-utils": "off", "testing-library/no-dom-import": "off", "testing-library/no-unnecessary-act": "off", + "testing-library/prefer-explicit-assert": "off", + "testing-library/prefer-find-by": "off", "testing-library/prefer-user-event": "off" } }, diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index cf222aec..005591d3 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -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
Loading...
+ } + + return ( +
+ Loaded this message: {state.data.returnedMessage}! +
+ ) } - }, []) - if (state.loading) { - return
Loading...
- } + test('waitForElementToBeRemoved', async () => { + render() + const loading = () => screen.getByText('Loading...') + await waitForElementToBeRemoved(loading) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) + + test('waitFor', async () => { + render() + await waitFor(() => screen.getByText(/Loading../)) + await waitFor(() => screen.getByText(/Loaded this message:/)) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) - return ( -
- Loaded this message: {state.data.returnedMessage}! -
- ) -} + test('findBy', async () => { + render() + 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() - const loading = () => screen.getByText('Loading...') - await waitForElementToBeRemoved(loading) - expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) - }) - - test('waitFor', async () => { - render() - const message = () => screen.getByText(/Loaded this message:/) - await waitFor(message) - expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) - }) - - test('findBy', async () => { - render() - 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

Loading..

+ } + + return ( +
Loaded this message: {fetchState.todo}
+ ) + } + + test('waitForElementToBeRemoved', async () => { + render() + const loading = () => screen.getByText('Loading..') + await waitForElementToBeRemoved(loading) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) + + test('waitFor', async () => { + render() + await waitFor(() => { + screen.getByText('Loading..') + }) + await waitFor(() => { + screen.getByText(/Loaded this message:/) + }) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) + + test('findBy', async () => { + render() + await expect(screen.findByTestId('message')).resolves.toHaveTextContent( + /Hello World/, + ) + }) + }, +) diff --git a/src/pure.js b/src/pure.js index 94b3b2bd..845aede1 100644 --- a/src/pure.js +++ b/src/pure.js @@ -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) + } + }) + + return result } finally { setReactActEnvironment(previousActEnvironment) }