From d30b895a7ee0843a6e778a6552f8b4c00b4fe7ac Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 1 Mar 2022 15:36:54 +0100 Subject: [PATCH 1/3] fix: Use new react-dom/client entrypoint --- package.json | 6 ++++++ src/__tests__/render.js | 3 ++- src/pure.js | 9 +++++---- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 55b31ec0..bef807f7 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,12 @@ "react/no-adjacent-inline-elements": "off", "import/no-unassigned-import": "off", "import/named": "off", + "import/no-unresolved": [ + "error", + { + "ignore": "react-dom/client" + } + ], "testing-library/no-container": "off", "testing-library/no-dom-import": "off", "testing-library/no-unnecessary-act": "off", diff --git a/src/__tests__/render.js b/src/__tests__/render.js index d5f78b57..340082a5 100644 --- a/src/__tests__/render.js +++ b/src/__tests__/render.js @@ -1,5 +1,6 @@ import * as React from 'react' import ReactDOM from 'react-dom' +import ReactDOMClient from 'react-dom/client' import ReactDOMServer from 'react-dom/server' import {fireEvent, render, screen} from '../' @@ -110,7 +111,7 @@ test('flushes useEffect cleanup functions sync on unmount()', () => { }) test('throws if `legacyRoot: false` is used with an incomaptible version', () => { - const isConcurrentReact = typeof ReactDOM.createRoot === 'function' + const isConcurrentReact = typeof ReactDOMClient.createRoot === 'function' const performConcurrentRender = () => render(
, {legacyRoot: false}) diff --git a/src/pure.js b/src/pure.js index 309e2090..f2a188a4 100644 --- a/src/pure.js +++ b/src/pure.js @@ -1,5 +1,6 @@ import * as React from 'react' import ReactDOM from 'react-dom' +import * as ReactDOMClient from 'react-dom/client' import { getQueriesForElement, prettyDOM, @@ -64,7 +65,7 @@ function createConcurrentRoot( container, {hydrate, ui, wrapper: WrapperComponent}, ) { - if (typeof ReactDOM.createRoot !== 'function') { + if (typeof ReactDOMClient.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).'`, ) @@ -72,13 +73,13 @@ function createConcurrentRoot( let root if (hydrate) { act(() => { - root = ReactDOM.hydrateRoot( + root = ReactDOMClient.hydrateRoot( container, WrapperComponent ? React.createElement(WrapperComponent, null, ui) : ui, ) }) } else { - root = ReactDOM.createRoot(container) + root = ReactDOMClient.createRoot(container) } return { @@ -175,7 +176,7 @@ function render( { container, baseElement = container, - legacyRoot = typeof ReactDOM.createRoot !== 'function', + legacyRoot = typeof ReactDOMClient.createRoot !== 'function', queries, hydrate = false, wrapper, From dfc5036da80608525e3f69359279b8291eb6b95d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 1 Mar 2022 15:59:29 +0100 Subject: [PATCH 2/3] proper lint config --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index bef807f7..a55e8797 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,9 @@ "import/no-unresolved": [ "error", { - "ignore": "react-dom/client" + "ignore": [ + "react-dom/client" + ] } ], "testing-library/no-container": "off", From f8e434fab148427ef4f30ddfc45343a0050001fd Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 1 Mar 2022 17:04:28 +0100 Subject: [PATCH 3/3] Drop support for React < 18.0.0-rc.1 --- .github/workflows/validate.yml | 5 +- jest.config.js | 15 ---- package.json | 16 +--- src/__tests__/new-act.js | 2 +- src/__tests__/no-act.js | 100 ----------------------- src/__tests__/old-act.js | 142 --------------------------------- src/__tests__/render.js | 62 +++++++++----- src/act-compat.js | 128 +---------------------------- src/pure.js | 53 ++++-------- 9 files changed, 70 insertions(+), 453 deletions(-) delete mode 100644 jest.config.js delete mode 100644 src/__tests__/no-act.js delete mode 100644 src/__tests__/old-act.js diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 45cc7d13..aafa6cad 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -12,7 +12,7 @@ on: pull_request: {} jobs: main: - continue-on-error: ${{ matrix.react != 'latest' }} + continue-on-error: ${{ matrix.react != 'current' }} # ignore all-contributors PRs if: ${{ !contains(github.head_ref, 'all-contributors') }} strategy: @@ -20,7 +20,7 @@ jobs: 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'] - react: [latest, next, experimental] + react: [current, next, experimental] runs-on: ubuntu-latest steps: - name: 🛑 Cancel Previous Runs @@ -47,6 +47,7 @@ jobs: # https://reactjs.org/blog/2019/10/22/react-release-channels.html#using-the-next-channel-for-integration-testing - name: ⚛️ Setup react run: npm install react@${{ matrix.react }} react-dom@${{ matrix.react }} + if: ${{ matrix.react != 'current' }} - name: ▶️ Run validate script run: npm run validate diff --git a/jest.config.js b/jest.config.js deleted file mode 100644 index f30f76e9..00000000 --- a/jest.config.js +++ /dev/null @@ -1,15 +0,0 @@ -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: 75, - functions: 78, - lines: 76, - statements: 76, - }, - }, -}) diff --git a/package.json b/package.json index a55e8797..9b9d5ebf 100644 --- a/package.json +++ b/package.json @@ -54,14 +54,14 @@ "dotenv-cli": "^4.0.0", "kcd-scripts": "^11.1.0", "npm-run-all": "^4.1.5", - "react": "^17.0.1", - "react-dom": "^17.0.1", + "react": "18.0.0-rc.1", + "react-dom": "18.0.0-rc.1", "rimraf": "^3.0.2", "typescript": "^4.1.2" }, "peerDependencies": { - "react": "*", - "react-dom": "*" + "react": "18.0.0-rc.1", + "react-dom": "18.0.0-rc.1" }, "eslintConfig": { "extends": "./node_modules/kcd-scripts/eslint.js", @@ -70,14 +70,6 @@ "react/no-adjacent-inline-elements": "off", "import/no-unassigned-import": "off", "import/named": "off", - "import/no-unresolved": [ - "error", - { - "ignore": [ - "react-dom/client" - ] - } - ], "testing-library/no-container": "off", "testing-library/no-dom-import": "off", "testing-library/no-unnecessary-act": "off", diff --git a/src/__tests__/new-act.js b/src/__tests__/new-act.js index 42552594..05f9d45a 100644 --- a/src/__tests__/new-act.js +++ b/src/__tests__/new-act.js @@ -8,7 +8,7 @@ jest.mock('react-dom/test-utils', () => ({ beforeEach(() => { jest.resetModules() - asyncAct = require('../act-compat').asyncAct + asyncAct = require('../act-compat').default jest.spyOn(console, 'error').mockImplementation(() => {}) }) diff --git a/src/__tests__/no-act.js b/src/__tests__/no-act.js deleted file mode 100644 index de4117bb..00000000 --- a/src/__tests__/no-act.js +++ /dev/null @@ -1,100 +0,0 @@ -let act, asyncAct, React, consoleErrorMock - -beforeEach(() => { - jest.resetModules() - act = require('../pure').act - asyncAct = require('../act-compat').asyncAct - React = require('react') - consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => {}) -}) - -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() - act(callback) - expect(callback).toHaveBeenCalledTimes(1) - expect(console.error).toHaveBeenCalledTimes( - // ReactDOM.render is deprecated in React 18 - React.version.startsWith('18') ? 1 : 0, - ) -}) - -test('async act works when it does not exist (older versions of react)', async () => { - const callback = jest.fn() - await asyncAct(async () => { - await Promise.resolve() - await callback() - }) - expect(console.error).toHaveBeenCalledTimes( - // ReactDOM.render is deprecated in React 18 - React.version.startsWith('18') ? 2 : 0, - ) - expect(callback).toHaveBeenCalledTimes(1) - - callback.mockClear() - console.error.mockClear() - - await asyncAct(async () => { - await Promise.resolve() - await callback() - }) - expect(console.error).toHaveBeenCalledTimes( - // ReactDOM.render is deprecated in React 18 - React.version.startsWith('18') ? 2 : 0, - ) - expect(callback).toHaveBeenCalledTimes(1) -}) - -test('async act recovers from errors', async () => { - try { - await asyncAct(async () => { - await null - throw new Error('test error') - }) - } catch (err) { - console.error('call console.error') - } - expect(console.error).toHaveBeenCalledTimes( - // ReactDOM.render is deprecated in React 18 - React.version.startsWith('18') ? 2 : 1, - ) - expect( - console.error.mock.calls[ - // ReactDOM.render is deprecated in React 18 - React.version.startsWith('18') ? 1 : 0 - ][0], - ).toMatch('call console.error') -}) - -test('async act recovers from sync errors', async () => { - try { - await asyncAct(() => { - throw new Error('test error') - }) - } catch (err) { - console.error('call console.error') - } - expect(console.error).toHaveBeenCalledTimes(1) - expect(console.error.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - call console.error, - ], - ] - `) -}) - -/* eslint no-console:0 */ diff --git a/src/__tests__/old-act.js b/src/__tests__/old-act.js deleted file mode 100644 index 0153fea3..00000000 --- a/src/__tests__/old-act.js +++ /dev/null @@ -1,142 +0,0 @@ -let asyncAct - -beforeEach(() => { - jest.resetModules() - asyncAct = require('../act-compat').asyncAct - jest.spyOn(console, 'error').mockImplementation(() => {}) -}) - -afterEach(() => { - console.error.mockRestore() -}) - -jest.mock('react-dom/test-utils', () => ({ - act: cb => { - cb() - return { - then() { - console.error( - 'Warning: Do not await the result of calling ReactTestUtils.act(...), it is not a Promise.', - ) - }, - } - }, -})) - -test('async act works even when the act is an old one', async () => { - const callback = jest.fn() - await asyncAct(async () => { - console.error('sigil') - await Promise.resolve() - await callback() - console.error('sigil') - }) - expect(console.error.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - sigil, - ], - Array [ - It looks like you're using a version of react-dom that supports the "act" function, but not an awaitable version of "act" which you will need. Please upgrade to at least react-dom@16.9.0 to remove this warning., - ], - Array [ - sigil, - ], - ] - `) - expect(callback).toHaveBeenCalledTimes(1) - - // and it doesn't warn you twice - callback.mockClear() - console.error.mockClear() - - await asyncAct(async () => { - await Promise.resolve() - await callback() - }) - expect(console.error).toHaveBeenCalledTimes(0) - expect(callback).toHaveBeenCalledTimes(1) -}) - -test('async act recovers from async errors', async () => { - try { - await asyncAct(async () => { - await null - throw new Error('test error') - }) - } catch (err) { - console.error('call console.error') - } - expect(console.error).toHaveBeenCalledTimes(2) - expect(console.error.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - It looks like you're using a version of react-dom that supports the "act" function, but not an awaitable version of "act" which you will need. Please upgrade to at least react-dom@16.9.0 to remove this warning., - ], - Array [ - call console.error, - ], - ] - `) -}) - -test('async act recovers from sync errors', async () => { - try { - await asyncAct(() => { - throw new Error('test error') - }) - } catch (err) { - console.error('call console.error') - } - expect(console.error).toHaveBeenCalledTimes(1) - expect(console.error.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - call console.error, - ], - ] - `) -}) - -test('async act can handle any sort of console.error', async () => { - await asyncAct(async () => { - console.error({error: 'some error'}) - await null - }) - - expect(console.error).toHaveBeenCalledTimes(2) - expect(console.error.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - Object { - error: some error, - }, - ], - Array [ - It looks like you're using a version of react-dom that supports the "act" function, but not an awaitable version of "act" which you will need. Please upgrade to at least react-dom@16.9.0 to remove this warning., - ], - ] - `) -}) - -test('async act should not show an error when ReactTestUtils.act returns something', async () => { - jest.resetModules() - jest.mock('react-dom/test-utils', () => ({ - act: () => { - return new Promise(resolve => { - console.error( - 'Warning: The callback passed to ReactTestUtils.act(...) function must not return anything', - ) - resolve() - }) - }, - })) - asyncAct = require('../act-compat').asyncAct - await asyncAct(async () => { - await null - }) - - expect(console.error).toHaveBeenCalledTimes(0) -}) - -/* eslint no-console:0 */ diff --git a/src/__tests__/render.js b/src/__tests__/render.js index 340082a5..88e2b98d 100644 --- a/src/__tests__/render.js +++ b/src/__tests__/render.js @@ -1,6 +1,5 @@ import * as React from 'react' import ReactDOM from 'react-dom' -import ReactDOMClient from 'react-dom/client' import ReactDOMServer from 'react-dom/server' import {fireEvent, render, screen} from '../' @@ -110,23 +109,6 @@ 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 ReactDOMClient.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') @@ -169,3 +151,47 @@ test('hydrate will make the UI interactive', () => { expect(container).toHaveTextContent('clicked:1') }) + +test('hydrate can have a wrapper', () => { + const wrapperComponentMountEffect = jest.fn() + function WrapperComponent({children}) { + React.useEffect(() => { + wrapperComponentMountEffect() + }) + + return children + } + const ui =
+ const container = document.createElement('div') + document.body.appendChild(container) + container.innerHTML = ReactDOMServer.renderToString(ui) + + render(ui, {container, hydrate: true, wrapper: WrapperComponent}) + + expect(wrapperComponentMountEffect).toHaveBeenCalledTimes(1) +}) + +test('legacyRoot uses legacy ReactDOM.render', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}) + render(
, {legacyRoot: true}) + + expect(console.error).toHaveBeenCalledTimes(1) + expect(console.error).toHaveBeenNthCalledWith( + 1, + "Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://reactjs.org/link/switch-to-createroot", + ) +}) + +test('legacyRoot uses legacy ReactDOM.hydrate', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}) + const ui =
+ const container = document.createElement('div') + container.innerHTML = ReactDOMServer.renderToString(ui) + render(ui, {container, hydrate: true, legacyRoot: true}) + + expect(console.error).toHaveBeenCalledTimes(1) + expect(console.error).toHaveBeenNthCalledWith( + 1, + "Warning: ReactDOM.hydrate is no longer supported in React 18. Use hydrateRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://reactjs.org/link/switch-to-createroot", + ) +}) diff --git a/src/act-compat.js b/src/act-compat.js index 17e5b05c..d7a09d68 100644 --- a/src/act-compat.js +++ b/src/act-compat.js @@ -1,18 +1,6 @@ -import * as React from 'react' -import ReactDOM from 'react-dom' import * as testUtils from 'react-dom/test-utils' 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 -// we do this little polyfill. No warnings, but it's -// better than nothing. -function actPolyfill(cb) { - ReactDOM.unstable_batchedUpdates(cb) - ReactDOM.render(
, document.createElement('div')) -} function getGlobalThis() { /* istanbul ignore else */ @@ -86,124 +74,10 @@ function withGlobalActEnvironment(actImplementation) { } } -const act = withGlobalActEnvironment(domAct || actPolyfill) - -let youHaveBeenWarned = false -let isAsyncActSupported = null - -function asyncAct(cb) { - if (actSupported === true) { - if (isAsyncActSupported === null) { - return new Promise((resolve, reject) => { - // patch console.error here - const originalConsoleError = console.error - console.error = function error(...args) { - /* if console.error fired *with that specific message* */ - /* istanbul ignore next */ - const firstArgIsString = typeof args[0] === 'string' - if ( - firstArgIsString && - args[0].indexOf( - 'Warning: Do not await the result of calling ReactTestUtils.act', - ) === 0 - ) { - // v16.8.6 - isAsyncActSupported = false - } else if ( - firstArgIsString && - args[0].indexOf( - 'Warning: The callback passed to ReactTestUtils.act(...) function must not return anything', - ) === 0 - ) { - // no-op - } else { - originalConsoleError.apply(console, args) - } - } - let cbReturn, result - try { - result = domAct(() => { - cbReturn = cb() - return cbReturn - }) - } catch (err) { - console.error = originalConsoleError - reject(err) - return - } - - result.then( - () => { - console.error = originalConsoleError - // if it got here, it means async act is supported - isAsyncActSupported = true - resolve() - }, - err => { - console.error = originalConsoleError - isAsyncActSupported = true - reject(err) - }, - ) - - // 16.8.6's act().then() doesn't call a resolve handler, so we need to manually flush here, sigh - - if (isAsyncActSupported === false) { - console.error = originalConsoleError - /* istanbul ignore next */ - if (!youHaveBeenWarned) { - // if act is supported and async act isn't and they're trying to use async - // act, then they need to upgrade from 16.8 to 16.9. - // This is a seamless upgrade, so we'll add a warning - console.error( - `It looks like you're using a version of react-dom that supports the "act" function, but not an awaitable version of "act" which you will need. Please upgrade to at least react-dom@16.9.0 to remove this warning.`, - ) - youHaveBeenWarned = true - } - - cbReturn.then(() => { - // a faux-version. - // todo - copy https://github.com/facebook/react/blob/master/packages/shared/enqueueTask.js - Promise.resolve().then(() => { - // use sync act to flush effects - act(() => {}) - resolve() - }) - }, reject) - } - }) - } else if (isAsyncActSupported === false) { - // use the polyfill directly - let result - act(() => { - result = cb() - }) - return result.then(() => { - return Promise.resolve().then(() => { - // use sync act to flush effects - act(() => {}) - }) - }) - } - // all good! regular act - return act(cb) - } - // use the polyfill - let result - act(() => { - result = cb() - }) - return result.then(() => { - return Promise.resolve().then(() => { - // use sync act to flush effects - act(() => {}) - }) - }) -} +const act = withGlobalActEnvironment(domAct) export default act export { - asyncAct, setIsReactActEnvironment as setReactActEnvironment, getIsReactActEnvironment, } diff --git a/src/pure.js b/src/pure.js index f2a188a4..64b761b0 100644 --- a/src/pure.js +++ b/src/pure.js @@ -7,19 +7,26 @@ import { configure as configureDTL, } from '@testing-library/dom' import act, { - asyncAct, getIsReactActEnvironment, setReactActEnvironment, } from './act-compat' import {fireEvent} from './fire-event' configureDTL({ + unstable_advanceTimersWrapper: cb => { + return act(cb) + }, + // We just want to run `waitFor` without IS_REACT_ACT_ENVIRONMENT + // But that's not necessarily how `asyncWrapper` is used since it's a public method. + // Let's just hope nobody else is using it. asyncWrapper: async cb => { - let result - await asyncAct(async () => { - result = await cb() - }) - return result + const previousActEnvironment = getIsReactActEnvironment() + setReactActEnvironment(false) + try { + return await cb() + } finally { + setReactActEnvironment(previousActEnvironment) + } }, eventWrapper: cb => { let result @@ -30,26 +37,6 @@ configureDTL({ }, }) -if (React.startTransition !== undefined) { - configureDTL({ - unstable_advanceTimersWrapper: cb => { - return act(cb) - }, - // We just want to run `waitFor` without IS_REACT_ACT_ENVIRONMENT - // But that's not necessarily how `asyncWrapper` is used since it's a public method. - // Let's just hope nobody else is using it. - asyncWrapper: async cb => { - const previousActEnvironment = getIsReactActEnvironment() - setReactActEnvironment(false) - try { - return await cb() - } finally { - setReactActEnvironment(previousActEnvironment) - } - }, - }) -} - // 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) /** @@ -65,11 +52,6 @@ function createConcurrentRoot( container, {hydrate, ui, wrapper: WrapperComponent}, ) { - if (typeof ReactDOMClient.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).'`, - ) - } let root if (hydrate) { act(() => { @@ -176,7 +158,7 @@ function render( { container, baseElement = container, - legacyRoot = typeof ReactDOMClient.createRoot !== 'function', + legacyRoot = false, queries, hydrate = false, wrapper, @@ -204,6 +186,9 @@ function render( mountedContainers.add(container) } else { mountedRootEntries.forEach(rootEntry => { + // Else is unreachable since `mountedContainers` has the `container`. + // Only reachable if one would accidentally add the container to `mountedContainers` but not the root to `mountedRootEntries` + /* istanbul ignore else */ if (rootEntry.container === container) { root = rootEntry.root } @@ -237,8 +222,4 @@ function cleanup() { export * from '@testing-library/dom' 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 -// people should just upgrade anyway. - /* eslint func-name-matching:0 */