From f684fdc43d0c5b0ec1ae2c67f327fad568c2b5e9 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 09:57:28 +0200 Subject: [PATCH 01/21] 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 9858181cbe2882ab13aea2f36ed233a0e4483211 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 10:27:04 +0200 Subject: [PATCH 02/21] 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 46dd1575..693f86aa 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 b3a885859da3d21f27f5562fb5402da87ccb6e0d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 20:00:37 +0200 Subject: [PATCH 03/21] 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 e6991a2df934ea2c70deff6d8ae659b77d0e6bb5 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 20:02:13 +0200 Subject: [PATCH 04/21] 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 7cff132970866063a206432830b7a4705c2b620a Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 20:51:42 +0200 Subject: [PATCH 05/21] 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 14442684813ff4a42cfd89bf7e54717877c69c0f Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 21:07:58 +0200 Subject: [PATCH 06/21] 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 bf22542fef9a2c5e12179f0390c0259fbf07a10b Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 21:11:35 +0200 Subject: [PATCH 07/21] 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 67b71c24..ce3fb26f 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: # TODO: relax `'16.9.1'` to `16` once GitHub has 16.9.1 cached. 16.9.0 is broken due to https://github.com/nodejs/node/issues/40030 node: [12, 14, '16.9.1'] From 59d28b6d056e1aa924c201e44679e16858d3052e Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 8 Jun 2021 21:20:40 +0200 Subject: [PATCH 08/21] 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 cea49efef6e284f54051018988b392febce55267 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:01:59 +0200 Subject: [PATCH 09/21] 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 aa0a89009c3617d48998db1f19b3b5e6f144335b Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:07:42 +0200 Subject: [PATCH 10/21] 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 ce3fb26f..45cc7d13 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -53,6 +53,8 @@ jobs: - name: ⬆️ Upload coverage report uses: codecov/codecov-action@v1 + with: + flags: ${{ matrix.react }} release: needs: main From 994e43c03c9ef68c723ea58c54c60ae852a8b14c Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:14:13 +0200 Subject: [PATCH 11/21] 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 c54df93f5e67796ec4a2d12890fae011bee96bb0 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 9 Jun 2021 07:46:40 +0200 Subject: [PATCH 12/21] 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 c83a9a472a6c4480ca3f721e0dee4313438b4c75 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 16 Jun 2021 10:18:21 +0200 Subject: [PATCH 13/21] { 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 7887030705cd2c076a06eb1184c0b07bf983792b Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 16 Jun 2021 10:36:07 +0200 Subject: [PATCH 14/21] 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')) From 2138e2feae882f52596713ada8ed0364516d669b Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 23 Jun 2021 09:53:10 +0200 Subject: [PATCH 15/21] Update minimal coverage --- jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index 8b6b3f4e..45d8fce9 100644 --- a/jest.config.js +++ b/jest.config.js @@ -6,7 +6,7 @@ module.exports = Object.assign(jestConfig, { // 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, + branches: 82, functions: 76, lines: 81, statements: 81, From f59f1627cd36d7402193a1dde3af6cddd28c77c0 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 29 Jun 2021 09:59:18 +0200 Subject: [PATCH 16/21] Use isomorphic act if available --- src/__tests__/no-act.js | 8 ++++++++ src/act-compat.js | 9 +++++---- src/pure.js | 5 +---- tests/setup-env.js | 4 ---- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/__tests__/no-act.js b/src/__tests__/no-act.js index d739e763..de4117bb 100644 --- a/src/__tests__/no-act.js +++ b/src/__tests__/no-act.js @@ -12,7 +12,15 @@ afterEach(() => { consoleErrorMock.mockRestore() }) +// no react-dom/test-utils also means no isomorphic act since isomorphic act got released after test-utils act jest.mock('react-dom/test-utils', () => ({})) +jest.mock('react', () => { + const ReactActual = jest.requireActual('react') + + delete ReactActual.unstable_act + + return ReactActual +}) test('act works even when there is no act from test utils', () => { const callback = jest.fn() diff --git a/src/act-compat.js b/src/act-compat.js index 40ecdab9..16124afc 100644 --- a/src/act-compat.js +++ b/src/act-compat.js @@ -2,8 +2,9 @@ import * as React from 'react' import ReactDOM from 'react-dom' import * as testUtils from 'react-dom/test-utils' -const reactAct = testUtils.act -const actSupported = reactAct !== undefined +const isomorphicAct = React.unstable_act +const domAct = testUtils.act +const actSupported = domAct !== undefined // act is supported react-dom@16.8.0 // so for versions that don't have act from test utils @@ -14,7 +15,7 @@ function actPolyfill(cb) { ReactDOM.render(
, document.createElement('div')) } -const act = reactAct || actPolyfill +const act = isomorphicAct || domAct || actPolyfill let youHaveBeenWarned = false let isAsyncActSupported = null @@ -50,7 +51,7 @@ function asyncAct(cb) { } let cbReturn, result try { - result = reactAct(() => { + result = domAct(() => { cbReturn = cb() return cbReturn }) diff --git a/src/pure.js b/src/pure.js index f605e397..091af4b6 100644 --- a/src/pure.js +++ b/src/pure.js @@ -14,10 +14,7 @@ configureDTL({ eventWrapper: cb => { let result 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) + result = cb() }) return result }, diff --git a/tests/setup-env.js b/tests/setup-env.js index f2927ef6..264828a9 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1,5 +1 @@ import '@testing-library/jest-dom/extend-expect' - -// 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 78eaa061ced0363c2c4033b6ad09458cd137983b Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sun, 5 Sep 2021 12:07:43 +0200 Subject: [PATCH 17/21] Inline waitFor from dtl --- src/dtlHelpers.js | 88 ++++++++++++++++++++ src/pure.js | 12 +-- src/wait-for.js | 201 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 11 deletions(-) create mode 100644 src/dtlHelpers.js create mode 100644 src/wait-for.js diff --git a/src/dtlHelpers.js b/src/dtlHelpers.js new file mode 100644 index 00000000..26427ec9 --- /dev/null +++ b/src/dtlHelpers.js @@ -0,0 +1,88 @@ +// @source @testing-library/dom/src/helpers.js + +// Constant node.nodeType for text nodes, see: +// https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType#Node_type_constants +const TEXT_NODE = 3 + +function jestFakeTimersAreEnabled() { + /* istanbul ignore else */ + if (typeof jest !== 'undefined' && jest !== null) { + return ( + // legacy timers + setTimeout._isMockFunction === true || + // modern timers + Object.prototype.hasOwnProperty.call(setTimeout, 'clock') + ) + } + // istanbul ignore next + return false +} + +function getDocument() { + /* istanbul ignore if */ + if (typeof window === 'undefined') { + throw new Error('Could not find default container') + } + return window.document +} +function getWindowFromNode(node) { + if (node.defaultView) { + // node is document + return node.defaultView + } else if (node.ownerDocument && node.ownerDocument.defaultView) { + // node is a DOM node + return node.ownerDocument.defaultView + } else if (node.window) { + // node is window + return node.window + } else if (node.then instanceof Function) { + throw new Error( + `It looks like you passed a Promise object instead of a DOM node. Did you do something like \`fireEvent.click(screen.findBy...\` when you meant to use a \`getBy\` query \`fireEvent.click(screen.getBy...\`, or await the findBy query \`fireEvent.click(await screen.findBy...\`?`, + ) + } else if (Array.isArray(node)) { + throw new Error( + `It looks like you passed an Array instead of a DOM node. Did you do something like \`fireEvent.click(screen.getAllBy...\` when you meant to use a \`getBy\` query \`fireEvent.click(screen.getBy...\`?`, + ) + } else if ( + typeof node.debug === 'function' && + typeof node.logTestingPlaygroundURL === 'function' + ) { + throw new Error( + `It looks like you passed a \`screen\` object. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a query, e.g. \`fireEvent.click(screen.getBy..., \`?`, + ) + } else { + // The user passed something unusual to a calling function + throw new Error( + `Unable to find the "window" object for the given node. Please file an issue with the code that's causing you to see this error: https://github.com/testing-library/dom-testing-library/issues/new`, + ) + } +} + +function checkContainerType(container) { + if ( + !container || + !(typeof container.querySelector === 'function') || + !(typeof container.querySelectorAll === 'function') + ) { + throw new TypeError( + `Expected container to be an Element, a Document or a DocumentFragment but got ${getTypeName( + container, + )}.`, + ) + } + + function getTypeName(object) { + if (typeof object === 'object') { + return object === null ? 'null' : object.constructor.name + } + return typeof object + } +} + +export { + getWindowFromNode, + getDocument, + checkContainerType, + jestFakeTimersAreEnabled, + TEXT_NODE, +} diff --git a/src/pure.js b/src/pure.js index 091af4b6..a25000ea 100644 --- a/src/pure.js +++ b/src/pure.js @@ -4,11 +4,11 @@ import { getQueriesForElement, prettyDOM, configure as configureDTL, - waitFor as waitForDTL, waitForElementToBeRemoved as waitForElementToBeRemovedDTL, } from '@testing-library/dom' import act from './act-compat' import {fireEvent} from './fire-event' +import {waitFor} from './wait-for' configureDTL({ eventWrapper: cb => { @@ -192,16 +192,6 @@ 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 diff --git a/src/wait-for.js b/src/wait-for.js new file mode 100644 index 00000000..4c765696 --- /dev/null +++ b/src/wait-for.js @@ -0,0 +1,201 @@ +// @source @testing-library/dom/src/wait-for.js +import {getConfig} from '@testing-library/dom' +import { + getWindowFromNode, + getDocument, + jestFakeTimersAreEnabled, + // We import these from the helpers rather than using the global version + // because these will be *real* timers, regardless of whether we're in + // an environment that's faked the timers out. + checkContainerType, +} from './dtlHelpers' + +// Not supported for external libraries. Only supported internally in @testing-library/dom +function runWithExpensiveErrorDiagnosticsDisabled(callback) { + return callback() +} + +// This is so the stack trace the developer sees is one that's +// closer to their code (because async stack traces are hard to follow). +function copyStackTrace(target, source) { + target.stack = source.stack.replace(source.message, target.message) +} + +function waitFor( + callback, + { + container = getDocument(), + timeout = getConfig().asyncUtilTimeout, + showOriginalStackTrace = getConfig().showOriginalStackTrace, + stackTraceError, + interval = 50, + onTimeout = error => { + error.message = getConfig().getElementError( + error.message, + container, + ).message + return error + }, + mutationObserverOptions = { + subtree: true, + childList: true, + attributes: true, + characterData: true, + }, + }, +) { + if (typeof callback !== 'function') { + throw new TypeError('Received `callback` arg must be a function') + } + + return new Promise(async (resolve, reject) => { + let lastError, intervalId, observer + let finished = false + let promiseStatus = 'idle' + + const overallTimeoutTimer = setTimeout(handleTimeout, timeout) + + const usingJestFakeTimers = jestFakeTimersAreEnabled() + if (usingJestFakeTimers) { + checkCallback() + // this is a dangerous rule to disable because it could lead to an + // infinite loop. However, eslint isn't smart enough to know that we're + // setting finished inside `onDone` which will be called when we're done + // waiting or when we've timed out. + // eslint-disable-next-line no-unmodified-loop-condition + while (!finished) { + if (!jestFakeTimersAreEnabled()) { + const error = new Error( + `Changed from using fake timers to real timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to real timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`, + ) + if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError) + reject(error) + return + } + // we *could* (maybe should?) use `advanceTimersToNextTimer` but it's + // possible that could make this loop go on forever if someone is using + // third party code that's setting up recursive timers so rapidly that + // the user's timer's don't get a chance to resolve. So we'll advance + // by an interval instead. (We have a test for this case). + jest.advanceTimersByTime(interval) + + // It's really important that checkCallback is run *before* we flush + // in-flight promises. To be honest, I'm not sure why, and I can't quite + // think of a way to reproduce the problem in a test, but I spent + // an entire day banging my head against a wall on this. + checkCallback() + + // In this rare case, we *need* to wait for in-flight promises + // to resolve before continuing. We don't need to take advantage + // of parallelization so we're fine. + // https://stackoverflow.com/a/59243586/971592 + // eslint-disable-next-line no-await-in-loop + await new Promise(r => { + setTimeout(r, 0) + jest.advanceTimersByTime(0) + }) + } + } else { + try { + checkContainerType(container) + } catch (e) { + reject(e) + return + } + intervalId = setInterval(checkRealTimersCallback, interval) + const {MutationObserver} = getWindowFromNode(container) + observer = new MutationObserver(checkRealTimersCallback) + observer.observe(container, mutationObserverOptions) + checkCallback() + } + + function onDone(error, result) { + finished = true + clearTimeout(overallTimeoutTimer) + + if (!usingJestFakeTimers) { + clearInterval(intervalId) + observer.disconnect() + } + + if (error) { + reject(error) + } else { + resolve(result) + } + } + + function checkRealTimersCallback() { + if (jestFakeTimersAreEnabled()) { + const error = new Error( + `Changed from using real timers to fake timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to fake timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`, + ) + if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError) + return reject(error) + } else { + return checkCallback() + } + } + + function checkCallback() { + if (promiseStatus === 'pending') return + try { + const result = runWithExpensiveErrorDiagnosticsDisabled(callback) + if (typeof result?.then === 'function') { + promiseStatus = 'pending' + result.then( + resolvedValue => { + promiseStatus = 'resolved' + onDone(null, resolvedValue) + }, + rejectedValue => { + promiseStatus = 'rejected' + lastError = rejectedValue + }, + ) + } else { + onDone(null, result) + } + // If `callback` throws, wait for the next mutation, interval, or timeout. + } catch (error) { + // Save the most recent callback error to reject the promise with it in the event of a timeout + lastError = error + } + } + + function handleTimeout() { + let error + if (lastError) { + error = lastError + if ( + !showOriginalStackTrace && + error.name === 'TestingLibraryElementError' + ) { + copyStackTrace(error, stackTraceError) + } + } else { + error = new Error('Timed out in waitFor.') + if (!showOriginalStackTrace) { + copyStackTrace(error, stackTraceError) + } + } + onDone(onTimeout(error), null) + } + }) +} + +function waitForWrapper(callback, options) { + // create the error here so its stack trace is as close to the + // calling code as possible + const stackTraceError = new Error('STACK_TRACE_MESSAGE') + return getConfig().asyncWrapper(() => + waitFor(callback, {stackTraceError, ...options}), + ) +} + +export {waitForWrapper as waitFor} + +/* +eslint + max-lines-per-function: ["error", {"max": 200}], +*/ From a2a9d4a124b2f42d2ed080311c7a6a93abcd9c41 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sun, 5 Sep 2021 12:10:33 +0200 Subject: [PATCH 18/21] fix wait-for --- src/wait-for.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/wait-for.js b/src/wait-for.js index 4c765696..7598bf23 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -9,6 +9,7 @@ import { // an environment that's faked the timers out. checkContainerType, } from './dtlHelpers' +import act, {asyncAct} from './act-compat' // Not supported for external libraries. Only supported internally in @testing-library/dom function runWithExpensiveErrorDiagnosticsDisabled(callback) { @@ -77,7 +78,9 @@ function waitFor( // third party code that's setting up recursive timers so rapidly that // the user's timer's don't get a chance to resolve. So we'll advance // by an interval instead. (We have a test for this case). - jest.advanceTimersByTime(interval) + act(() => { + jest.advanceTimersByTime(interval) + }) // It's really important that checkCallback is run *before* we flush // in-flight promises. To be honest, I'm not sure why, and I can't quite @@ -90,9 +93,11 @@ function waitFor( // of parallelization so we're fine. // https://stackoverflow.com/a/59243586/971592 // eslint-disable-next-line no-await-in-loop - await new Promise(r => { - setTimeout(r, 0) - jest.advanceTimersByTime(0) + await asyncAct(async () => { + await new Promise(r => { + setTimeout(r, 0) + jest.advanceTimersByTime(0) + }) }) } } else { From 60395b4a5bf1bedb92fc45f1dc0d6d5c0e423ae5 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sun, 5 Sep 2021 14:41:15 +0200 Subject: [PATCH 19/21] Use advanceTimersWrapper --- jest.config.js | 8 +- package.json | 2 +- src/dtlHelpers.js | 88 -------------------- src/pure.js | 23 +++--- src/wait-for.js | 206 ---------------------------------------------- 5 files changed, 15 insertions(+), 312 deletions(-) delete mode 100644 src/dtlHelpers.js delete mode 100644 src/wait-for.js diff --git a/jest.config.js b/jest.config.js index 45d8fce9..680a9038 100644 --- a/jest.config.js +++ b/jest.config.js @@ -6,10 +6,10 @@ module.exports = Object.assign(jestConfig, { // 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: 82, - functions: 76, - lines: 81, - statements: 81, + branches: 80, + functions: 84, + lines: 89, + statements: 89, }, }, }) diff --git a/package.json b/package.json index 1cd283fb..dc6d0bec 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "license": "MIT", "dependencies": { "@babel/runtime": "^7.12.5", - "@testing-library/dom": "^8.0.0" + "@testing-library/dom": "https://pkg.csb.dev/testing-library/dom-testing-library/commit/9d12234d/@testing-library/dom" }, "devDependencies": { "@testing-library/jest-dom": "^5.11.6", diff --git a/src/dtlHelpers.js b/src/dtlHelpers.js deleted file mode 100644 index 26427ec9..00000000 --- a/src/dtlHelpers.js +++ /dev/null @@ -1,88 +0,0 @@ -// @source @testing-library/dom/src/helpers.js - -// Constant node.nodeType for text nodes, see: -// https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType#Node_type_constants -const TEXT_NODE = 3 - -function jestFakeTimersAreEnabled() { - /* istanbul ignore else */ - if (typeof jest !== 'undefined' && jest !== null) { - return ( - // legacy timers - setTimeout._isMockFunction === true || - // modern timers - Object.prototype.hasOwnProperty.call(setTimeout, 'clock') - ) - } - // istanbul ignore next - return false -} - -function getDocument() { - /* istanbul ignore if */ - if (typeof window === 'undefined') { - throw new Error('Could not find default container') - } - return window.document -} -function getWindowFromNode(node) { - if (node.defaultView) { - // node is document - return node.defaultView - } else if (node.ownerDocument && node.ownerDocument.defaultView) { - // node is a DOM node - return node.ownerDocument.defaultView - } else if (node.window) { - // node is window - return node.window - } else if (node.then instanceof Function) { - throw new Error( - `It looks like you passed a Promise object instead of a DOM node. Did you do something like \`fireEvent.click(screen.findBy...\` when you meant to use a \`getBy\` query \`fireEvent.click(screen.getBy...\`, or await the findBy query \`fireEvent.click(await screen.findBy...\`?`, - ) - } else if (Array.isArray(node)) { - throw new Error( - `It looks like you passed an Array instead of a DOM node. Did you do something like \`fireEvent.click(screen.getAllBy...\` when you meant to use a \`getBy\` query \`fireEvent.click(screen.getBy...\`?`, - ) - } else if ( - typeof node.debug === 'function' && - typeof node.logTestingPlaygroundURL === 'function' - ) { - throw new Error( - `It looks like you passed a \`screen\` object. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a query, e.g. \`fireEvent.click(screen.getBy..., \`?`, - ) - } else { - // The user passed something unusual to a calling function - throw new Error( - `Unable to find the "window" object for the given node. Please file an issue with the code that's causing you to see this error: https://github.com/testing-library/dom-testing-library/issues/new`, - ) - } -} - -function checkContainerType(container) { - if ( - !container || - !(typeof container.querySelector === 'function') || - !(typeof container.querySelectorAll === 'function') - ) { - throw new TypeError( - `Expected container to be an Element, a Document or a DocumentFragment but got ${getTypeName( - container, - )}.`, - ) - } - - function getTypeName(object) { - if (typeof object === 'object') { - return object === null ? 'null' : object.constructor.name - } - return typeof object - } -} - -export { - getWindowFromNode, - getDocument, - checkContainerType, - jestFakeTimersAreEnabled, - TEXT_NODE, -} diff --git a/src/pure.js b/src/pure.js index a25000ea..dc74e9a4 100644 --- a/src/pure.js +++ b/src/pure.js @@ -4,11 +4,9 @@ import { getQueriesForElement, prettyDOM, configure as configureDTL, - waitForElementToBeRemoved as waitForElementToBeRemovedDTL, } from '@testing-library/dom' import act from './act-compat' import {fireEvent} from './fire-event' -import {waitFor} from './wait-for' configureDTL({ eventWrapper: cb => { @@ -20,6 +18,15 @@ configureDTL({ }, }) +if (typeof React.startTransition !== undefined) { + configureDTL({ + unstable_advanceTimersWrapper: cb => { + return act(cb) + }, + asyncWrapper: cb => cb(), + }) +} + // 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) /** @@ -192,19 +199,9 @@ function cleanup() { mountedContainers.clear() } -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, waitFor, waitForElementToBeRemoved} +export {render, cleanup, act, fireEvent} // 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/src/wait-for.js b/src/wait-for.js deleted file mode 100644 index 7598bf23..00000000 --- a/src/wait-for.js +++ /dev/null @@ -1,206 +0,0 @@ -// @source @testing-library/dom/src/wait-for.js -import {getConfig} from '@testing-library/dom' -import { - getWindowFromNode, - getDocument, - jestFakeTimersAreEnabled, - // We import these from the helpers rather than using the global version - // because these will be *real* timers, regardless of whether we're in - // an environment that's faked the timers out. - checkContainerType, -} from './dtlHelpers' -import act, {asyncAct} from './act-compat' - -// Not supported for external libraries. Only supported internally in @testing-library/dom -function runWithExpensiveErrorDiagnosticsDisabled(callback) { - return callback() -} - -// This is so the stack trace the developer sees is one that's -// closer to their code (because async stack traces are hard to follow). -function copyStackTrace(target, source) { - target.stack = source.stack.replace(source.message, target.message) -} - -function waitFor( - callback, - { - container = getDocument(), - timeout = getConfig().asyncUtilTimeout, - showOriginalStackTrace = getConfig().showOriginalStackTrace, - stackTraceError, - interval = 50, - onTimeout = error => { - error.message = getConfig().getElementError( - error.message, - container, - ).message - return error - }, - mutationObserverOptions = { - subtree: true, - childList: true, - attributes: true, - characterData: true, - }, - }, -) { - if (typeof callback !== 'function') { - throw new TypeError('Received `callback` arg must be a function') - } - - return new Promise(async (resolve, reject) => { - let lastError, intervalId, observer - let finished = false - let promiseStatus = 'idle' - - const overallTimeoutTimer = setTimeout(handleTimeout, timeout) - - const usingJestFakeTimers = jestFakeTimersAreEnabled() - if (usingJestFakeTimers) { - checkCallback() - // this is a dangerous rule to disable because it could lead to an - // infinite loop. However, eslint isn't smart enough to know that we're - // setting finished inside `onDone` which will be called when we're done - // waiting or when we've timed out. - // eslint-disable-next-line no-unmodified-loop-condition - while (!finished) { - if (!jestFakeTimersAreEnabled()) { - const error = new Error( - `Changed from using fake timers to real timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to real timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`, - ) - if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError) - reject(error) - return - } - // we *could* (maybe should?) use `advanceTimersToNextTimer` but it's - // possible that could make this loop go on forever if someone is using - // third party code that's setting up recursive timers so rapidly that - // the user's timer's don't get a chance to resolve. So we'll advance - // by an interval instead. (We have a test for this case). - act(() => { - jest.advanceTimersByTime(interval) - }) - - // It's really important that checkCallback is run *before* we flush - // in-flight promises. To be honest, I'm not sure why, and I can't quite - // think of a way to reproduce the problem in a test, but I spent - // an entire day banging my head against a wall on this. - checkCallback() - - // In this rare case, we *need* to wait for in-flight promises - // to resolve before continuing. We don't need to take advantage - // of parallelization so we're fine. - // https://stackoverflow.com/a/59243586/971592 - // eslint-disable-next-line no-await-in-loop - await asyncAct(async () => { - await new Promise(r => { - setTimeout(r, 0) - jest.advanceTimersByTime(0) - }) - }) - } - } else { - try { - checkContainerType(container) - } catch (e) { - reject(e) - return - } - intervalId = setInterval(checkRealTimersCallback, interval) - const {MutationObserver} = getWindowFromNode(container) - observer = new MutationObserver(checkRealTimersCallback) - observer.observe(container, mutationObserverOptions) - checkCallback() - } - - function onDone(error, result) { - finished = true - clearTimeout(overallTimeoutTimer) - - if (!usingJestFakeTimers) { - clearInterval(intervalId) - observer.disconnect() - } - - if (error) { - reject(error) - } else { - resolve(result) - } - } - - function checkRealTimersCallback() { - if (jestFakeTimersAreEnabled()) { - const error = new Error( - `Changed from using real timers to fake timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to fake timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`, - ) - if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError) - return reject(error) - } else { - return checkCallback() - } - } - - function checkCallback() { - if (promiseStatus === 'pending') return - try { - const result = runWithExpensiveErrorDiagnosticsDisabled(callback) - if (typeof result?.then === 'function') { - promiseStatus = 'pending' - result.then( - resolvedValue => { - promiseStatus = 'resolved' - onDone(null, resolvedValue) - }, - rejectedValue => { - promiseStatus = 'rejected' - lastError = rejectedValue - }, - ) - } else { - onDone(null, result) - } - // If `callback` throws, wait for the next mutation, interval, or timeout. - } catch (error) { - // Save the most recent callback error to reject the promise with it in the event of a timeout - lastError = error - } - } - - function handleTimeout() { - let error - if (lastError) { - error = lastError - if ( - !showOriginalStackTrace && - error.name === 'TestingLibraryElementError' - ) { - copyStackTrace(error, stackTraceError) - } - } else { - error = new Error('Timed out in waitFor.') - if (!showOriginalStackTrace) { - copyStackTrace(error, stackTraceError) - } - } - onDone(onTimeout(error), null) - } - }) -} - -function waitForWrapper(callback, options) { - // create the error here so its stack trace is as close to the - // calling code as possible - const stackTraceError = new Error('STACK_TRACE_MESSAGE') - return getConfig().asyncWrapper(() => - waitFor(callback, {stackTraceError, ...options}), - ) -} - -export {waitForWrapper as waitFor} - -/* -eslint - max-lines-per-function: ["error", {"max": 200}], -*/ From e9f188c305e425fb3ac1fb3aae94eca270dcc392 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 11 Sep 2021 12:02:21 +0200 Subject: [PATCH 20/21] Use latest DTL version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index dc6d0bec..cbf61367 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "license": "MIT", "dependencies": { "@babel/runtime": "^7.12.5", - "@testing-library/dom": "https://pkg.csb.dev/testing-library/dom-testing-library/commit/9d12234d/@testing-library/dom" + "@testing-library/dom": "^8.5.0" }, "devDependencies": { "@testing-library/jest-dom": "^5.11.6", From f9c851e38ce41314273aa2861f04b80ba18ff76b Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sun, 12 Sep 2021 12:10:46 +0200 Subject: [PATCH 21/21] Document expected "missing act" --- src/__tests__/end-to-end.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index cf222aec..787a944d 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -17,6 +17,8 @@ function ComponentWithLoader() { let cancelled = false fetchAMessage().then(data => { if (!cancelled) { + // Will trigger "missing act" warnings in React 18 with real timers + // Need to wait for an action on https://github.com/reactwg/react-18/discussions/23#discussioncomment-1087897 setState({data, loading: false}) } })