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

fix: Resolve warning using the wrong react-dom entrypoint #1018

Merged
merged 3 commits into from
Mar 4, 2022
Merged
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
5 changes: 3 additions & 2 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ 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:
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']
react: [latest, next, experimental]
react: [current, next, experimental]
runs-on: ubuntu-latest
steps:
- name: 🛑 Cancel Previous Runs
Expand All @@ -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
Expand Down
15 changes: 0 additions & 15 deletions jest.config.js

This file was deleted.

8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caret range would match 18.0.0-rc.0-next which is definitely not what we want.
Will widen to ^18.0.0 once React 18 lands.

"react-dom": "18.0.0-rc.1"
},
"eslintConfig": {
"extends": "./node_modules/kcd-scripts/eslint.js",
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/new-act.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {})
})

Expand Down
100 changes: 0 additions & 100 deletions src/__tests__/no-act.js

This file was deleted.

142 changes: 0 additions & 142 deletions src/__tests__/old-act.js

This file was deleted.

61 changes: 44 additions & 17 deletions src/__tests__/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,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 ReactDOM.createRoot === 'function'

const performConcurrentRender = () => render(<div />, {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')

Expand Down Expand Up @@ -168,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 = <div />
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(<div />, {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 = <div />
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",
)
})
Loading