Skip to content
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

fix: Require await act(...) #1214

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
10 changes: 5 additions & 5 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ module.exports = Object.assign(jestConfig, {
// Full coverage across the build matrix (React 18, 19) but not in a single job
// Ful coverage is checked via codecov
'./src/act-compat': {
branches: 90,
branches: 80,
},
'./src/pure': {
// minimum coverage of jobs using React 18 and 19
branches: 95,
functions: 88,
lines: 92,
statements: 92,
branches: 90,
functions: 81,
lines: 91,
statements: 91,
},
},
})
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
"react/no-adjacent-inline-elements": "off",
"import/no-unassigned-import": "off",
"import/named": "off",
"testing-library/no-await-sync-events": "off",
"testing-library/no-container": "off",
"testing-library/no-debugging-utils": "off",
"testing-library/no-dom-import": "off",
Expand Down
99 changes: 99 additions & 0 deletions src/__tests__/act-compat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import * as React from 'react'
import {render, fireEvent, screen} from '../'
import {actIfEnabled} from '../act-compat'

beforeEach(() => {
global.IS_REACT_ACT_ENVIRONMENT = true
})

test('render calls useEffect immediately', async () => {
const effectCb = jest.fn()
function MyUselessComponent() {
React.useEffect(effectCb)
return null
}
await render(<MyUselessComponent />)
expect(effectCb).toHaveBeenCalledTimes(1)
})

test('findByTestId returns the element', async () => {
const ref = React.createRef()
await render(<div ref={ref} data-testid="foo" />)
expect(await screen.findByTestId('foo')).toBe(ref.current)
})

test('fireEvent triggers useEffect calls', async () => {
const effectCb = jest.fn()
function Counter() {
React.useEffect(effectCb)
const [count, setCount] = React.useState(0)
return <button onClick={() => setCount(count + 1)}>{count}</button>
}
const {
container: {firstChild: buttonNode},
} = await render(<Counter />)

effectCb.mockClear()
// eslint-disable-next-line testing-library/no-await-sync-events -- TODO: Remove lint rule.
await fireEvent.click(buttonNode)
expect(buttonNode).toHaveTextContent('1')
expect(effectCb).toHaveBeenCalledTimes(1)
})

test('calls to hydrate will run useEffects', async () => {
const effectCb = jest.fn()
function MyUselessComponent() {
React.useEffect(effectCb)
return null
}
await render(<MyUselessComponent />, {hydrate: true})
expect(effectCb).toHaveBeenCalledTimes(1)
})

test('cleans up IS_REACT_ACT_ENVIRONMENT if its callback throws', async () => {
global.IS_REACT_ACT_ENVIRONMENT = false

await expect(() =>
actIfEnabled(() => {
throw new Error('threw')
}),
).rejects.toThrow('threw')

expect(global.IS_REACT_ACT_ENVIRONMENT).toEqual(false)
})

test('cleans up IS_REACT_ACT_ENVIRONMENT if its async callback throws', async () => {
global.IS_REACT_ACT_ENVIRONMENT = false

await expect(() =>
actIfEnabled(async () => {
throw new Error('thenable threw')
}),
).rejects.toThrow('thenable threw')

expect(global.IS_REACT_ACT_ENVIRONMENT).toEqual(false)
})

test('state update from microtask does not trigger "missing act" warning', async () => {
let triggerStateUpdateFromMicrotask
function App() {
const [state, setState] = React.useState(0)
triggerStateUpdateFromMicrotask = () => setState(1)
React.useEffect(() => {
// eslint-disable-next-line jest/no-conditional-in-test
if (state === 1) {
Promise.resolve().then(() => {
setState(2)
})
}
}, [state])
return state
}
const {container} = await render(<App />)

await actIfEnabled(() => {
triggerStateUpdateFromMicrotask()
})

expect(container).toHaveTextContent('2')
})
79 changes: 18 additions & 61 deletions src/__tests__/act.js
Original file line number Diff line number Diff line change
@@ -1,69 +1,26 @@
import * as React from 'react'
import {act, render, fireEvent, screen} from '../'
import {act, render} from '../'

test('render calls useEffect immediately', () => {
const effectCb = jest.fn()
function MyUselessComponent() {
React.useEffect(effectCb)
return null
}
render(<MyUselessComponent />)
expect(effectCb).toHaveBeenCalledTimes(1)
})

test('findByTestId returns the element', async () => {
const ref = React.createRef()
render(<div ref={ref} data-testid="foo" />)
expect(await screen.findByTestId('foo')).toBe(ref.current)
})

test('fireEvent triggers useEffect calls', () => {
const effectCb = jest.fn()
function Counter() {
React.useEffect(effectCb)
const [count, setCount] = React.useState(0)
return <button onClick={() => setCount(count + 1)}>{count}</button>
}
const {
container: {firstChild: buttonNode},
} = render(<Counter />)

effectCb.mockClear()
fireEvent.click(buttonNode)
expect(buttonNode).toHaveTextContent('1')
expect(effectCb).toHaveBeenCalledTimes(1)
beforeEach(() => {
global.IS_REACT_ACT_ENVIRONMENT = true
})

test('calls to hydrate will run useEffects', () => {
const effectCb = jest.fn()
function MyUselessComponent() {
React.useEffect(effectCb)
return null
test('does not work outside IS_REACT_ENVIRONMENT like React.act', async () => {
let setState
function Component() {
const [state, _setState] = React.useState(0)
setState = _setState
return state
}
render(<MyUselessComponent />, {hydrate: true})
expect(effectCb).toHaveBeenCalledTimes(1)
})
await render(<Component />)

test('cleans up IS_REACT_ACT_ENVIRONMENT if its callback throws', () => {
global.IS_REACT_ACT_ENVIRONMENT = false

expect(() =>
act(() => {
throw new Error('threw')
}),
).toThrow('threw')

expect(global.IS_REACT_ACT_ENVIRONMENT).toEqual(false)
})

test('cleans up IS_REACT_ACT_ENVIRONMENT if its async callback throws', async () => {
global.IS_REACT_ACT_ENVIRONMENT = false

await expect(() =>
act(async () => {
throw new Error('thenable threw')
}),
).rejects.toThrow('thenable threw')

expect(global.IS_REACT_ACT_ENVIRONMENT).toEqual(false)
await expect(async () => {
await act(() => {
setState(1)
})
}).toErrorDev(
'The current testing environment is not configured to support act(...)',
{withoutStack: true},
)
})
5 changes: 3 additions & 2 deletions src/__tests__/auto-cleanup-skip.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ import * as React from 'react'
let render
beforeAll(() => {
process.env.RTL_SKIP_AUTO_CLEANUP = 'true'
globalThis.IS_REACT_ACT_ENVIRONMENT = true
const rtl = require('../')
render = rtl.render
})

// This one verifies that if RTL_SKIP_AUTO_CLEANUP is set
// then we DON'T auto-wire up the afterEach for folks
test('first', () => {
render(<div>hi</div>)
test('first', async () => {
await render(<div>hi</div>)
})

test('second', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/auto-cleanup.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {render} from '../'
// This just verifies that by importing RTL in an
// environment which supports afterEach (like jest)
// we'll get automatic cleanup between tests.
test('first', () => {
render(<div>hi</div>)
test('first', async () => {
await render(<div>hi</div>)
})

test('second', () => {
Expand Down
49 changes: 32 additions & 17 deletions src/__tests__/cleanup.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react'
import {render, cleanup} from '../'

test('cleans up the document', () => {
test('cleans up the document', async () => {
const spy = jest.fn()
const divId = 'my-div'

Expand All @@ -16,18 +16,18 @@ test('cleans up the document', () => {
}
}

render(<Test />)
cleanup()
await render(<Test />)
await cleanup()
expect(document.body).toBeEmptyDOMElement()
expect(spy).toHaveBeenCalledTimes(1)
})

test('cleanup does not error when an element is not a child', () => {
render(<div />, {container: document.createElement('div')})
cleanup()
test('cleanup does not error when an element is not a child', async () => {
await render(<div />, {container: document.createElement('div')})
await cleanup()
})

test('cleanup runs effect cleanup functions', () => {
test('cleanup runs effect cleanup functions', async () => {
const spy = jest.fn()

const Test = () => {
Expand All @@ -36,11 +36,23 @@ test('cleanup runs effect cleanup functions', () => {
return null
}

render(<Test />)
cleanup()
await render(<Test />)
await cleanup()
expect(spy).toHaveBeenCalledTimes(1)
})

test('cleanup cleans up every root and disconnects containers', async () => {
const {container: container1} = await render(<div />)
const {container: container2} = await render(<span />)

await cleanup()

expect(container1).toBeEmptyDOMElement()
expect(container1.isConnected).toBe(false)
expect(container2).toBeEmptyDOMElement()
expect(container2.isConnected).toBe(false)
})

describe('fake timers and missing act warnings', () => {
beforeEach(() => {
jest.resetAllMocks()
Expand All @@ -55,7 +67,7 @@ describe('fake timers and missing act warnings', () => {
jest.useRealTimers()
})

test('cleanup does not flush microtasks', () => {
test('cleanup does flush microtasks', async () => {
const microTaskSpy = jest.fn()
function Test() {
const counter = 1
Expand All @@ -72,22 +84,25 @@ describe('fake timers and missing act warnings', () => {

return () => {
cancelled = true
Promise.resolve().then(() => {
microTaskSpy()
})
}
}, [counter])

return null
}
render(<Test />)

cleanup()
await render(<Test />)
expect(microTaskSpy).toHaveBeenCalledTimes(1)

expect(microTaskSpy).toHaveBeenCalledTimes(0)
await cleanup()
expect(microTaskSpy).toHaveBeenCalledTimes(2)
// console.error is mocked
// eslint-disable-next-line no-console
expect(console.error).toHaveBeenCalledTimes(0)
})

test('cleanup does not swallow missing act warnings', () => {
test('cleanup does not swallow missing act warnings', async () => {
const deferredStateUpdateSpy = jest.fn()
function Test() {
const counter = 1
Expand All @@ -109,10 +124,10 @@ describe('fake timers and missing act warnings', () => {

return null
}
render(<Test />)
await render(<Test />)

jest.runAllTimers()
cleanup()
await cleanup()

expect(deferredStateUpdateSpy).toHaveBeenCalledTimes(1)
// console.error is mocked
Expand Down
12 changes: 6 additions & 6 deletions src/__tests__/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,24 @@ afterEach(() => {
console.log.mockRestore()
})

test('debug pretty prints the container', () => {
test('debug pretty prints the container', async () => {
const HelloWorld = () => <h1>Hello World</h1>
const {debug} = render(<HelloWorld />)
const {debug} = await render(<HelloWorld />)
debug()
expect(console.log).toHaveBeenCalledTimes(1)
expect(console.log).toHaveBeenCalledWith(
expect.stringContaining('Hello World'),
)
})

test('debug pretty prints multiple containers', () => {
test('debug pretty prints multiple containers', async () => {
const HelloWorld = () => (
<>
<h1 data-testid="testId">Hello World</h1>
<h1 data-testid="testId">Hello World</h1>
</>
)
const {debug} = render(<HelloWorld />)
const {debug} = await render(<HelloWorld />)
const multipleElements = screen.getAllByTestId('testId')
debug(multipleElements)

Expand All @@ -36,9 +36,9 @@ test('debug pretty prints multiple containers', () => {
)
})

test('allows same arguments as prettyDOM', () => {
test('allows same arguments as prettyDOM', async () => {
const HelloWorld = () => <h1>Hello World</h1>
const {debug, container} = render(<HelloWorld />)
const {debug, container} = await render(<HelloWorld />)
debug(container, 6, {highlight: false})
expect(console.log).toHaveBeenCalledTimes(1)
expect(console.log.mock.calls[0]).toMatchInlineSnapshot(`
Expand Down
Loading