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

feat: Use concurrent React when available #925

Closed
wants to merge 14 commits into from
3 changes: 3 additions & 0 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
# ignore all-contributors PRs
if: ${{ !contains(github.head_ref, 'all-contributors') }}
strategy:
fail-fast: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should only be enabled in next/experimental React tag builds to keep latest tag builds fast.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would land this change regardless of this PR. Personally, I'd like to know if a job only fails in one job. Otherwise how do I know what's causing the issue?

matrix:
node: [12, 14, 16]
react: [latest, next, experimental]
Expand Down Expand Up @@ -47,6 +48,8 @@ jobs:

- name: ⬆️ Upload coverage report
uses: codecov/codecov-action@v1
with:
flags: ${{ matrix.react }}

release:
needs: main
Expand Down
15 changes: 15 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const {jest: jestConfig} = require('kcd-scripts/config')

module.exports = Object.assign(jestConfig, {
coverageThreshold: {
...jestConfig.coverageThreshold,
// full coverage across the build matrix (React 17, 18) but not in a single job
'./src/pure': {
// minimum coverage of jobs using React 17 and 18
branches: 85,
functions: 76,
lines: 81,
statements: 81,
},
},
})
19 changes: 5 additions & 14 deletions src/__tests__/cleanup.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,7 @@ describe('fake timers and missing act warnings', () => {
expect(microTaskSpy).toHaveBeenCalledTimes(0)
// console.error is mocked
// eslint-disable-next-line no-console
expect(console.error).toHaveBeenCalledTimes(
// ReactDOM.render is deprecated in React 18
React.version.startsWith('18') ? 1 : 0,
)
expect(console.error).toHaveBeenCalledTimes(0)
})

test('cleanup does not swallow missing act warnings', () => {
Expand Down Expand Up @@ -118,16 +115,10 @@ describe('fake timers and missing act warnings', () => {
expect(deferredStateUpdateSpy).toHaveBeenCalledTimes(1)
// console.error is mocked
// eslint-disable-next-line no-console
expect(console.error).toHaveBeenCalledTimes(
// ReactDOM.render is deprecated in React 18
React.version.startsWith('18') ? 2 : 1,
)
expect(console.error).toHaveBeenCalledTimes(1)
// eslint-disable-next-line no-console
expect(
console.error.mock.calls[
// ReactDOM.render is deprecated in React 18
React.version.startsWith('18') ? 1 : 0
][0],
).toMatch('a test was not wrapped in act(...)')
expect(console.error.mock.calls[0][0]).toMatch(
'a test was not wrapped in act(...)',
)
})
})
40 changes: 34 additions & 6 deletions src/__tests__/end-to-end.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import {render, waitForElementToBeRemoved, screen} from '../'
import {render, waitForElementToBeRemoved, screen, waitFor} from '../'

const fetchAMessage = () =>
new Promise(resolve => {
Expand Down Expand Up @@ -29,9 +29,37 @@ class ComponentWithLoader extends React.Component {
}
}

test('it waits for the data to be loaded', async () => {
render(<ComponentWithLoader />)
const loading = () => screen.getByText('Loading...')
await waitForElementToBeRemoved(loading)
expect(screen.getByTestId('message')).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/,
)
})
})
6 changes: 3 additions & 3 deletions src/__tests__/new-act.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
let asyncAct, consoleErrorMock
let asyncAct

jest.mock('react-dom/test-utils', () => ({
act: cb => {
Expand All @@ -9,11 +9,11 @@ jest.mock('react-dom/test-utils', () => ({
beforeEach(() => {
jest.resetModules()
asyncAct = require('../act-compat').asyncAct
consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => {})
jest.spyOn(console, 'error').mockImplementation(() => {})
})

afterEach(() => {
consoleErrorMock.mockRestore()
console.error.mockRestore()
})

test('async act works when it does not exist (older versions of react)', async () => {
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/old-act.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
let asyncAct, consoleErrorMock
let asyncAct

beforeEach(() => {
jest.resetModules()
asyncAct = require('../act-compat').asyncAct
consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => {})
jest.spyOn(console, 'error').mockImplementation(() => {})
})

afterEach(() => {
consoleErrorMock.mockRestore()
console.error.mockRestore()
})

jest.mock('react-dom/test-utils', () => ({
Expand Down
33 changes: 33 additions & 0 deletions src/__tests__/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,36 @@ test('flushes useEffect cleanup functions sync on unmount()', () => {

expect(spy).toHaveBeenCalledTimes(1)
})

test('throws if `legacyRoot: false` is used with an incomaptible version', () => {
const isConcurrentReact = typeof ReactDOM.createRoot === 'function'

const performConcurrentRender = () => render(<div />, {legacyRoot: false})

// eslint-disable-next-line jest/no-if -- jest doesn't support conditional tests
if (isConcurrentReact) {
// eslint-disable-next-line jest/no-conditional-expect -- yes, jest still doesn't support conditional tests
expect(performConcurrentRender).not.toThrow()
} else {
// eslint-disable-next-line jest/no-conditional-expect -- yes, jest still doesn't support conditional tests
expect(performConcurrentRender).toThrowError(
`Attempted to use concurrent React with \`react-dom@${ReactDOM.version}\`. Be sure to use the \`next\` or \`experimental\` release channel (https://reactjs.org/docs/release-channels.html).`,
)
}
})

test('can be called multiple times on the same container', () => {
const container = document.createElement('div')

const {unmount} = render(<strong />, {container})

expect(container).toContainHTML('<strong></strong>')

render(<em />, {container})

expect(container).toContainHTML('<em></em>')

unmount()

expect(container).toBeEmptyDOMElement()
})
5 changes: 1 addition & 4 deletions src/__tests__/stopwatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,5 @@ test('unmounts a component', async () => {
// and get an error.
await sleep(5)
// eslint-disable-next-line no-console
expect(console.error).toHaveBeenCalledTimes(
// ReactDOM.render is deprecated in React 18
React.version.startsWith('18') ? 1 : 0,
)
expect(console.error).not.toHaveBeenCalled()
})