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

test(react): remove support for testing React 17 #3800

Merged
merged 5 commits into from
Oct 12, 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
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
5 changes: 0 additions & 5 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,11 @@
'use strict'

const path = require('node:path')
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
14 changes: 4 additions & 10 deletions src/ConfirmationDialog/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 @@ -1212,11 +1210,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 @@ -1254,11 +1248,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
Loading