Add Vitest console enforcement#7844
Open
joshblack wants to merge 5 commits into
Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
github-actions
Bot
added
staff
Contributor
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds shared Vitest console enforcement through a private workspace package and wires it into the repository’s Vitest project configs, with related test updates to handle existing console output.
Changes:
- Adds
@primer/vitest-configwith sharedvitest-fail-on-consolesetup. - Updates React, styled-react, doc-gen, and postcss Vitest configs/package dependencies.
- Adjusts tests to wrap async updates, mock expected console output, or avoid console noise.
Show a summary per file
| File | Description |
|---|---|
packages/vitest-config/* |
New shared Vitest setup package for console enforcement. |
packages/*/vitest.config* |
Consumes shared setup in node/browser Vitest projects. |
packages/*/package.json |
Adds workspace dependency on shared Vitest config. |
packages/react/src/**/*.test.tsx |
Updates tests for console enforcement compatibility. |
package-lock.json |
Locks new workspace package and dependency updates. |
Copilot's findings
- Files reviewed: 39/40 changed files
- Comments generated: 12
Comment on lines
+16
to
+18
| /^Warning: Unexpected return value from a callback ref in .+\. A callback ref should not return a function\./.test( | ||
| message, | ||
| ) |
| }).toThrow( | ||
| 'The `Tooltip` component expects a single React element that contains interactive content. Consider using a `<button>` or equivalent interactive element instead.', | ||
| ) | ||
| }, 'The `Tooltip` component expects a single React element that contains interactive content. Consider using a `<button>` or equivalent interactive element instead.') |
Comment on lines
+66
to
+75
| function expectRenderError(callback: () => void, error: string | RegExp, expectedConsoleErrors = 3) { | ||
| const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) | ||
| try { | ||
| expect(callback).toThrow(error) | ||
| const messages = consoleError.mock.calls.map(args => args.map(String).join(' ')) | ||
| expect(messages).toHaveLength(expectedConsoleErrors) | ||
| if (typeof error === 'string') { | ||
| expect(messages.join('\n')).toContain(error) | ||
| } else { | ||
| expect(messages.join('\n')).toMatch(error) |
Comment on lines
+244
to
+248
| function expectRenderError(callback: () => void, error: string | RegExp, expectedConsoleErrors = 3) { | ||
| const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) | ||
| try { | ||
| expect(callback).toThrow(error) | ||
| const messages = consoleError.mock.calls.map(args => args.map(String).join(' ')) |
Comment on lines
+205
to
+209
| function expectRenderError(callback: () => void, error: string | RegExp, expectedConsoleErrors = 4) { | ||
| const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) | ||
| try { | ||
| expect(callback).toThrow(error) | ||
| const messages = consoleError.mock.calls.map(args => args.map(String).join(' ')) |
Comment on lines
+23
to
+26
| const messages = consoleError.mock.calls.map(args => args.map(String).join(' ')) | ||
| expect(messages).toHaveLength(1) | ||
| expect(messages[0]).toContain('non-boolean attribute') | ||
| expect(messages[0]).toContain('inline') |
Comment on lines
+312
to
+322
| expect(messages).toHaveLength(5) | ||
| expect( | ||
| messages.some(message => message.includes('React does not recognize') && message.includes('popoverTarget')), | ||
| ).toBe(true) | ||
| expect( | ||
| messages.every( | ||
| message => | ||
| (message.includes('React does not recognize') && message.includes('popoverTarget')) || | ||
| message.includes('Unexpected return value from a callback ref'), | ||
| ), | ||
| ).toBe(true) |
Comment on lines
+52
to
+62
| expect(messages).toHaveLength(7) | ||
| expect(messages.some(message => message.includes('React does not recognize') && message.includes('groupId'))).toBe( | ||
| true, | ||
| ) | ||
| expect( | ||
| messages.every( | ||
| message => | ||
| (message.includes('React does not recognize') && message.includes('groupId')) || | ||
| message.includes('Unexpected return value from a callback ref'), | ||
| ), | ||
| ).toBe(true) |
Comment on lines
+11
to
+19
| const {container} = render(<ActionList className="test-class" items={[]} />) | ||
|
|
||
| expect(container.firstElementChild).toHaveClass(classes.List) | ||
| expect(container.firstElementChild).toHaveClass('test-class') | ||
| const messages = consoleError.mock.calls.map(args => args.map(String).join(' ')) | ||
| expect(messages).toHaveLength(1) | ||
| expect(messages[0]).toContain('React does not recognize') | ||
| expect(messages[0]).toContain('groupId') | ||
| consoleError.mockRestore() |
Comment on lines
+164
to
+171
| expect(messages).toHaveLength(12) | ||
| expect( | ||
| messages.every( | ||
| message => | ||
| message.includes('Unexpected return value from a callback ref') || | ||
| message.includes('React does not recognize'), | ||
| ), | ||
| ).toBe(true) |
In React 19, errors thrown during rendering no longer trigger console.error the same number of times as React 18. Additionally, React 19 natively supports `popover`/`popoverTarget` attributes (removing the "React does not recognize" warning) and allows callback refs to return cleanup functions (removing the "Unexpected return value from a callback ref" warning). - Simplify `expectRenderError` in Group, Heading, UnderlineNav, Tooltip, and UnderlinePanels tests — keep `toThrow` assertion and console spy, remove brittle count/content checks - Update AnchoredOverlay, ConfirmationDialog, and deprecated ActionMenu tests — keep console spy to suppress React 18 prop warnings, remove version-specific count assertions Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com>
jonrohan
approved these changes
May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes N/A
Adds a shared
@primer/vitest-configpackage that enablesvitest-fail-on-consolein CI and viaPRIMER_TEST_FAIL_ON_CONSOLE=true, then updates the repository Vitest configurations to consume that shared setup.This also updates tests to explicitly handle expected console output or avoid interactions that produced console noise, without changing component source.
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
npm run buildPRIMER_TEST_FAIL_ON_CONSOLE=true npx vitest runnpm run type-checknpm run lintnpm run lint:cssnpm run format:diffMerge checklist