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

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
primer · Oct 12, 2023 · 341243f · 341243f
1 parent b99b3ee
commit 341243f
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 53 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -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
@@ -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
@@ -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

diff --git a/jest.config.js b/jest.config.js
@@ -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}'],

diff --git a/src/ConfirmationDialog/ConfirmationDialog.test.tsx b/src/ConfirmationDialog/ConfirmationDialog.test.tsx
@@ -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)
@@ -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()
   })
 })
diff --git a/src/drafts/MarkdownEditor/MarkdownEditor.test.tsx b/src/drafts/MarkdownEditor/MarkdownEditor.test.tsx
@@ -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
@@ -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()
@@ -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()
     })