Skip to content

Commit 693228c

Browse files
eps1lonkentcdodds
authored andcommittedSep 2, 2020
feat: use act to flush instead of custom implementation (#768)
BREAKING CHANGE: cleanup is now synchronous and wraps the unmounting process in `act`.
1 parent 5a10621 commit 693228c

File tree

3 files changed

+91
-79
lines changed

3 files changed

+91
-79
lines changed
 

Diff for: ‎src/__tests__/cleanup.js

+87-6
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from 'react'
22
import {render, cleanup} from '../'
33

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

@@ -17,17 +17,17 @@ test('cleans up the document', async () => {
1717
}
1818

1919
render(<Test />)
20-
await cleanup()
20+
cleanup()
2121
expect(document.body).toBeEmptyDOMElement()
2222
expect(spy).toHaveBeenCalledTimes(1)
2323
})
2424

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

30-
test('cleanup runs effect cleanup functions', async () => {
30+
test('cleanup runs effect cleanup functions', () => {
3131
const spy = jest.fn()
3232

3333
const Test = () => {
@@ -37,6 +37,87 @@ test('cleanup runs effect cleanup functions', async () => {
3737
}
3838

3939
render(<Test />)
40-
await cleanup()
40+
cleanup()
4141
expect(spy).toHaveBeenCalledTimes(1)
4242
})
43+
44+
describe('fake timers and missing act warnings', () => {
45+
beforeEach(() => {
46+
jest.resetAllMocks()
47+
jest.spyOn(console, 'error').mockImplementation(() => {
48+
// assert messages explicitly
49+
})
50+
jest.useFakeTimers()
51+
})
52+
53+
afterEach(() => {
54+
jest.useRealTimers()
55+
})
56+
57+
test('cleanup does not flush immediates', () => {
58+
const microTaskSpy = jest.fn()
59+
function Test() {
60+
const counter = 1
61+
const [, setDeferredCounter] = React.useState(null)
62+
React.useEffect(() => {
63+
let cancelled = false
64+
setImmediate(() => {
65+
microTaskSpy()
66+
if (!cancelled) {
67+
setDeferredCounter(counter)
68+
}
69+
})
70+
71+
return () => {
72+
cancelled = true
73+
}
74+
}, [counter])
75+
76+
return null
77+
}
78+
render(<Test />)
79+
80+
cleanup()
81+
82+
expect(microTaskSpy).toHaveBeenCalledTimes(0)
83+
// console.error is mocked
84+
// eslint-disable-next-line no-console
85+
expect(console.error).toHaveBeenCalledTimes(0)
86+
})
87+
88+
test('cleanup does not swallow missing act warnings', () => {
89+
const deferredStateUpdateSpy = jest.fn()
90+
function Test() {
91+
const counter = 1
92+
const [, setDeferredCounter] = React.useState(null)
93+
React.useEffect(() => {
94+
let cancelled = false
95+
setImmediate(() => {
96+
deferredStateUpdateSpy()
97+
if (!cancelled) {
98+
setDeferredCounter(counter)
99+
}
100+
})
101+
102+
return () => {
103+
cancelled = true
104+
}
105+
}, [counter])
106+
107+
return null
108+
}
109+
render(<Test />)
110+
111+
jest.runAllImmediates()
112+
cleanup()
113+
114+
expect(deferredStateUpdateSpy).toHaveBeenCalledTimes(1)
115+
// console.error is mocked
116+
// eslint-disable-next-line no-console
117+
expect(console.error).toHaveBeenCalledTimes(1)
118+
// eslint-disable-next-line no-console
119+
expect(console.error.mock.calls[0][0]).toMatch(
120+
'a test was not wrapped in act(...)',
121+
)
122+
})
123+
})

Diff for: ‎src/flush-microtasks.js

-67
This file was deleted.

Diff for: ‎src/pure.js

+4-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
} from '@testing-library/dom'
88
import act, {asyncAct} from './act-compat'
99
import {fireEvent} from './fire-event'
10-
import flush from './flush-microtasks'
1110

1211
configureDTL({
1312
asyncWrapper: async cb => {
@@ -100,17 +99,16 @@ function render(
10099
}
101100
}
102101

103-
async function cleanup() {
102+
function cleanup() {
104103
mountedContainers.forEach(cleanupAtContainer)
105-
// flush microtask queue after unmounting in case
106-
// unmount sequence generates new microtasks
107-
await flush()
108104
}
109105

110106
// maybe one day we'll expose this (perhaps even as a utility returned by render).
111107
// but let's wait until someone asks for it.
112108
function cleanupAtContainer(container) {
113-
ReactDOM.unmountComponentAtNode(container)
109+
act(() => {
110+
ReactDOM.unmountComponentAtNode(container)
111+
})
114112
if (container.parentNode === document.body) {
115113
document.body.removeChild(container)
116114
}

0 commit comments

Comments
 (0)
Please sign in to comment.