Skip to content

[v36 Release] Update minimum version of react and react-dom to v18 #3240

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

Merged
merged 8 commits into from
Jun 8, 2023
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: 5 additions & 0 deletions .changeset/nasty-bottles-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': major
---

Update minimum version for react and react-dom to v18
26 changes: 0 additions & 26 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ jobs:
run: npm run lint:md

test:
strategy:
fail-fast: false
matrix:
react: [17, 18]
runs-on: ubuntu-latest
steps:
- name: Checkout repository
Expand All @@ -69,30 +65,16 @@ jobs:
node-version: 18
cache: 'npm'

- name: Set React version
run: node script/set-react-version.js ${{ matrix.react }}

- name: Install dependencies
if: ${{ matrix.react == 17 }}
run: npm install --legacy-peer-deps

- name: Install dependencies
if: ${{ matrix.react == 18 }}
run: npm ci

- name: Build
run: npm run build

- name: Test
run: npm run test -- --coverage
env:
REACT_VERSION_17: ${{ matrix.react == 17 }}

type-check:
strategy:
fail-fast: false
matrix:
react: [17, 18]
runs-on: ubuntu-latest
steps:
- name: Checkout repository
Expand All @@ -104,15 +86,7 @@ jobs:
node-version: 18
cache: 'npm'

- name: Set React version
run: node script/set-react-version.js ${{ matrix.react }}

- name: Install dependencies
if: ${{ matrix.react == 17 }}
run: npm install --legacy-peer-deps

- name: Install dependencies
if: ${{ matrix.react != 17 }}
run: npm ci

- name: Type check
Expand Down
6 changes: 0 additions & 6 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,10 @@

'use strict'

const {REACT_VERSION_17} = process.env

/**
* @type {import('jest').Config}
*/
module.exports = {
globals: {
REACT_VERSION_LATEST: REACT_VERSION_17 ? REACT_VERSION_17 !== 'true' : true,
REACT_VERSION_17: REACT_VERSION_17 === 'true',
},
testEnvironment: 'jsdom',
cacheDirectory: '.test',
collectCoverageFrom: ['src/**/*.{js,jsx,ts,tsx}', '!src/stories/**', '!**/*.stories.{js,jsx,ts,tsx}'],
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@
"yaml": "2.2.2"
},
"peerDependencies": {
"react": "^17.0.0 || ^18.0.0",
"react-dom": "^17.0.0 || ^18.0.0",
"react": "^18.0.0",
"react-dom": "^18.0.0",
"styled-components": "4.x || 5.x"
},
"prettier": "@github/prettier-config",
Expand Down
92 changes: 0 additions & 92 deletions script/set-react-version.js

This file was deleted.

11 changes: 0 additions & 11 deletions src/UnderlineNav2/UnderlineNav.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import {
import {UnderlineNav} from '.'
import {behavesAsComponent, checkExports, checkStoriesForAxeViolations} from '../utils/testing'

declare const REACT_VERSION_LATEST: boolean

// window.matchMedia() is not implemented by JSDOM so we have to create a mock:
// https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom
Object.defineProperty(window, 'matchMedia', {
Expand Down Expand Up @@ -101,7 +99,6 @@ describe('UnderlineNav', () => {
expect(nav.getElementsByTagName('svg').length).toEqual(7)
})
it('fires onSelect on click', async () => {
const spy = jest.spyOn(console, 'error').mockImplementation()
const onSelect = jest.fn()
const {getByRole} = render(
<UnderlineNav aria-label="Test Navigation">
Expand All @@ -114,14 +111,6 @@ describe('UnderlineNav', () => {
const user = userEvent.setup()
await user.click(item)
expect(onSelect).toHaveBeenCalledTimes(1)

if (REACT_VERSION_LATEST) {
expect(spy).not.toHaveBeenCalled()
} else {
// Warning: It looks like you're using the wrong act() around your test interactions
expect(spy).toHaveBeenCalledTimes(2)
}
spy.mockRestore()
})
it('fires onSelect on keypress', async () => {
const onSelect = jest.fn()
Expand Down
14 changes: 4 additions & 10 deletions src/__tests__/ConfirmationDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import {ThemeProvider} from '../ThemeProvider'
import {SSRProvider} from '../utils/ssr'
import {behavesAsComponent, checkExports} from '../utils/testing'

declare const REACT_VERSION_LATEST: boolean

const Basic = ({confirmButtonType}: Pick<React.ComponentProps<typeof ConfirmationDialog>, 'confirmButtonType'>) => {
const [isOpen, setIsOpen] = useState(false)
const buttonRef = useRef<HTMLButtonElement>(null)
Expand Down Expand Up @@ -126,14 +124,10 @@ describe('ConfirmationDialog', () => {
expect(getByText('Primary')).toEqual(document.activeElement)
expect(getByText('Secondary')).not.toEqual(document.activeElement)

// REACT_VERSION_LATEST should be treated as a constant for the test
// environment
if (REACT_VERSION_LATEST) {
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith(
expect.stringContaining('Warning: ReactDOM.render is no longer supported in React 18'),
)
}
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith(
expect.stringContaining('Warning: ReactDOM.render is no longer supported in React 18'),
)
spy.mockRestore()
})
})
14 changes: 2 additions & 12 deletions src/drafts/MarkdownEditor/MarkdownEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import {act} from 'react-dom/test-utils'
import MarkdownEditor, {MarkdownEditorHandle, MarkdownEditorProps, Mentionable, Reference, SavedReply} from '.'
import ThemeProvider from '../../ThemeProvider'

declare const REACT_VERSION_LATEST: boolean

type UncontrolledEditorProps = Omit<MarkdownEditorProps, 'value' | 'onChange' | 'onRenderPreview' | 'children'> &
Partial<Pick<MarkdownEditorProps, 'onChange' | 'onRenderPreview' | 'children'>> & {
hideLabel?: boolean
Expand Down Expand Up @@ -1147,11 +1145,7 @@ describe('MarkdownEditor', () => {
//
// At the moment, it doesn't seem clear how to appropriately wrap this
// interaction in an act() in order to cover this warning
if (REACT_VERSION_LATEST) {
expect(spy).toHaveBeenCalled()
} else {
expect(spy).not.toHaveBeenCalled()
}
expect(spy).toHaveBeenCalled()
expect(queryByRole('listbox')).toBeInTheDocument()

spy.mockClear()
Expand Down Expand Up @@ -1189,11 +1183,7 @@ describe('MarkdownEditor', () => {
// Note: this spy assertion for console.error() is for an act() violation.
// It's not clear where this act() violation is located as wrapping the
// above code does not address this.
if (REACT_VERSION_LATEST) {
expect(spy).toHaveBeenCalled()
} else {
expect(spy).not.toHaveBeenCalled()
}
expect(spy).toHaveBeenCalled()
spy.mockRestore()
})

Expand Down