Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop usage of react-dom/test-utils #1270

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
fail-fast: false
matrix:
node: [14, 16, 18]
react: [latest, canary, experimental]
react: ['18.x', latest, canary, experimental]
runs-on: ubuntu-latest
steps:
- name: ⬇️ Checkout repo
Expand Down
16 changes: 16 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const {jest: jestConfig} = require('kcd-scripts/config')

module.exports = Object.assign(jestConfig, {
coverageThreshold: {
...jestConfig.coverageThreshold,
// Full coverage across the build matrix (React 18, 19) but not in a single job
// Ful coverage is checked via codecov
'./src/act-compat.js': {
// minimum coverage of jobs using React 18 and 19
branches: 90,
functions: 100,
lines: 100,
statements: 100,
},
},
})
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@
"typescript": "^4.1.2"
},
"peerDependencies": {
"react": "^18.0.0",
"react-dom": "^18.0.0"
"react": "^18.0.0 || ^18.3.0-canary",
"react-dom": "^18.0.0 || ^18.3.0-canary"
},
"eslintConfig": {
"extends": "./node_modules/kcd-scripts/eslint.js",
Expand Down
13 changes: 8 additions & 5 deletions src/__tests__/new-act.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
let asyncAct

jest.mock('react-dom/test-utils', () => ({
act: cb => {
return cb()
},
}))
jest.mock('react', () => {
return {
...jest.requireActual('react'),
act: cb => {
return cb()
},
}
})

beforeEach(() => {
jest.resetModules()
Expand Down
25 changes: 0 additions & 25 deletions src/__tests__/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,29 +212,4 @@ describe('render API', () => {

expect(wrapperComponentMountEffect).toHaveBeenCalledTimes(1)
})

test('legacyRoot uses legacy ReactDOM.render', () => {
expect(() => {
render(<div />, {legacyRoot: true})
}).toErrorDev(
[
"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",
],
{withoutStack: true},
)
})

test('legacyRoot uses legacy ReactDOM.hydrate', () => {
const ui = <div />
const container = document.createElement('div')
container.innerHTML = ReactDOMServer.renderToString(ui)
expect(() => {
render(ui, {container, hydrate: true, legacyRoot: true})
}).toErrorDev(
[
"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",
],
{withoutStack: true},
)
})
})
25 changes: 0 additions & 25 deletions src/__tests__/renderHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,28 +60,3 @@ test('allows wrapper components', async () => {

expect(result.current).toEqual('provided')
})

test('legacyRoot uses legacy ReactDOM.render', () => {
const Context = React.createContext('default')
function Wrapper({children}) {
return <Context.Provider value="provided">{children}</Context.Provider>
}
let result
expect(() => {
result = renderHook(
() => {
return React.useContext(Context)
},
{
wrapper: Wrapper,
legacyRoot: true,
},
).result
}).toErrorDev(
[
"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",
],
{withoutStack: true},
)
expect(result.current).toEqual('provided')
})
7 changes: 4 additions & 3 deletions src/act-compat.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as testUtils from 'react-dom/test-utils'
import * as React from 'react'
import * as DeprecatedReactTestUtils from 'react-dom/test-utils'

const domAct = testUtils.act
const reactAct = React.act ?? DeprecatedReactTestUtils.act

function getGlobalThis() {
/* istanbul ignore else */
Expand Down Expand Up @@ -78,7 +79,7 @@ function withGlobalActEnvironment(actImplementation) {
}
}

const act = withGlobalActEnvironment(domAct)
const act = withGlobalActEnvironment(reactAct)

export default act
export {
Expand Down
105 changes: 26 additions & 79 deletions src/pure.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react'
import ReactDOM from 'react-dom'
import * as ReactDOMClient from 'react-dom/client'
import {
getQueriesForElement,
Expand Down Expand Up @@ -73,7 +72,7 @@ configureDTL({
*/
const mountedContainers = new Set()
/**
* @type Array<{container: import('react-dom').Container, root: ReturnType<typeof createConcurrentRoot>}>
* @type Array<{container: import('react-dom').Container, root: ReturnType<typeof createTestRoot>}>
*/
const mountedRootEntries = []

Expand All @@ -89,73 +88,10 @@ function wrapUiIfNeeded(innerElement, wrapperComponent) {
: innerElement
}

function createConcurrentRoot(
container,
{hydrate, ui, wrapper: WrapperComponent},
) {
let root
if (hydrate) {
act(() => {
root = ReactDOMClient.hydrateRoot(
container,
strictModeIfNeeded(wrapUiIfNeeded(ui, WrapperComponent)),
)
})
} else {
root = ReactDOMClient.createRoot(container)
}

return {
hydrate() {
/* istanbul ignore if */
if (!hydrate) {
throw new Error(
'Attempted to hydrate a non-hydrateable root. This is a bug in `@testing-library/react`.',
)
}
// Nothing to do since hydration happens when creating the root object.
},
render(element) {
root.render(element)
},
unmount() {
root.unmount()
},
}
}

function createLegacyRoot(container) {
return {
hydrate(element) {
ReactDOM.hydrate(element, container)
},
render(element) {
ReactDOM.render(element, container)
},
unmount() {
ReactDOM.unmountComponentAtNode(container)
},
}
}

function renderRoot(
function createTestView(
ui,
{baseElement, container, hydrate, queries, root, wrapper: WrapperComponent},
{baseElement, container, queries, root, WrapperComponent},
) {
act(() => {
if (hydrate) {
root.hydrate(
strictModeIfNeeded(wrapUiIfNeeded(ui, WrapperComponent)),
container,
)
} else {
root.render(
strictModeIfNeeded(wrapUiIfNeeded(ui, WrapperComponent)),
container,
)
}
})

return {
container,
baseElement,
Expand All @@ -171,11 +107,10 @@ function renderRoot(
})
},
rerender: rerenderUi => {
renderRoot(rerenderUi, {
container,
baseElement,
root,
wrapper: WrapperComponent,
act(() => {
root.render(
strictModeIfNeeded(wrapUiIfNeeded(rerenderUi, WrapperComponent)),
)
})
// 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
Expand All @@ -201,10 +136,9 @@ function render(
{
container,
baseElement = container,
legacyRoot = false,
queries,
hydrate = false,
wrapper,
wrapper: WrapperComponent,
} = {},
) {
if (!baseElement) {
Expand All @@ -219,8 +153,16 @@ 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 ? createLegacyRoot : createConcurrentRoot
root = createRootImpl(container, {hydrate, ui, wrapper})
if (hydrate) {
act(() => {
root = ReactDOMClient.hydrateRoot(
container,
strictModeIfNeeded(wrapUiIfNeeded(ui, WrapperComponent)),
)
})
} else {
root = ReactDOMClient.createRoot(container)
}

mountedRootEntries.push({container, root})
// we'll add it to the mounted containers regardless of whether it's actually
Expand All @@ -238,12 +180,17 @@ function render(
})
}

return renderRoot(ui, {
if (!hydrate) {
act(() => {
root.render(strictModeIfNeeded(wrapUiIfNeeded(ui, WrapperComponent)))
})
}

return createTestView(ui, {
container,
baseElement,
queries,
hydrate,
wrapper,
WrapperComponent,
root,
})
}
Expand Down
1 change: 1 addition & 0 deletions tests/shouldIgnoreConsoleError.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ module.exports = function shouldIgnoreConsoleError(format) {
// Ignore it too.
return true
}
// TODO: Suppress deprecation warning from react-dom/test-utils
}
}
// Looks legit
Expand Down
9 changes: 2 additions & 7 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,12 @@ export interface RenderOptions<
*/
baseElement?: BaseElement
/**
* If `hydrate` is set to `true`, then it will render with `ReactDOM.hydrate`. This may be useful if you are using server-side
* rendering and use ReactDOM.hydrate to mount your components.
* If `hydrate` is set to `true`, then it will create a root with `ReactDOMClient.hydrateRoot`. This may be useful if you are using server-side
* rendering and use ReactDOMClient.hydrateRoot to mount your components.
*
* @see https://testing-library.com/docs/react-testing-library/api/#hydrate)
*/
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 to bind. Overrides the default set from DOM Testing Library unless merged.
*
Expand Down
Loading