diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 191bbcc1..6ffdf5ef 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -16,6 +16,7 @@ jobs: # ignore all-contributors PRs if: ${{ !contains(github.head_ref, 'all-contributors') }} strategy: + fail-fast: false matrix: node: [12, 14, 16] react: [latest, next, experimental] @@ -47,6 +48,8 @@ jobs: - name: ⬆️ Upload coverage report uses: codecov/codecov-action@v1 + with: + flags: ${{ matrix.react }} release: needs: main diff --git a/jest.config.js b/jest.config.js new file mode 100644 index 00000000..8b6b3f4e --- /dev/null +++ b/jest.config.js @@ -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, + }, + }, +}) diff --git a/src/__tests__/cleanup.js b/src/__tests__/cleanup.js index 9d3f52d4..0dcbac12 100644 --- a/src/__tests__/cleanup.js +++ b/src/__tests__/cleanup.js @@ -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', () => { @@ -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(...)', + ) }) }) diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index 87c70f1b..67affabf 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -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 => { @@ -29,9 +29,37 @@ class ComponentWithLoader extends React.Component { } } -test('it waits for the data to be loaded', async () => { - render() - 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() + 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/, + ) + }) }) diff --git a/src/__tests__/new-act.js b/src/__tests__/new-act.js index af81e29c..42552594 100644 --- a/src/__tests__/new-act.js +++ b/src/__tests__/new-act.js @@ -1,4 +1,4 @@ -let asyncAct, consoleErrorMock +let asyncAct jest.mock('react-dom/test-utils', () => ({ act: cb => { @@ -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 () => { diff --git a/src/__tests__/old-act.js b/src/__tests__/old-act.js index 6081fef8..0153fea3 100644 --- a/src/__tests__/old-act.js +++ b/src/__tests__/old-act.js @@ -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', () => ({ diff --git a/src/__tests__/render.js b/src/__tests__/render.js index fea1a649..ac996444 100644 --- a/src/__tests__/render.js +++ b/src/__tests__/render.js @@ -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(
, {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(, {container}) + + expect(container).toContainHTML('') + + render(, {container}) + + expect(container).toContainHTML('') + + unmount() + + expect(container).toBeEmptyDOMElement() +}) diff --git a/src/__tests__/stopwatch.js b/src/__tests__/stopwatch.js index 400fce10..eeaf395c 100644 --- a/src/__tests__/stopwatch.js +++ b/src/__tests__/stopwatch.js @@ -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() }) diff --git a/src/pure.js b/src/pure.js index 75098f78..f605e397 100644 --- a/src/pure.js +++ b/src/pure.js @@ -4,53 +4,83 @@ import { getQueriesForElement, prettyDOM, configure as configureDTL, + waitFor as waitForDTL, + waitForElementToBeRemoved as waitForElementToBeRemovedDTL, } from '@testing-library/dom' -import act, {asyncAct} from './act-compat' +import act from './act-compat' import {fireEvent} from './fire-event' configureDTL({ - asyncWrapper: async cb => { - let result - await asyncAct(async () => { - result = await cb() - }) - return result - }, eventWrapper: cb => { let result act(() => { - result = cb() + // TODO: Remove ReactDOM.flushSync once `act` flushes the microtask queue. + // Otherwise `act` wrapping updates that schedule microtask would need to be followed with `await null` to flush the microtask queue manually + // See https://github.com/reactwg/react-18/discussions/21#discussioncomment-796755 + result = ReactDOM.flushSync(cb) }) return result }, }) +// Ideally we'd just use a WeakMap where containers are keys and roots are values. +// We use two variables so that we can bail out in constant time when we render with a new container (most common use case) +/** + * @type {Set} + */ const mountedContainers = new Set() +/** + * @type Array<{container: import('react-dom').Container, root: ReturnType}> + */ +const mountedRootEntries = [] -function render( - ui, - { - container, - baseElement = container, - queries, - hydrate = false, - wrapper: WrapperComponent, - } = {}, -) { - if (!baseElement) { - // default to document.body instead of documentElement to avoid output of potentially-large - // head elements (such as JSS style blocks) in debug output - baseElement = document.body +function createConcurrentRoot(container, options) { + if (typeof ReactDOM.createRoot !== 'function') { + throw new TypeError( + `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).'`, + ) } - if (!container) { - container = baseElement.appendChild(document.createElement('div')) + const root = options.hydrate + ? ReactDOM.hydrateRoot(container) + : ReactDOM.createRoot(container) + + return { + hydrate(element) { + /* istanbul ignore if */ + if (!options.hydrate) { + throw new Error( + 'Attempted to hydrate a non-hydrateable root. This is a bug in `@testing-library/react`.', + ) + } + root.render(element) + }, + render(element) { + root.render(element) + }, + unmount() { + root.unmount() + }, } +} - // we'll add it to the mounted containers regardless of whether it's actually - // added to document.body so the cleanup method works regardless of whether - // they're passing us a custom container or not. - mountedContainers.add(container) +function createLegacyRoot(container) { + return { + hydrate(element) { + ReactDOM.hydrate(element, container) + }, + render(element) { + ReactDOM.render(element, container) + }, + unmount() { + ReactDOM.unmountComponentAtNode(container) + }, + } +} +function renderRoot( + ui, + {baseElement, container, hydrate, queries, root, wrapper: WrapperComponent}, +) { const wrapUiIfNeeded = innerElement => WrapperComponent ? React.createElement(WrapperComponent, null, innerElement) @@ -58,9 +88,9 @@ function render( act(() => { if (hydrate) { - ReactDOM.hydrate(wrapUiIfNeeded(ui), container) + root.hydrate(wrapUiIfNeeded(ui), container) } else { - ReactDOM.render(wrapUiIfNeeded(ui), container) + root.render(wrapUiIfNeeded(ui), container) } }) @@ -75,11 +105,15 @@ function render( console.log(prettyDOM(el, maxLength, options)), unmount: () => { act(() => { - ReactDOM.unmountComponentAtNode(container) + root.unmount() }) }, rerender: rerenderUi => { - render(wrapUiIfNeeded(rerenderUi), {container, baseElement}) + renderRoot(wrapUiIfNeeded(rerenderUi), { + container, + baseElement, + root, + }) // Intentionally do not return anything to avoid unnecessarily complicating the API. // folks can use all the same utilities we return in the first place that are bound to the container }, @@ -99,25 +133,91 @@ function render( } } -function cleanup() { - mountedContainers.forEach(cleanupAtContainer) +function render( + ui, + { + container, + baseElement = container, + legacyRoot = typeof ReactDOM.createRoot !== 'function', + queries, + hydrate = false, + wrapper, + } = {}, +) { + if (!baseElement) { + // default to document.body instead of documentElement to avoid output of potentially-large + // head elements (such as JSS style blocks) in debug output + baseElement = document.body + } + if (!container) { + container = baseElement.appendChild(document.createElement('div')) + } + + let root + // eslint-disable-next-line no-negated-condition -- we want to map the evolution of this over time. The root is created first. Only later is it re-used so we don't want to read the case that happens later first. + if (!mountedContainers.has(container)) { + const createRootImpl = legacyRoot ? createLegacyRoot : createConcurrentRoot + root = createRootImpl(container, {hydrate}) + + mountedRootEntries.push({container, root}) + // we'll add it to the mounted containers regardless of whether it's actually + // added to document.body so the cleanup method works regardless of whether + // they're passing us a custom container or not. + mountedContainers.add(container) + } else { + mountedRootEntries.forEach(rootEntry => { + if (rootEntry.container === container) { + root = rootEntry.root + } + }) + } + + return renderRoot(ui, { + container, + baseElement, + queries, + hydrate, + wrapper, + root, + }) } -// 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) { - act(() => { - ReactDOM.unmountComponentAtNode(container) +function cleanup() { + mountedRootEntries.forEach(({root, container}) => { + act(() => { + root.unmount() + }) + if (container.parentNode === document.body) { + document.body.removeChild(container) + } }) - if (container.parentNode === document.body) { - document.body.removeChild(container) - } - mountedContainers.delete(container) + mountedRootEntries.length = 0 + mountedContainers.clear() +} + +function waitFor(callback, options) { + return waitForDTL(() => { + let result + act(() => { + result = callback() + }) + return result + }, options) +} + +function waitForElementToBeRemoved(callback, options) { + return waitForElementToBeRemovedDTL(() => { + let result + act(() => { + result = callback() + }) + return result + }, options) } // just re-export everything from dom-testing-library export * from '@testing-library/dom' -export {render, cleanup, act, fireEvent} +export {render, cleanup, act, fireEvent, waitFor, waitForElementToBeRemoved} // NOTE: we're not going to export asyncAct because that's our own compatibility // thing for people using react-dom@16.8.0. Anyone else doesn't need it and diff --git a/tests/setup-env.js b/tests/setup-env.js index 6c0b953b..f2927ef6 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1,20 +1,5 @@ import '@testing-library/jest-dom/extend-expect' -let consoleErrorMock - -beforeEach(() => { - const originalConsoleError = console.error - consoleErrorMock = jest - .spyOn(console, 'error') - .mockImplementation((message, ...optionalParams) => { - // Ignore ReactDOM.render/ReactDOM.hydrate deprecation warning - if (message.indexOf('Use createRoot instead.') !== -1) { - return - } - originalConsoleError(message, ...optionalParams) - }) -}) - -afterEach(() => { - consoleErrorMock.mockRestore() -}) +// TODO: Can be removed in a future React release: https://github.com/reactwg/react-18/discussions/23#discussioncomment-798952 +// eslint-disable-next-line import/no-extraneous-dependencies -- need the version from React not an explicitly declared one +jest.mock('scheduler', () => require('scheduler/unstable_mock')) diff --git a/types/index.d.ts b/types/index.d.ts index 6d0c67ee..cd2701cf 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -37,6 +37,11 @@ export interface RenderOptions< container?: Container baseElement?: Element hydrate?: boolean + /** + * Set to `true` if you want to force synchronous `ReactDOM.render`. + * Otherwise `render` will default to concurrent React if available. + */ + legacyRoot?: boolean queries?: Q wrapper?: React.ComponentType }