Skip to content

Commit

Permalink
feat: use act to flush instead of custom implementation (#768)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: cleanup is now synchronous and wraps the unmounting process in `act`.
  • Loading branch information
eps1lon authored Aug 30, 2020
1 parent 5a10621 commit 9b62634
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 79 deletions.
93 changes: 87 additions & 6 deletions src/__tests__/cleanup.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react'
import {render, cleanup} from '../'

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

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

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

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

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

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

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

describe('fake timers and missing act warnings', () => {
beforeEach(() => {
jest.resetAllMocks()
jest.spyOn(console, 'error').mockImplementation(() => {
// assert messages explicitly
})
jest.useFakeTimers()
})

afterEach(() => {
jest.useRealTimers()
})

test('cleanup does not flush immediates', () => {
const microTaskSpy = jest.fn()
function Test() {
const counter = 1
const [, setDeferredCounter] = React.useState(null)
React.useEffect(() => {
let cancelled = false
setImmediate(() => {
microTaskSpy()
if (!cancelled) {
setDeferredCounter(counter)
}
})

return () => {
cancelled = true
}
}, [counter])

return null
}
render(<Test />)

cleanup()

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

test('cleanup does not swallow missing act warnings', () => {
const deferredStateUpdateSpy = jest.fn()
function Test() {
const counter = 1
const [, setDeferredCounter] = React.useState(null)
React.useEffect(() => {
let cancelled = false
setImmediate(() => {
deferredStateUpdateSpy()
if (!cancelled) {
setDeferredCounter(counter)
}
})

return () => {
cancelled = true
}
}, [counter])

return null
}
render(<Test />)

jest.runAllImmediates()
cleanup()

expect(deferredStateUpdateSpy).toHaveBeenCalledTimes(1)
// console.error is mocked
// eslint-disable-next-line no-console
expect(console.error).toHaveBeenCalledTimes(1)
// eslint-disable-next-line no-console
expect(console.error.mock.calls[0][0]).toMatch(
'a test was not wrapped in act(...)',
)
})
})
67 changes: 0 additions & 67 deletions src/flush-microtasks.js

This file was deleted.

10 changes: 4 additions & 6 deletions src/pure.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
} from '@testing-library/dom'
import act, {asyncAct} from './act-compat'
import {fireEvent} from './fire-event'
import flush from './flush-microtasks'

configureDTL({
asyncWrapper: async cb => {
Expand Down Expand Up @@ -100,17 +99,16 @@ function render(
}
}

async function cleanup() {
function cleanup() {
mountedContainers.forEach(cleanupAtContainer)
// flush microtask queue after unmounting in case
// unmount sequence generates new microtasks
await flush()
}

// maybe one day we'll expose this (perhaps even as a utility returned by render).
// but let's wait until someone asks for it.
function cleanupAtContainer(container) {
ReactDOM.unmountComponentAtNode(container)
act(() => {
ReactDOM.unmountComponentAtNode(container)
})
if (container.parentNode === document.body) {
document.body.removeChild(container)
}
Expand Down

0 comments on commit 9b62634

Please sign in to comment.