From 46a743e0a5d99ab4b336e081220e7fd6148d80f9 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 09:57:28 +0200 Subject: [PATCH 01/14] React 18 compat --- src/__mocks__/pure.js | 15 ++++ src/__tests__/end-to-end.js | 3 + src/__tests__/render.js | 33 ++++++++ src/pure.js | 153 +++++++++++++++++++++++++++--------- tests/setup-env.js | 2 + 5 files changed, 170 insertions(+), 36 deletions(-) create mode 100644 src/__mocks__/pure.js diff --git a/src/__mocks__/pure.js b/src/__mocks__/pure.js new file mode 100644 index 00000000..733917e6 --- /dev/null +++ b/src/__mocks__/pure.js @@ -0,0 +1,15 @@ +import * as ReactDOM from 'react-dom' + +const pure = jest.requireActual('../pure') + +const originalRender = pure.render +// Test concurrent react in the experimental release channel +function possiblyConcurrentRender(ui, options) { + return originalRender(ui, { + concurrent: ReactDOM.version.includes('-experimental-'), + ...options, + }) +} +pure.render = possiblyConcurrentRender + +module.exports = pure diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index 87c70f1b..398fcf30 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -30,6 +30,9 @@ class ComponentWithLoader extends React.Component { } test('it waits for the data to be loaded', async () => { + // TODO: discussions/23#discussioncomment-812450 + jest.useFakeTimers() + render() const loading = () => screen.getByText('Loading...') await waitForElementToBeRemoved(loading) diff --git a/src/__tests__/render.js b/src/__tests__/render.js index fea1a649..304a208c 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 `concurrent` is used with an incomaptible version', () => { + const isConcurrentReact = typeof ReactDOM.createRoot === 'function' + + const performConcurrentRender = () => render(
, {concurrent: true}) + + // 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/pure.js b/src/pure.js index 75098f78..568aed16 100644 --- a/src/pure.js +++ b/src/pure.js @@ -19,38 +19,69 @@ configureDTL({ 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 + 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 = ReactDOM.createRoot(container, options) + + return { + hydrate(element) { + 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 +89,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 +106,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,20 +134,66 @@ function render( } } -function cleanup() { - mountedContainers.forEach(cleanupAtContainer) +function render( + ui, + { + container, + baseElement = container, + concurrent = false, + 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 createRoot = concurrent ? createConcurrentRoot : createLegacyRoot + root = createRoot(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() } // just re-export everything from dom-testing-library diff --git a/tests/setup-env.js b/tests/setup-env.js index 6c0b953b..57248f37 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1,3 +1,5 @@ +jest.mock('scheduler', () => require('scheduler/unstable_mock')) +jest.mock('../src/pure') import '@testing-library/jest-dom/extend-expect' let consoleErrorMock From 70d2b34114ec55f0fd59dee93ab799f6b5c71c71 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 10:27:04 +0200 Subject: [PATCH 02/14] concurrent: false -> legacyRoot: true --- src/__tests__/render.js | 4 ++-- src/pure.js | 9 ++++++--- types/index.d.ts | 5 +++++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/__tests__/render.js b/src/__tests__/render.js index 304a208c..148b3003 100644 --- a/src/__tests__/render.js +++ b/src/__tests__/render.js @@ -102,10 +102,10 @@ test('flushes useEffect cleanup functions sync on unmount()', () => { expect(spy).toHaveBeenCalledTimes(1) }) -test('throws if `concurrent` is used with an incomaptible version', () => { +test('throws if `legacyRoot: false` is used with an incomaptible version', () => { const isConcurrentReact = typeof ReactDOM.createRoot === 'function' - const performConcurrentRender = () => render(
, {concurrent: true}) + const performConcurrentRender = () => render(
, {legacyRoot: false}) // eslint-disable-next-line jest/no-if -- jest doesn't support conditional tests if (isConcurrentReact) { diff --git a/src/pure.js b/src/pure.js index 568aed16..f5637012 100644 --- a/src/pure.js +++ b/src/pure.js @@ -139,7 +139,7 @@ function render( { container, baseElement = container, - concurrent = false, + legacyRoot, queries, hydrate = false, wrapper, @@ -157,8 +157,11 @@ function render( 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 createRoot = concurrent ? createConcurrentRoot : createLegacyRoot - root = createRoot(container, {hydrate}) + const createRootImpl = + legacyRoot === true || typeof ReactDOM.createRoot !== 'function' + ? createLegacyRoot + : createConcurrentRoot + root = createRootImpl(container, {hydrate}) mountedRootEntries.push({container, root}) // we'll add it to the mounted containers regardless of whether it's actually 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 } From 0e069a90eb321addbad04481da2d92d019816e6d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 20:00:37 +0200 Subject: [PATCH 03/14] fix logic for default of used root implementation --- src/__tests__/render.js | 2 +- src/pure.js | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/__tests__/render.js b/src/__tests__/render.js index 148b3003..ac996444 100644 --- a/src/__tests__/render.js +++ b/src/__tests__/render.js @@ -114,7 +114,7 @@ test('throws if `legacyRoot: false` is used with an incomaptible version', () => } 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)."`, + `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).`, ) } }) diff --git a/src/pure.js b/src/pure.js index f5637012..a2db6860 100644 --- a/src/pure.js +++ b/src/pure.js @@ -139,7 +139,7 @@ function render( { container, baseElement = container, - legacyRoot, + legacyRoot = typeof ReactDOM.createRoot !== 'function', queries, hydrate = false, wrapper, @@ -157,10 +157,7 @@ function render( 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 === true || typeof ReactDOM.createRoot !== 'function' - ? createLegacyRoot - : createConcurrentRoot + const createRootImpl = legacyRoot ? createLegacyRoot : createConcurrentRoot root = createRootImpl(container, {hydrate}) mountedRootEntries.push({container, root}) From d83e34ad26ca081b5f8ec0d2f40d284ffdba01e6 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 20:02:13 +0200 Subject: [PATCH 04/14] Add links to reactwg discussions --- src/__tests__/end-to-end.js | 2 +- src/pure.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index 398fcf30..3b0e3d71 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -30,7 +30,7 @@ class ComponentWithLoader extends React.Component { } test('it waits for the data to be loaded', async () => { - // TODO: discussions/23#discussioncomment-812450 + // TODO: https://github.com/reactwg/react-18/discussions/23#discussioncomment-812450 jest.useFakeTimers() render() diff --git a/src/pure.js b/src/pure.js index a2db6860..c3da243b 100644 --- a/src/pure.js +++ b/src/pure.js @@ -21,6 +21,7 @@ configureDTL({ act(() => { // 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 From bc9ef29ccb6a1827d52430cc3714ff420196391d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 20:51:42 +0200 Subject: [PATCH 05/14] fix waitFor* in concurrent Reat --- src/__tests__/end-to-end.js | 3 --- src/pure.js | 31 +++++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index 3b0e3d71..87c70f1b 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -30,9 +30,6 @@ class ComponentWithLoader extends React.Component { } test('it waits for the data to be loaded', async () => { - // TODO: https://github.com/reactwg/react-18/discussions/23#discussioncomment-812450 - jest.useFakeTimers() - render() const loading = () => screen.getByText('Loading...') await waitForElementToBeRemoved(loading) diff --git a/src/pure.js b/src/pure.js index c3da243b..f469d854 100644 --- a/src/pure.js +++ b/src/pure.js @@ -4,18 +4,13 @@ import { getQueriesForElement, prettyDOM, configure as configureDTL, + waitFor as waitForDTL, + waitForElementToBeRemoved as waitForElementToBeRemovedDTL, } from '@testing-library/dom' import act, {asyncAct} 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(() => { @@ -197,9 +192,29 @@ function cleanup() { 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 From 08c79325a1e2f884e74d4194c2448b325d91df02 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 21:07:58 +0200 Subject: [PATCH 06/14] cleanup --- src/__mocks__/pure.js | 15 --------------- src/pure.js | 2 +- tests/setup-env.js | 5 +++-- 3 files changed, 4 insertions(+), 18 deletions(-) delete mode 100644 src/__mocks__/pure.js diff --git a/src/__mocks__/pure.js b/src/__mocks__/pure.js deleted file mode 100644 index 733917e6..00000000 --- a/src/__mocks__/pure.js +++ /dev/null @@ -1,15 +0,0 @@ -import * as ReactDOM from 'react-dom' - -const pure = jest.requireActual('../pure') - -const originalRender = pure.render -// Test concurrent react in the experimental release channel -function possiblyConcurrentRender(ui, options) { - return originalRender(ui, { - concurrent: ReactDOM.version.includes('-experimental-'), - ...options, - }) -} -pure.render = possiblyConcurrentRender - -module.exports = pure diff --git a/src/pure.js b/src/pure.js index f469d854..328d21e4 100644 --- a/src/pure.js +++ b/src/pure.js @@ -7,7 +7,7 @@ import { 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({ diff --git a/tests/setup-env.js b/tests/setup-env.js index 57248f37..4532cc52 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1,5 +1,3 @@ -jest.mock('scheduler', () => require('scheduler/unstable_mock')) -jest.mock('../src/pure') import '@testing-library/jest-dom/extend-expect' let consoleErrorMock @@ -20,3 +18,6 @@ beforeEach(() => { afterEach(() => { consoleErrorMock.mockRestore() }) + +// 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')) From 2676666b8dc288915e23ae7930433343186834d5 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 21:11:35 +0200 Subject: [PATCH 07/14] Let full matrix build Otherwise we can't see directly which version is failing --- .github/workflows/validate.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 191bbcc1..bad02303 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] From a06995d8993f543fe8a72c25fbacd97361f140df Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 21:20:40 +0200 Subject: [PATCH 08/14] Ignore coverage for now --- jest.config.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 jest.config.js 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, + }, + }, +}) From 7e7e0d036cb8ba1237c2c8599176491246067306 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:01:59 +0200 Subject: [PATCH 09/14] Mark scheduler mocking as TODO --- tests/setup-env.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/setup-env.js b/tests/setup-env.js index 4532cc52..8a280118 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -19,5 +19,6 @@ 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')) From 1b201e0518a1db0bf629a74995d7a8d8b3757371 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:07:42 +0200 Subject: [PATCH 10/14] Merge codecov reports across react versions --- .github/workflows/validate.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index bad02303..6ffdf5ef 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -48,6 +48,8 @@ jobs: - name: ⬆️ Upload coverage report uses: codecov/codecov-action@v1 + with: + flags: ${{ matrix.react }} release: needs: main From a776ef66fb941a93a57af0afa517aab409e19c8c Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:14:13 +0200 Subject: [PATCH 11/14] Exclude internal bugs from coverage These only exists as invariants. --- src/pure.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pure.js b/src/pure.js index 328d21e4..24f5b698 100644 --- a/src/pure.js +++ b/src/pure.js @@ -44,6 +44,7 @@ function createConcurrentRoot(container, options) { 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`.', From f57072204ca534137c4c357ce1de525486c09d7e Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:46:40 +0200 Subject: [PATCH 12/14] Add failing tests for waitFor and findBy --- src/__tests__/end-to-end.js | 40 +++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) 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/, + ) + }) }) From 00d534e314e23fe090db703e833ca5be58e20ecb Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 16 Jun 2021 10:18:21 +0200 Subject: [PATCH 13/14] { hydrate: true } -> hydrateRoot --- src/pure.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pure.js b/src/pure.js index 24f5b698..f605e397 100644 --- a/src/pure.js +++ b/src/pure.js @@ -40,7 +40,9 @@ function createConcurrentRoot(container, options) { `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).'`, ) } - const root = ReactDOM.createRoot(container, options) + const root = options.hydrate + ? ReactDOM.hydrateRoot(container) + : ReactDOM.createRoot(container) return { hydrate(element) { From 2ac2377ee89edef467576d7e4daa96e1146c9982 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 16 Jun 2021 10:36:07 +0200 Subject: [PATCH 14/14] Revert "test: Ignore React 18 legacy root deprecation warnings (#929)" This reverts commit c1878a9ea68a3a13982233684b924a917e87a1f6. --- src/__tests__/cleanup.js | 19 +++++-------------- src/__tests__/new-act.js | 6 +++--- src/__tests__/old-act.js | 6 +++--- src/__tests__/stopwatch.js | 5 +---- tests/setup-env.js | 19 ------------------- 5 files changed, 12 insertions(+), 43 deletions(-) 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__/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__/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/tests/setup-env.js b/tests/setup-env.js index 8a280118..f2927ef6 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1,24 +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'))