Skip to content

Commit

Permalink
Update minimum version of react and react-dom to v18 (#3240)
Browse files Browse the repository at this point in the history
* feat(project): update React minimum version to 18

* chore: add changeset

* ci: remove type-check (17) option

* test: remove console.error spy

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
  • Loading branch information
joshblack and joshblack committed Jun 8, 2023
1 parent fb62eb5 commit 5d7d55b
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 161 deletions.
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

0 comments on commit 5d7d55b

Please sign in to comment.