From 956de8d841fcb2fde71464545f180320be6e0676 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 19:34:19 +0000 Subject: [PATCH 01/14] Plan console warning cleanup Agent-Logs-Url: https://github.com/primer/react/sessions/6f872a32-6ca6-4638-b31e-81b5134c5538 Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- package-lock.json | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7db0d43de0f..880c4ad5141 100644 --- a/package-lock.json +++ b/package-lock.json @@ -82,8 +82,8 @@ "react-dom": "^18.3.1" }, "devDependencies": { - "@primer/react": "38.21.1", - "@primer/styled-react": "1.0.6", + "@primer/react": "38.22.0", + "@primer/styled-react": "1.0.7", "@types/react": "^18.3.11", "@types/react-dom": "^18.3.0", "@vitejs/plugin-react": "^4.3.3", @@ -96,8 +96,8 @@ "name": "example-nextjs", "version": "0.0.0", "dependencies": { - "@primer/react": "38.21.1", - "@primer/styled-react": "1.0.6", + "@primer/react": "38.22.0", + "@primer/styled-react": "1.0.7", "next": "^16.1.7", "react": "^19.2.0", "react-dom": "^19.2.0", @@ -139,8 +139,8 @@ "version": "0.0.0", "dependencies": { "@primer/octicons-react": "^19.21.0", - "@primer/react": "38.21.1", - "@primer/styled-react": "1.0.6", + "@primer/react": "38.22.0", + "@primer/styled-react": "1.0.7", "clsx": "^2.1.1", "next": "^16.1.7", "react": "^19.2.0", @@ -27709,7 +27709,7 @@ }, "packages/react": { "name": "@primer/react", - "version": "38.21.1", + "version": "38.22.0", "license": "MIT", "dependencies": { "@github/mini-throttle": "^2.1.1", @@ -28082,7 +28082,7 @@ }, "packages/styled-react": { "name": "@primer/styled-react", - "version": "1.0.6", + "version": "1.0.7", "dependencies": { "@styled-system/css": "^5.1.5", "@styled-system/props": "^5.1.5", @@ -28099,7 +28099,7 @@ "@babel/preset-react": "^7.28.5", "@babel/preset-typescript": "^7.28.5", "@primer/primitives": "10.x || 11.x", - "@primer/react": "^38.20.0", + "@primer/react": "^38.22.0", "@rollup/plugin-babel": "^6.1.0", "@storybook/react-vite": "^10.3.3", "@types/react": "18.3.11", From 0a7400e9fcda82577b369cf76f3973995c1ab5a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 19:39:04 +0000 Subject: [PATCH 02/14] Add CI console failure guard Agent-Logs-Url: https://github.com/primer/react/sessions/6f872a32-6ca6-4638-b31e-81b5134c5538 Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- package-lock.json | 18 ++++++- package.json | 3 +- packages/doc-gen/vitest.config.mts | 1 + .../postcss-preset-primer/vitest.config.ts | 1 + packages/react/config/vitest/browser/setup.ts | 1 + .../src/AnchoredOverlay/AnchoredOverlay.tsx | 2 +- .../__tests__/Breadcrumbs.test.tsx | 50 +++++++++++-------- .../react/src/CircleBadge/CircleBadge.tsx | 15 ++++-- .../react/src/PageLayout/usePaneWidth.test.ts | 41 ++++++++------- .../react/src/deprecated/ActionList/Group.tsx | 2 +- .../hooks/__tests__/useMergedRefs.test.tsx | 24 ++++++--- packages/react/src/hooks/useMergedRefs.ts | 20 ++++---- packages/react/src/live-region/Announce.tsx | 8 +-- packages/react/vitest.config.mts | 1 + .../config/vitest/browser/setup.ts | 1 + .../styled-react/vitest.config.browser.ts | 1 + packages/styled-react/vitest.config.ts | 1 + script/vitest/setup.ts | 10 ++++ 18 files changed, 132 insertions(+), 68 deletions(-) create mode 100644 script/vitest/setup.ts diff --git a/package-lock.json b/package-lock.json index 880c4ad5141..e5b1b62b90b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -65,7 +65,8 @@ "turbo": "^2.6.3", "typescript": "^6.0.3", "typescript-eslint": "^8.59.1", - "vitest": "^4.1.5" + "vitest": "^4.1.5", + "vitest-fail-on-console": "^0.10.1" }, "engines": { "node": ">=12", @@ -26684,6 +26685,21 @@ } } }, + "node_modules/vitest-fail-on-console": { + "version": "0.10.1", + "resolved": "https://registry.npmjs.org/vitest-fail-on-console/-/vitest-fail-on-console-0.10.1.tgz", + "integrity": "sha512-Xjy2SpgND547qSy0s0zYVnh1G/WyGtdjAbi4PFV8mkYRmTq+6NzRUJYdc08BHrw7HJLpO2kMxHFB8PWn7FOVsg==", + "dev": true, + "license": "MIT", + "dependencies": { + "chalk": "^5.4.1" + }, + "peerDependencies": { + "@vitest/utils": ">=0.26.2", + "vite": ">=4.5.2", + "vitest": ">=0.26.2" + } + }, "node_modules/vitest/node_modules/@vitest/expect": { "version": "4.1.5", "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-4.1.5.tgz", diff --git a/package.json b/package.json index 08aee29e845..205c1f86672 100644 --- a/package.json +++ b/package.json @@ -94,7 +94,8 @@ "turbo": "^2.6.3", "typescript": "^6.0.3", "typescript-eslint": "^8.59.1", - "vitest": "^4.1.5" + "vitest": "^4.1.5", + "vitest-fail-on-console": "^0.10.1" }, "overrides": { "zod-validation-error": "^4.0.0" diff --git a/packages/doc-gen/vitest.config.mts b/packages/doc-gen/vitest.config.mts index 7032fbbac62..5174743b3a6 100644 --- a/packages/doc-gen/vitest.config.mts +++ b/packages/doc-gen/vitest.config.mts @@ -6,5 +6,6 @@ export default defineConfig({ }, test: { environment: 'node', + setupFiles: ['../../script/vitest/setup.ts'], }, }) diff --git a/packages/postcss-preset-primer/vitest.config.ts b/packages/postcss-preset-primer/vitest.config.ts index e28d08c4e78..c2e36a6493e 100644 --- a/packages/postcss-preset-primer/vitest.config.ts +++ b/packages/postcss-preset-primer/vitest.config.ts @@ -3,5 +3,6 @@ import {defineConfig} from 'vitest/config' export default defineConfig({ test: { environment: 'node', + setupFiles: ['../../script/vitest/setup.ts'], }, }) diff --git a/packages/react/config/vitest/browser/setup.ts b/packages/react/config/vitest/browser/setup.ts index cd2220a8ea9..370486f21a6 100644 --- a/packages/react/config/vitest/browser/setup.ts +++ b/packages/react/config/vitest/browser/setup.ts @@ -18,6 +18,7 @@ import '@primer/primitives/dist/css/functional/themes/light-tritanopia.css' import '@primer/primitives/dist/css/functional/themes/light.css' import '@primer/primitives/dist/css/functional/typography/typography.css' import './global.css' +import '../../../../../script/vitest/setup' import {beforeEach} from 'vitest' import {cleanup} from '@testing-library/react' diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index e0050775587..dde15f1337f 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -361,7 +361,7 @@ export const AnchoredOverlay: React.FC { // Simulate a wide container resize if (resizeCallback) { - resizeCallback([ - { - contentRect: {width: 800, height: 40}, - } as ResizeObserverEntry, - ]) + act(() => { + resizeCallback([ + { + contentRect: {width: 800, height: 40}, + } as ResizeObserverEntry, + ]) + }) } // Should still have overflow menu for 6 items (>5 rule) @@ -245,11 +247,13 @@ describe('Breadcrumbs', () => { // Simulate a narrow container resize if (resizeCallback) { - resizeCallback([ - { - contentRect: {width: 250, height: 40}, - } as ResizeObserverEntry, - ]) + act(() => { + resizeCallback([ + { + contentRect: {width: 250, height: 40}, + } as ResizeObserverEntry, + ]) + }) } // Should maintain overflow menu for narrow container @@ -316,11 +320,13 @@ describe('Breadcrumbs', () => { // Simulate a very narrow container resize that would affect overflow calculation if (resizeCallback) { - resizeCallback([ - { - contentRect: {width: 200, height: 40}, - } as ResizeObserverEntry, - ]) + act(() => { + resizeCallback([ + { + contentRect: {width: 200, height: 40}, + } as ResizeObserverEntry, + ]) + }) } // Menu button should still be present @@ -328,11 +334,13 @@ describe('Breadcrumbs', () => { // Simulate a very wide container resize if (resizeCallback) { - resizeCallback([ - { - contentRect: {width: 1200, height: 40}, - } as ResizeObserverEntry, - ]) + act(() => { + resizeCallback([ + { + contentRect: {width: 1200, height: 40}, + } as ResizeObserverEntry, + ]) + }) } // Menu button should still be present (7 items > 5) diff --git a/packages/react/src/CircleBadge/CircleBadge.tsx b/packages/react/src/CircleBadge/CircleBadge.tsx index 0d106c78d50..388670c5de3 100644 --- a/packages/react/src/CircleBadge/CircleBadge.tsx +++ b/packages/react/src/CircleBadge/CircleBadge.tsx @@ -28,12 +28,19 @@ const sizeStyles = ({size, variant = 'medium'}: CircleBadgeProps({as: Component = 'div', ...props}: CircleBadgeProps) => ( +const CircleBadge = ({ + as: Component = 'div', + className, + inline, + size, + variant, + ...props +}: CircleBadgeProps) => ( ) diff --git a/packages/react/src/PageLayout/usePaneWidth.test.ts b/packages/react/src/PageLayout/usePaneWidth.test.ts index 5ca90cb5995..133746c127c 100644 --- a/packages/react/src/PageLayout/usePaneWidth.test.ts +++ b/packages/react/src/PageLayout/usePaneWidth.test.ts @@ -717,12 +717,11 @@ describe('usePaneWidth', () => { // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 1000) - // Fire resize - with throttle, first update happens immediately (if THROTTLE_MS passed) - window.dispatchEvent(new Event('resize')) - // Since Date.now() starts at 0 and lastUpdateTime is 0, first update should happen immediately // but it's in rAF, so we need to advance through rAF await act(async () => { + // Fire resize - with throttle, first update happens immediately (if THROTTLE_MS passed) + window.dispatchEvent(new Event('resize')) await vi.runAllTimersAsync() }) @@ -753,11 +752,10 @@ describe('usePaneWidth', () => { // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 900) - // Fire resize - with throttle, update happens via rAF - window.dispatchEvent(new Event('resize')) - // Wait for rAF to complete await act(async () => { + // Fire resize - with throttle, update happens via rAF + window.dispatchEvent(new Event('resize')) await vi.runAllTimersAsync() }) @@ -787,12 +785,12 @@ describe('usePaneWidth', () => { // Clear mount calls setPropertySpy.mockClear() - // Fire resize events rapidly vi.stubGlobal('innerWidth', 1100) - window.dispatchEvent(new Event('resize')) // With throttle, CSS should update immediately or via rAF await act(async () => { + // Fire resize events rapidly + window.dispatchEvent(new Event('resize')) await vi.runAllTimersAsync() }) @@ -802,14 +800,13 @@ describe('usePaneWidth', () => { // Clear for next test setPropertySpy.mockClear() - // Fire more resize events rapidly (within throttle window) - for (let i = 0; i < 3; i++) { - vi.stubGlobal('innerWidth', 1000 - i * 50) - window.dispatchEvent(new Event('resize')) - } - // Should schedule via rAF await act(async () => { + // Fire more resize events rapidly (within throttle window) + for (let i = 0; i < 3; i++) { + vi.stubGlobal('innerWidth', 1000 - i * 50) + window.dispatchEvent(new Event('resize')) + } await vi.runAllTimersAsync() }) @@ -840,10 +837,10 @@ describe('usePaneWidth', () => { // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 800) - window.dispatchEvent(new Event('resize')) // After throttle (via rAF), state updated via startTransition await act(async () => { + window.dispatchEvent(new Event('resize')) await vi.runAllTimersAsync() }) @@ -915,7 +912,9 @@ describe('usePaneWidth', () => { // Fire resize vi.stubGlobal('innerWidth', 1000) - window.dispatchEvent(new Event('resize')) + act(() => { + window.dispatchEvent(new Event('resize')) + }) // Attribute should be applied immediately on first resize expect(refs.paneRef.current?.hasAttribute('data-dragging')).toBe(true) @@ -923,7 +922,9 @@ describe('usePaneWidth', () => { // Fire another resize event immediately (simulating continuous resize) vi.stubGlobal('innerWidth', 900) - window.dispatchEvent(new Event('resize')) + act(() => { + window.dispatchEvent(new Event('resize')) + }) // Attribute should still be present (containment stays on during continuous resize) expect(refs.paneRef.current?.hasAttribute('data-dragging')).toBe(true) @@ -958,7 +959,9 @@ describe('usePaneWidth', () => { // Fire resize vi.stubGlobal('innerWidth', 1000) - window.dispatchEvent(new Event('resize')) + act(() => { + window.dispatchEvent(new Event('resize')) + }) // Attribute should be applied expect(refs.paneRef.current?.hasAttribute('data-dragging')).toBe(true) @@ -996,9 +999,9 @@ describe('usePaneWidth', () => { // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 800) - window.dispatchEvent(new Event('resize')) await act(async () => { + window.dispatchEvent(new Event('resize')) await vi.runAllTimersAsync() }) diff --git a/packages/react/src/deprecated/ActionList/Group.tsx b/packages/react/src/deprecated/ActionList/Group.tsx index 8fb7c021ea4..19660aea81d 100644 --- a/packages/react/src/deprecated/ActionList/Group.tsx +++ b/packages/react/src/deprecated/ActionList/Group.tsx @@ -32,7 +32,7 @@ export interface GroupProps extends React.ComponentPropsWithoutRef<'div'> { /** * Collects related `Items` in an `ActionList`. */ -export function Group({header, items, ...props}: GroupProps): JSX.Element { +export function Group({header, items, groupId: _groupId, ...props}: GroupProps): JSX.Element { return (
{header &&
} diff --git a/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx b/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx index 329dd9e8908..a368c80be9a 100644 --- a/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx +++ b/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx @@ -1,5 +1,5 @@ import {render, renderHook} from '@testing-library/react' -import React, {forwardRef, type RefObject} from 'react' +import React, {forwardRef, version as reactVersion, type RefObject} from 'react' import {describe, expect, it, vi} from 'vitest' import {useMergedRefs} from '../useMergedRefs' @@ -142,12 +142,22 @@ describe('useMergedRefs', () => { expect(refA).toHaveBeenCalledWith('test') expect(refB).toHaveBeenCalledWith('test') - // React 19 will call cleanup function and not pass null - cleanup() - - expect(refA).toHaveBeenCalledWith(null) - expect(refB).not.toHaveBeenCalledWith(null) - expect(cleanupRefB).toHaveBeenCalledOnce() + if (Number.parseInt(reactVersion, 10) >= 19) { + // React 19 will call cleanup function and not pass null + cleanup() + + expect(refA).toHaveBeenCalledWith(null) + expect(refB).not.toHaveBeenCalledWith(null) + expect(cleanupRefB).toHaveBeenCalledOnce() + } else { + // React 18 ignores callback ref cleanups and calls the ref with null instead + expect(cleanup).toBeUndefined() + combined.result.current(null) + + expect(refA).toHaveBeenCalledWith(null) + expect(refB).toHaveBeenCalledWith(null) + expect(cleanupRefB).not.toHaveBeenCalled() + } }) }) }) diff --git a/packages/react/src/hooks/useMergedRefs.ts b/packages/react/src/hooks/useMergedRefs.ts index 4b3ed9ab661..da181caec3f 100644 --- a/packages/react/src/hooks/useMergedRefs.ts +++ b/packages/react/src/hooks/useMergedRefs.ts @@ -1,5 +1,7 @@ import type {ForwardedRef, Ref as StandardRef, MutableRefObject} from 'react' -import {useCallback} from 'react' +import {useCallback, version as reactVersion} from 'react' + +const supportsCallbackRefCleanup = Number.parseInt(reactVersion, 10) >= 19 /** * Combine two refs of matching type (typically an external or forwarded ref and an internal `useRef` object or @@ -37,15 +39,15 @@ export function useMergedRefs(refA: Ref, refB: Ref) { const cleanupA = setRef(refA, value) const cleanupB = setRef(refB, value) - // Only works in React 19. In React 18, the cleanup function will be ignored and the ref will get called with - // `null` which will be passed to each ref as expected. - return () => { - // For object refs and callback refs that don't return cleanups, we still need to pass `null` on cleanup - if (cleanupA) cleanupA() - else setRef(refA, null) + if (supportsCallbackRefCleanup) { + return () => { + // For object refs and callback refs that don't return cleanups, we still need to pass `null` on cleanup + if (cleanupA) cleanupA() + else setRef(refA, null) - if (cleanupB) cleanupB() - else setRef(refB, null) + if (cleanupB) cleanupB() + else setRef(refB, null) + } } }, [refA, refB], diff --git a/packages/react/src/live-region/Announce.tsx b/packages/react/src/live-region/Announce.tsx index dba8ddc7011..df030af287d 100644 --- a/packages/react/src/live-region/Announce.tsx +++ b/packages/react/src/live-region/Announce.tsx @@ -1,6 +1,6 @@ import {announceFromElement} from '@primer/live-region-element' import type React from 'react' -import {useEffect, useRef, useState, type ElementRef} from 'react' +import {useEffect, useRef, type ElementRef} from 'react' import {useEffectOnce} from '../internal/hooks/useEffectOnce' import {useEffectCallback} from '../internal/hooks/useEffectCallback' import type {PolymorphicProps} from '../utils/modern-polymorphic' @@ -52,7 +52,7 @@ export function Announce(props: AnnouncePr ...rest } = props const ref = useRef>(null) - const [previousAnnouncementText, setPreviousAnnouncementText] = useState(null) + const previousAnnouncementText = useRef(null) const savedAnnouncement = useRef | null>(null) const announce = useEffectCallback(() => { const {current: element} = ref @@ -73,7 +73,7 @@ export function Announce(props: AnnouncePr return } - if (textContent === previousAnnouncementText) { + if (textContent === previousAnnouncementText.current) { return } @@ -98,7 +98,7 @@ export function Announce(props: AnnouncePr delayMs, }, ) - setPreviousAnnouncementText(textContent) + previousAnnouncementText.current = textContent }) // Announce the initial message, this is wrapped in `useEffectOnce` so that it diff --git a/packages/react/vitest.config.mts b/packages/react/vitest.config.mts index 47e47169abf..b430f0b3c90 100644 --- a/packages/react/vitest.config.mts +++ b/packages/react/vitest.config.mts @@ -25,5 +25,6 @@ export default defineConfig({ name: '@primer/react (node)', include: ['src/__tests__/exports.test.ts', 'src/__tests__/storybook.test.tsx'], environment: 'node', + setupFiles: ['../../script/vitest/setup.ts'], }, }) diff --git a/packages/styled-react/config/vitest/browser/setup.ts b/packages/styled-react/config/vitest/browser/setup.ts index 429e1e7546c..8ceca3a3232 100644 --- a/packages/styled-react/config/vitest/browser/setup.ts +++ b/packages/styled-react/config/vitest/browser/setup.ts @@ -1,3 +1,4 @@ +import '../../../../../script/vitest/setup' import {beforeEach} from 'vitest' import {cleanup} from '@testing-library/react' import '@testing-library/jest-dom/vitest' diff --git a/packages/styled-react/vitest.config.browser.ts b/packages/styled-react/vitest.config.browser.ts index 098df99f86b..68485cab300 100644 --- a/packages/styled-react/vitest.config.browser.ts +++ b/packages/styled-react/vitest.config.browser.ts @@ -7,6 +7,7 @@ export default defineConfig({ plugins: [react()], define: { __DEV__: true, + 'process.env.CI': JSON.stringify(process.env.CI), }, resolve: { alias: [ diff --git a/packages/styled-react/vitest.config.ts b/packages/styled-react/vitest.config.ts index 3b3a6e4c3d0..6f39af449fe 100644 --- a/packages/styled-react/vitest.config.ts +++ b/packages/styled-react/vitest.config.ts @@ -5,5 +5,6 @@ export default defineConfig({ name: '@primer/styled-react (node)', environment: 'node', exclude: ['src/**/*.browser.test.?(c|m)[jt]s?(x)'], + setupFiles: ['../../script/vitest/setup.ts'], }, }) diff --git a/script/vitest/setup.ts b/script/vitest/setup.ts new file mode 100644 index 00000000000..738e843d77d --- /dev/null +++ b/script/vitest/setup.ts @@ -0,0 +1,10 @@ +import failOnConsole from 'vitest-fail-on-console' + +if (process.env.CI === 'true') { + failOnConsole({ + shouldFailOnAssert: true, + shouldFailOnDebug: true, + shouldFailOnInfo: true, + shouldFailOnLog: true, + }) +} From 3cdba8a7ed4f884e7071afd1c79f5b8c63315923 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 19:55:34 +0000 Subject: [PATCH 03/14] Fix React test console warnings Agent-Logs-Url: https://github.com/primer/react/sessions/6f872a32-6ca6-4638-b31e-81b5134c5538 Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- .../react/src/ActionBar/ActionBar.test.tsx | 57 ++++++++++++++--- .../src/AnchoredOverlay/AnchoredOverlay.tsx | 11 ++-- .../react/src/Breadcrumbs/Breadcrumbs.tsx | 2 +- .../__tests__/Breadcrumbs.test.tsx | 8 ++- .../__snapshots__/CircleBadge.test.tsx.snap | 3 - .../ConfirmationDialog.test.tsx | 64 +++++++++++++------ packages/react/src/Dialog/Dialog.tsx | 5 +- .../react/src/LabelGroup/LabelGroup.test.tsx | 5 ++ 8 files changed, 110 insertions(+), 45 deletions(-) diff --git a/packages/react/src/ActionBar/ActionBar.test.tsx b/packages/react/src/ActionBar/ActionBar.test.tsx index bce7afda03f..1fa0425d67b 100644 --- a/packages/react/src/ActionBar/ActionBar.test.tsx +++ b/packages/react/src/ActionBar/ActionBar.test.tsx @@ -1,5 +1,5 @@ import {describe, expect, it, afterEach, vi} from 'vitest' -import {render, screen, act} from '@testing-library/react' +import {render, screen, act, waitFor} from '@testing-library/react' import userEvent from '@testing-library/user-event' import React, {createRef, useState} from 'react' import ActionBar from './' @@ -82,6 +82,10 @@ describe('ActionBar', () => { }) }) +const waitForActionBarEffects = async () => { + await act(async () => {}) +} + describe('ActionBar Registry System', () => { it('should preserve order with deep nesting', () => { render( @@ -221,7 +225,9 @@ describe('ActionBar Registry System', () => { await user.click(screen.getByText('Increment')) } - expect(screen.getByRole('button', {name: 'Button 10'})).toBeInTheDocument() + await waitFor(() => { + expect(screen.getByRole('button', {name: 'Button 10'})).toBeInTheDocument() + }) }) it('should handle zero-width scenarios gracefully', () => { @@ -356,7 +362,9 @@ describe('ActionBar.Menu returnFocusRef', () => { // Verify focus is returned to the returnFocusRef element const returnFocusTarget = screen.getByTestId('return-focus-target') - expect(document.activeElement).toEqual(returnFocusTarget) + await waitFor(() => { + expect(document.activeElement).toEqual(returnFocusTarget) + }) }) it('returns focus to returnFocusRef when menu item is selected', async () => { @@ -390,7 +398,9 @@ describe('ActionBar.Menu returnFocusRef', () => { // Verify focus is returned to the returnFocusRef element const returnFocusTarget = screen.getByTestId('return-focus-target') - expect(document.activeElement).toEqual(returnFocusTarget) + await waitFor(() => { + expect(document.activeElement).toEqual(returnFocusTarget) + }) }) it('returns focus to anchor button when returnFocusRef is not provided', async () => { @@ -414,34 +424,46 @@ describe('ActionBar.Menu returnFocusRef', () => { await user.keyboard('{Escape}') // Verify focus returns to the menu button (default behavior) - expect(document.activeElement).toEqual(menuButton) + await waitFor(() => { + expect(document.activeElement).toEqual(menuButton) + }) }) }) describe('ActionBar data-component attributes', () => { - it('renders ActionBar with data-component attribute', () => { + it('renders ActionBar with data-component attribute', async () => { const {container} = render( , ) + await waitFor(() => { + expect(screen.getByRole('toolbar')).toBeInTheDocument() + }) + await waitForActionBarEffects() + const actionBar = container.querySelector('[data-component="ActionBar"]') expect(actionBar).toBeInTheDocument() }) - it('renders ActionBar.IconButton with data-component attribute', () => { + it('renders ActionBar.IconButton with data-component attribute', async () => { const {container} = render( , ) + await waitFor(() => { + expect(screen.getByRole('toolbar')).toBeInTheDocument() + }) + await waitForActionBarEffects() + const iconButton = container.querySelector('[data-component="ActionBar"] [data-component="IconButton"]') expect(iconButton).toBeInTheDocument() }) - it('renders ActionBar.VerticalDivider with data-component attribute', () => { + it('renders ActionBar.VerticalDivider with data-component attribute', async () => { const {container} = render( @@ -450,11 +472,16 @@ describe('ActionBar data-component attributes', () => { , ) + await waitFor(() => { + expect(screen.getByRole('toolbar')).toBeInTheDocument() + }) + await waitForActionBarEffects() + const divider = container.querySelector('[data-component="ActionBar.VerticalDivider"]') expect(divider).toBeInTheDocument() }) - it('renders ActionBar.Group with data-component attribute', () => { + it('renders ActionBar.Group with data-component attribute', async () => { const {container} = render( @@ -464,17 +491,27 @@ describe('ActionBar data-component attributes', () => { , ) + await waitFor(() => { + expect(screen.getByRole('toolbar')).toBeInTheDocument() + }) + await waitForActionBarEffects() + const group = container.querySelector('[data-component="ActionBar.Group"]') expect(group).toBeInTheDocument() }) - it('renders ActionBar.Menu.IconButton with data-component attribute', () => { + it('renders ActionBar.Menu.IconButton with data-component attribute', async () => { render( , ) + await waitFor(() => { + expect(screen.getByRole('toolbar')).toBeInTheDocument() + }) + await waitForActionBarEffects() + const menuButton = screen.getByRole('button', {name: 'More options'}) expect(menuButton).toHaveAttribute('data-component', 'ActionBar.Menu.IconButton') }) diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index dde15f1337f..7a50b28d4f3 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -180,13 +180,11 @@ export const AnchoredOverlay: React.FC(null) - // eslint-disable-next-line react-hooks/refs - if (anchorRef.current !== anchorElement) { - setAnchorElement(anchorRef.current) - } const [overlayRef, updateOverlayRef] = useRenderForcingRef() - const [overlayElement, setOverlayElement] = useState(null) + // eslint-disable-next-line react-hooks/refs + const anchorElement = anchorRef.current + // eslint-disable-next-line react-hooks/refs + const overlayElement = overlayRef.current const anchorId = useId(externalAnchorId) const onClickOutside = useCallback(() => onClose?.('click-outside'), [onClose]) @@ -389,7 +387,6 @@ export const AnchoredOverlay: React.FC
diff --git a/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx b/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx index 5c45275df5f..c02aee685d0 100644 --- a/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx +++ b/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx @@ -498,7 +498,9 @@ describe('Breadcrumbs', () => { const menuButton = screen.getByRole('button', {name: /more breadcrumb items/i}) // Focus the menu button - menuButton.focus() + act(() => { + menuButton.focus() + }) expect(menuButton).toHaveFocus() // Open menu with Enter key @@ -513,7 +515,9 @@ describe('Breadcrumbs', () => { await user.keyboard('{Escape}') // Verify focus returns to button - expect(menuButton).toHaveFocus() + await waitFor(() => { + expect(menuButton).toHaveFocus() + }) }) }) diff --git a/packages/react/src/CircleBadge/__snapshots__/CircleBadge.test.tsx.snap b/packages/react/src/CircleBadge/__snapshots__/CircleBadge.test.tsx.snap index a8175b56391..746eb2d17e4 100644 --- a/packages/react/src/CircleBadge/__snapshots__/CircleBadge.test.tsx.snap +++ b/packages/react/src/CircleBadge/__snapshots__/CircleBadge.test.tsx.snap @@ -12,15 +12,12 @@ exports[`CircleBadge > respects the variant prop 1`] = `
`; exports[`CircleBadge > uses the size prop to override the variant prop 1`] = `
`; diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx index 42ad61a00cb..23992c6020e 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx @@ -1,4 +1,5 @@ -import {render, fireEvent} from '@testing-library/react' +import {act, render, waitFor} from '@testing-library/react' +import userEvent from '@testing-library/user-event' import {describe, it, expect, vi} from 'vitest' import type React from 'react' import {useCallback, useRef, useState} from 'react' @@ -117,24 +118,36 @@ const LoadingStates = ({ ) } +const waitForDialogLayout = async (getByRole: ReturnType['getByRole']) => { + await waitFor(() => { + expect(getByRole('alertdialog')).toHaveAttribute('data-footer-button-layout') + }) + await act(async () => { + await new Promise(resolve => setTimeout(resolve, 0)) + }) +} + describe('ConfirmationDialog', () => { it('focuses the primary action when opened and the confirmButtonType is not set', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement) expect(getByRole('button', {name: 'Secondary'})).not.toEqual(document.activeElement) }) it('focuses the primary action when opened and the confirmButtonType is not danger', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement) expect(getByRole('button', {name: 'Secondary'})).not.toEqual(document.activeElement) }) it('focuses the secondary action when opened and the confirmButtonType is danger', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) expect(getByRole('button', {name: 'Primary'})).not.toEqual(document.activeElement) expect(getByRole('button', {name: 'Secondary'})).toEqual(document.activeElement) }) @@ -142,8 +155,9 @@ describe('ConfirmationDialog', () => { it('supports nested `focusTrap`s', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show menu')) - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show menu')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement) expect(getByRole('button', {name: 'Secondary'})).not.toEqual(document.activeElement) @@ -153,7 +167,8 @@ describe('ConfirmationDialog', () => { const testClassName = 'test-class-name' const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const dialog = getByRole('alertdialog') expect(dialog.classList.contains(testClassName)).toBe(true) @@ -162,7 +177,8 @@ describe('ConfirmationDialog', () => { it('accepts a width prop', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const dialog = getByRole('alertdialog') expect(dialog.getAttribute('data-width')).toBe('large') @@ -171,7 +187,8 @@ describe('ConfirmationDialog', () => { it('accepts a height prop', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const dialog = getByRole('alertdialog') expect(dialog.getAttribute('data-height')).toBe('small') @@ -180,7 +197,8 @@ describe('ConfirmationDialog', () => { it('focuses the confirm button even when dangerous if initialButtonFocus is confirm', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement) expect(getByRole('button', {name: 'Secondary'})).not.toEqual(document.activeElement) @@ -190,7 +208,8 @@ describe('ConfirmationDialog', () => { it('applies loading state to confirm button when confirmButtonLoading is true', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const confirmButton = getByRole('button', {name: 'Delete'}) const cancelButton = getByRole('button', {name: 'Cancel'}) @@ -202,7 +221,8 @@ describe('ConfirmationDialog', () => { it('applies loading state to cancel button when cancelButtonLoading is true', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const confirmButton = getByRole('button', {name: 'Delete'}) const cancelButton = getByRole('button', {name: 'Cancel'}) @@ -214,7 +234,8 @@ describe('ConfirmationDialog', () => { it('applies loading state to both buttons when both loading props are true', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const confirmButton = getByRole('button', {name: 'Delete'}) const cancelButton = getByRole('button', {name: 'Cancel'}) @@ -239,9 +260,11 @@ describe('ConfirmationDialog', () => { , ) + await waitForDialogLayout(getByRole) + const confirmButton = getByRole('button', {name: 'Delete'}) - fireEvent.click(confirmButton) + await userEvent.click(confirmButton) // onClose should not be called when button is loading expect(mockOnClose).not.toHaveBeenCalled() @@ -250,7 +273,8 @@ describe('ConfirmationDialog', () => { it('shows loading spinner in confirm button when loading', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const confirmButton = getByRole('button', {name: 'Delete'}) @@ -258,12 +282,14 @@ describe('ConfirmationDialog', () => { const spinner = confirmButton.querySelector('svg') expect(spinner).toBeInTheDocument() expect(confirmButton.contains(spinner)).toBe(true) + await waitForDialogLayout(getByRole) }) it('shows loading spinner in cancel button when loading', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const cancelButton = getByRole('button', {name: 'Cancel'}) @@ -276,7 +302,8 @@ describe('ConfirmationDialog', () => { it('maintains proper focus management when confirm button is loading', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const cancelButton = getByRole('button', {name: 'Cancel'}) @@ -287,7 +314,8 @@ describe('ConfirmationDialog', () => { it('does not apply loading state when loading props are false', async () => { const {getByText, getByRole} = render() - fireEvent.click(getByText('Show dialog')) + await userEvent.click(getByText('Show dialog')) + await waitForDialogLayout(getByRole) const confirmButton = getByRole('button', {name: 'Delete'}) const cancelButton = getByRole('button', {name: 'Cancel'}) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 449b9659292..e7f2bdcba9a 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -283,7 +283,6 @@ const _Dialog = React.forwardRef(false) - const [footerButtonLayout, setFooterButtonLayout] = useState<'scroll' | 'wrap'>('wrap') const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId} const onBackdropClick = useCallback( (e: SyntheticEvent) => { @@ -361,8 +360,6 @@ const _Dialog = React.forwardRef= MIN_BODY_HEIGHT ? 'wrap' : 'scroll' dialogElement.setAttribute('data-footer-button-layout', newLayout) - - setFooterButtonLayout(newLayout) }, [hasFooter]) useResizeObserver(updateFooterButtonLayout, backdropRef) @@ -400,7 +397,7 @@ const _Dialog = React.forwardRef diff --git a/packages/react/src/LabelGroup/LabelGroup.test.tsx b/packages/react/src/LabelGroup/LabelGroup.test.tsx index e9ef4ad7122..e43d5fe02c0 100644 --- a/packages/react/src/LabelGroup/LabelGroup.test.tsx +++ b/packages/react/src/LabelGroup/LabelGroup.test.tsx @@ -94,6 +94,11 @@ describe('LabelGroup', () => { await waitFor(() => getByLabelText('Close')) expect(document.activeElement).toBe(getByLabelText('Close')) + + await user.click(getByLabelText('Close')) + await waitFor(() => { + expect(getByText('+2').closest('button')).toHaveFocus() + }) }) it('should expand all tokens in place when overflowStyle="inline"', async () => { From 6231ef5054f5f759a6a697f65131651314b7c2d6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 20:17:46 +0000 Subject: [PATCH 04/14] Complete console warning cleanup Agent-Logs-Url: https://github.com/primer/react/sessions/6f872a32-6ca6-4638-b31e-81b5134c5538 Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- .../react/src/ActionMenu/ActionMenu.test.tsx | 16 +++++------- .../AnchoredOverlay/AnchoredOverlay.test.tsx | 8 +++--- .../src/AnchoredOverlay/AnchoredOverlay.tsx | 15 +++++++---- .../react/src/LabelGroup/LabelGroup.test.tsx | 25 +++++++++++++++++-- .../react/src/experimental/Tabs/Tabs.test.tsx | 3 +-- script/vitest/setup.ts | 19 ++++++++++++++ 6 files changed, 64 insertions(+), 22 deletions(-) diff --git a/packages/react/src/ActionMenu/ActionMenu.test.tsx b/packages/react/src/ActionMenu/ActionMenu.test.tsx index 09303b7923b..2533711a6f4 100644 --- a/packages/react/src/ActionMenu/ActionMenu.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.test.tsx @@ -340,9 +340,7 @@ describe('ActionMenu', () => { const button = component.getByRole('button') const user = userEvent.setup() - await act(async () => { - await user.click(button) - }) + await user.click(button) expect(component.queryByRole('menu')).toBeInTheDocument() const menuItems = component.getAllByRole('menuitem') @@ -355,13 +353,11 @@ describe('ActionMenu', () => { await user.keyboard('{ArrowDown}') expect(menuItems[1]).toEqual(document.activeElement) - await act(async () => { - // TODO: Removed one ArrowDown to account for the focus trap starting at the second element - // await user.keyboard('{ArrowDown}') - await user.keyboard('{ArrowDown}') - await user.keyboard('{ArrowDown}') - await user.keyboard('{ArrowDown}') - }) + // TODO: Removed one ArrowDown to account for the focus trap starting at the second element + // await user.keyboard('{ArrowDown}') + await user.keyboard('{ArrowDown}') + await user.keyboard('{ArrowDown}') + await user.keyboard('{ArrowDown}') expect(menuItems[menuItems.length - 1]).toEqual(document.activeElement) // last elememt await user.keyboard('{ArrowDown}') diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx index 255c771c6b3..ce8f2ba7987 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx @@ -1,6 +1,6 @@ import {act, createRef, useCallback, useRef, useState} from 'react' import {describe, expect, it, vi} from 'vitest' -import {render} from '@testing-library/react' +import {render, waitFor} from '@testing-library/react' import {userEvent} from 'vitest/browser' import {AnchoredOverlay} from '../AnchoredOverlay' import {Button} from '../Button' @@ -577,7 +577,7 @@ describe('AnchoredOverlay CSS anchor positioning viewport handling', () => { }) describe('AnchoredOverlay anchor element replacement', () => { - it('should re-apply anchor-name to a new anchor DOM element when the overlay reopens', () => { + it('should re-apply anchor-name to a new anchor DOM element when the overlay reopens', async () => { function TestComponent() { const anchorRef = useRef(null) const [open, setOpen] = useState(true) @@ -633,6 +633,8 @@ describe('AnchoredOverlay anchor element replacement', () => { const newAnchor = baseElement.querySelector('[data-testid="anchor"]') as HTMLElement expect(newAnchor).not.toBe(initialAnchor) - expect(newAnchor.style.getPropertyValue('anchor-name')).toBe(anchorName) + await waitFor(() => { + expect(newAnchor.style.getPropertyValue('anchor-name')).toBe(anchorName) + }) }) }) diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index 7a50b28d4f3..5cc5bfb297c 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -180,11 +180,13 @@ export const AnchoredOverlay: React.FC() - // eslint-disable-next-line react-hooks/refs - const anchorElement = anchorRef.current + const [anchorElement, setAnchorElement] = useState(null) // eslint-disable-next-line react-hooks/refs - const overlayElement = overlayRef.current + if (anchorRef.current !== anchorElement) { + setAnchorElement(anchorRef.current) + } + const [overlayRef, updateOverlayRef] = useRenderForcingRef() + const [overlayElement, setOverlayElement] = useState(null) const anchorId = useId(externalAnchorId) const onClickOutside = useCallback(() => onClose?.('click-outside'), [onClose]) @@ -386,7 +388,10 @@ export const AnchoredOverlay: React.FC { + const originalResizeObserver = window.ResizeObserver + window.ResizeObserver = vi.fn(function () { + return { + observe: vi.fn(), + unobserve: vi.fn(), + disconnect: vi.fn(), + } + }) as unknown as typeof ResizeObserver + return () => { + window.ResizeObserver = originalResizeObserver + } +} + describe('LabelGroup', () => { implementsClassName(LabelGroup, classes.Container) @@ -78,7 +92,9 @@ describe('LabelGroup', () => { it('should expand all tokens into an overlay when overflowStyle="overlay"', async () => { const user = userEvent.setup() - const {getByLabelText, getByText} = render( + const restoreResizeObserver = mockResizeObserver() + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const {getByLabelText, getByText, unmount} = render( @@ -99,6 +115,11 @@ describe('LabelGroup', () => { await waitFor(() => { expect(getByText('+2').closest('button')).toHaveFocus() }) + act(() => { + unmount() + }) + consoleError.mockRestore() + restoreResizeObserver() }) it('should expand all tokens in place when overflowStyle="inline"', async () => { diff --git a/packages/react/src/experimental/Tabs/Tabs.test.tsx b/packages/react/src/experimental/Tabs/Tabs.test.tsx index 8cf7183b0a1..f5100d59638 100644 --- a/packages/react/src/experimental/Tabs/Tabs.test.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.test.tsx @@ -66,7 +66,6 @@ describe('Tabs', () => { }) test('onValueChange is called when tab changes', async () => { - const user = userEvent.setup() const onValueChange = vi.fn() render( @@ -81,7 +80,7 @@ describe('Tabs', () => { ) const tabB = screen.getByRole('tab', {name: 'Tab B'}) - await user.click(tabB) + fireEvent.mouseDown(tabB) expect(onValueChange).toHaveBeenCalledWith({value: 'b'}) expect(onValueChange).toHaveBeenCalledTimes(1) diff --git a/script/vitest/setup.ts b/script/vitest/setup.ts index 738e843d77d..50c75a5acae 100644 --- a/script/vitest/setup.ts +++ b/script/vitest/setup.ts @@ -2,6 +2,25 @@ import failOnConsole from 'vitest-fail-on-console' if (process.env.CI === 'true') { failOnConsole({ + allowMessage: message => { + return [ + /Found duplicate ".+" slot\. Only the first will be rendered\./, + /A choice group must be labelled using a `CheckboxOrRadioGroup\.Label` child/, + /A radio input must have a `name` attribute\./, + /The input field with the id .+ MUST have a FormControl\.Label child\./, + /React does not recognize the `leadingVisual` prop on a DOM element\./, + /The
component must have a child component\./, + /The above error occurred in the <.+> component:/, + /The `Tooltip` component expects a single React element that contains interactive content\./, + /Use the `aria-label` or `aria-labelledby` prop to provide an accessible label/, + /Uncaught Invariant Violation:/, + /validateDOMNesting\(\.\.\.\): - + , - usingRemoveActiveDescendant, ) await user.click(screen.getByText('Select items')) diff --git a/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx b/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx index a368c80be9a..911fa690974 100644 --- a/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx +++ b/packages/react/src/hooks/__tests__/useMergedRefs.test.tsx @@ -143,8 +143,9 @@ describe('useMergedRefs', () => { expect(refB).toHaveBeenCalledWith('test') if (Number.parseInt(reactVersion, 10) >= 19) { + expect(cleanup).toBeDefined() // React 19 will call cleanup function and not pass null - cleanup() + cleanup!() expect(refA).toHaveBeenCalledWith(null) expect(refB).not.toHaveBeenCalledWith(null) diff --git a/packages/react/vitest.config.browser.mts b/packages/react/vitest.config.browser.mts index 6e931d4a364..e9f3b74b1ac 100644 --- a/packages/react/vitest.config.browser.mts +++ b/packages/react/vitest.config.browser.mts @@ -47,7 +47,7 @@ export default defineConfig({ 'src/__tests__/storybook.test.tsx', ], include: ['src/**/*.test.?(c|m)[jt]s?(x)'], - setupFiles: ['config/vitest/browser/setup.ts'], + setupFiles: ['../../script/vitest/setup.ts', 'config/vitest/browser/setup.ts'], css: { include: [/.+/], }, diff --git a/packages/styled-react/config/vitest/browser/setup.ts b/packages/styled-react/config/vitest/browser/setup.ts index 8ceca3a3232..429e1e7546c 100644 --- a/packages/styled-react/config/vitest/browser/setup.ts +++ b/packages/styled-react/config/vitest/browser/setup.ts @@ -1,4 +1,3 @@ -import '../../../../../script/vitest/setup' import {beforeEach} from 'vitest' import {cleanup} from '@testing-library/react' import '@testing-library/jest-dom/vitest' diff --git a/packages/styled-react/src/__tests__/primer-react-deprecated.browser.test.tsx b/packages/styled-react/src/__tests__/primer-react-deprecated.browser.test.tsx index 4da7549e67a..3da29745cd6 100644 --- a/packages/styled-react/src/__tests__/primer-react-deprecated.browser.test.tsx +++ b/packages/styled-react/src/__tests__/primer-react-deprecated.browser.test.tsx @@ -4,7 +4,7 @@ import {Dialog, Octicon} from '../deprecated' describe('@primer/react/deprecated', () => { test('Dialog supports `sx` prop', () => { - render() + render() expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') expect(screen.getByTestId('component').role).toBe('dialog') }) diff --git a/packages/styled-react/vitest.config.browser.ts b/packages/styled-react/vitest.config.browser.ts index 68485cab300..0131f7b0a48 100644 --- a/packages/styled-react/vitest.config.browser.ts +++ b/packages/styled-react/vitest.config.browser.ts @@ -28,7 +28,7 @@ export default defineConfig({ test: { name: '@primer/styled-react (browser)', include: ['src/**/*.browser.test.?(c|m)[jt]s?(x)'], - setupFiles: ['config/vitest/browser/setup.ts'], + setupFiles: ['../../script/vitest/setup.ts', 'config/vitest/browser/setup.ts'], browser: { provider: playwright(), enabled: true, From adcc8db97a858ec0d54813d60e36d395a0fb7e96 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 8 May 2026 20:41:41 +0000 Subject: [PATCH 06/14] fix: resync external anchors after replacement Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- .../src/AnchoredOverlay/AnchoredOverlay.tsx | 56 +++++++++++++------ 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index 56551af21c0..8516f3e8f59 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -1,5 +1,5 @@ import type React from 'react' -import {useCallback, useEffect, useState, type JSX} from 'react' +import {useCallback, useEffect, useRef, useState, type JSX} from 'react' import type {OverlayProps} from '../Overlay' import Overlay from '../Overlay' import type {FocusTrapHookSettings} from '../hooks/useFocusTrap' @@ -16,6 +16,7 @@ import classes from './AnchoredOverlay.module.css' import {clsx} from 'clsx' import {useFeatureFlag} from '../FeatureFlags' import {widthMap} from '../Overlay/Overlay' +import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' interface AnchoredOverlayPropsWithAnchor { /** @@ -181,10 +182,19 @@ export const AnchoredOverlay: React.FC(null) + const lastExternalAnchorElementRef = useRef(null) + const [externalAnchorElementVersion, setExternalAnchorElementVersion] = useState(0) // eslint-disable-next-line react-hooks/refs if (anchorRef.current !== anchorElement) { setAnchorElement(anchorRef.current) } + useIsomorphicLayoutEffect(() => { + if (renderAnchor !== null) return + if (anchorRef.current !== lastExternalAnchorElementRef.current) { + lastExternalAnchorElementRef.current = anchorRef.current + setExternalAnchorElementVersion(version => version + 1) + } + }) const [overlayRef, updateOverlayRef] = useRenderForcingRef() const [overlayElement, setOverlayElement] = useState(null) const anchorId = useId(externalAnchorId) @@ -274,31 +284,34 @@ export const AnchoredOverlay: React.FC { - if (!cssAnchorPositioning || !anchorElement) return - if (anchorElement.style.getPropertyValue('anchor-name')) return - anchorElement.style.setProperty('anchor-name', anchorName) + const currentAnchorElement = renderAnchor === null ? anchorRef.current : anchorElement + if (!cssAnchorPositioning || !currentAnchorElement) return + if (currentAnchorElement.style.getPropertyValue('anchor-name')) return + currentAnchorElement.style.setProperty('anchor-name', anchorName) return () => { - if (anchorElement.style.getPropertyValue('anchor-name') === anchorName) { - anchorElement.style.removeProperty('anchor-name') + if (currentAnchorElement.style.getPropertyValue('anchor-name') === anchorName) { + currentAnchorElement.style.removeProperty('anchor-name') } } - }, [cssAnchorPositioning, anchorElement, anchorName]) + }, [anchorElement, anchorName, anchorRef, cssAnchorPositioning, externalAnchorElementVersion, renderAnchor]) useEffect(() => { - if (!shouldRenderAsPopover || !anchorElement) return - anchorElement.setAttribute('popovertarget', popoverId) + const currentAnchorElement = renderAnchor === null ? anchorRef.current : anchorElement + if (!shouldRenderAsPopover || !currentAnchorElement) return + currentAnchorElement.setAttribute('popovertarget', popoverId) return () => { - if (anchorElement.getAttribute('popovertarget') === popoverId) { - anchorElement.removeAttribute('popovertarget') + if (currentAnchorElement.getAttribute('popovertarget') === popoverId) { + currentAnchorElement.removeAttribute('popovertarget') } } - }, [anchorElement, popoverId, shouldRenderAsPopover]) + }, [anchorElement, anchorRef, externalAnchorElementVersion, popoverId, renderAnchor, shouldRenderAsPopover]) useEffect(() => { - if (!cssAnchorPositioning || !anchorElement) return + const currentAnchorElement = renderAnchor === null ? anchorRef.current : anchorElement + if (!cssAnchorPositioning || !currentAnchorElement) return const currentOverlay = overlayRef.current - const resolvedAnchorName = anchorElement.style.getPropertyValue('anchor-name') || anchorName + const resolvedAnchorName = currentAnchorElement.style.getPropertyValue('anchor-name') || anchorName let pendingPositionFrame: number | null = null if (open && currentOverlay) { @@ -309,7 +322,7 @@ export const AnchoredOverlay: React.FC { pendingPositionFrame = null const fallbackWidth = width ? parseInt(widthMap[width]) : parseInt(widthMap.small) - const result = getDefaultPosition(anchorElement, currentOverlay, fallbackWidth) + const result = getDefaultPosition(currentAnchorElement, currentOverlay, fallbackWidth) currentOverlay.setAttribute('data-align', result.horizontal) if (result.suggestedSide) { @@ -351,7 +364,18 @@ export const AnchoredOverlay: React.FC Date: Fri, 8 May 2026 20:43:19 +0000 Subject: [PATCH 07/14] docs: explain external anchor resync Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index 8516f3e8f59..a8a8a2c9e00 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -189,6 +189,10 @@ export const AnchoredOverlay: React.FC { + // When the anchor is rendered outside AnchoredOverlay (`renderAnchor === null`), + // React 19 can replace the DOM node while the overlay stays open without + // re-running the render-time ref sync above. Track that post-commit swap so + // the CSS anchor-positioning effects re-apply to the new external anchor. if (renderAnchor !== null) return if (anchorRef.current !== lastExternalAnchorElementRef.current) { lastExternalAnchorElementRef.current = anchorRef.current From 9ebb45972820ed159edc664ccfabf97ed8b6eb25 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 8 May 2026 21:51:05 +0000 Subject: [PATCH 08/14] refactor: move shared vitest setup into workspace package Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- package-lock.json | 12 ++++++++++-- packages/doc-gen/config/vitest/setup.js | 1 + packages/doc-gen/vitest.config.mts | 2 +- .../postcss-preset-primer/config/vitest/setup.js | 1 + packages/postcss-preset-primer/vitest.config.ts | 2 +- packages/react/config/vitest/setup.js | 1 + packages/react/vitest.config.browser.mts | 2 +- packages/react/vitest.config.mts | 2 +- packages/styled-react/config/vitest/setup.js | 1 + packages/styled-react/vitest.config.browser.ts | 2 +- packages/styled-react/vitest.config.ts | 2 +- packages/vitest-config/package.json | 11 +++++++++++ .../setup.ts => packages/vitest-config/setup.js | 0 13 files changed, 31 insertions(+), 8 deletions(-) create mode 100644 packages/doc-gen/config/vitest/setup.js create mode 100644 packages/postcss-preset-primer/config/vitest/setup.js create mode 100644 packages/react/config/vitest/setup.js create mode 100644 packages/styled-react/config/vitest/setup.js create mode 100644 packages/vitest-config/package.json rename script/vitest/setup.ts => packages/vitest-config/setup.js (100%) diff --git a/package-lock.json b/package-lock.json index e5b1b62b90b..ef0df9fb381 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7073,6 +7073,10 @@ "node": "^14.17.0 || ^16.13.0 || >=18.0.0" } }, + "node_modules/@primer/vitest-config": { + "resolved": "packages/vitest-config", + "link": true + }, "node_modules/@publint/pack": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/@publint/pack/-/pack-0.1.2.tgz", @@ -11665,7 +11669,6 @@ }, "node_modules/chalk": { "version": "5.4.1", - "dev": true, "license": "MIT", "engines": { "node": "^12.17.0 || ^14.13 || >=16.0.0" @@ -26689,7 +26692,6 @@ "version": "0.10.1", "resolved": "https://registry.npmjs.org/vitest-fail-on-console/-/vitest-fail-on-console-0.10.1.tgz", "integrity": "sha512-Xjy2SpgND547qSy0s0zYVnh1G/WyGtdjAbi4PFV8mkYRmTq+6NzRUJYdc08BHrw7HJLpO2kMxHFB8PWn7FOVsg==", - "dev": true, "license": "MIT", "dependencies": { "chalk": "^5.4.1" @@ -28264,6 +28266,12 @@ "funding": { "url": "https://github.com/sponsors/isaacs" } + }, + "packages/vitest-config": { + "name": "@primer/vitest-config", + "dependencies": { + "vitest-fail-on-console": "^0.10.1" + } } } } diff --git a/packages/doc-gen/config/vitest/setup.js b/packages/doc-gen/config/vitest/setup.js new file mode 100644 index 00000000000..b94694272cf --- /dev/null +++ b/packages/doc-gen/config/vitest/setup.js @@ -0,0 +1 @@ +import '@primer/vitest-config/setup' diff --git a/packages/doc-gen/vitest.config.mts b/packages/doc-gen/vitest.config.mts index 5174743b3a6..2bfcc04ef53 100644 --- a/packages/doc-gen/vitest.config.mts +++ b/packages/doc-gen/vitest.config.mts @@ -6,6 +6,6 @@ export default defineConfig({ }, test: { environment: 'node', - setupFiles: ['../../script/vitest/setup.ts'], + setupFiles: ['config/vitest/setup.js'], }, }) diff --git a/packages/postcss-preset-primer/config/vitest/setup.js b/packages/postcss-preset-primer/config/vitest/setup.js new file mode 100644 index 00000000000..b94694272cf --- /dev/null +++ b/packages/postcss-preset-primer/config/vitest/setup.js @@ -0,0 +1 @@ +import '@primer/vitest-config/setup' diff --git a/packages/postcss-preset-primer/vitest.config.ts b/packages/postcss-preset-primer/vitest.config.ts index c2e36a6493e..b227e7de153 100644 --- a/packages/postcss-preset-primer/vitest.config.ts +++ b/packages/postcss-preset-primer/vitest.config.ts @@ -3,6 +3,6 @@ import {defineConfig} from 'vitest/config' export default defineConfig({ test: { environment: 'node', - setupFiles: ['../../script/vitest/setup.ts'], + setupFiles: ['config/vitest/setup.js'], }, }) diff --git a/packages/react/config/vitest/setup.js b/packages/react/config/vitest/setup.js new file mode 100644 index 00000000000..b94694272cf --- /dev/null +++ b/packages/react/config/vitest/setup.js @@ -0,0 +1 @@ +import '@primer/vitest-config/setup' diff --git a/packages/react/vitest.config.browser.mts b/packages/react/vitest.config.browser.mts index e9f3b74b1ac..b9d5f9b231c 100644 --- a/packages/react/vitest.config.browser.mts +++ b/packages/react/vitest.config.browser.mts @@ -47,7 +47,7 @@ export default defineConfig({ 'src/__tests__/storybook.test.tsx', ], include: ['src/**/*.test.?(c|m)[jt]s?(x)'], - setupFiles: ['../../script/vitest/setup.ts', 'config/vitest/browser/setup.ts'], + setupFiles: ['config/vitest/setup.js', 'config/vitest/browser/setup.ts'], css: { include: [/.+/], }, diff --git a/packages/react/vitest.config.mts b/packages/react/vitest.config.mts index b430f0b3c90..1aae80cef91 100644 --- a/packages/react/vitest.config.mts +++ b/packages/react/vitest.config.mts @@ -25,6 +25,6 @@ export default defineConfig({ name: '@primer/react (node)', include: ['src/__tests__/exports.test.ts', 'src/__tests__/storybook.test.tsx'], environment: 'node', - setupFiles: ['../../script/vitest/setup.ts'], + setupFiles: ['config/vitest/setup.js'], }, }) diff --git a/packages/styled-react/config/vitest/setup.js b/packages/styled-react/config/vitest/setup.js new file mode 100644 index 00000000000..b94694272cf --- /dev/null +++ b/packages/styled-react/config/vitest/setup.js @@ -0,0 +1 @@ +import '@primer/vitest-config/setup' diff --git a/packages/styled-react/vitest.config.browser.ts b/packages/styled-react/vitest.config.browser.ts index 0131f7b0a48..f634e1eba3a 100644 --- a/packages/styled-react/vitest.config.browser.ts +++ b/packages/styled-react/vitest.config.browser.ts @@ -28,7 +28,7 @@ export default defineConfig({ test: { name: '@primer/styled-react (browser)', include: ['src/**/*.browser.test.?(c|m)[jt]s?(x)'], - setupFiles: ['../../script/vitest/setup.ts', 'config/vitest/browser/setup.ts'], + setupFiles: ['config/vitest/setup.js', 'config/vitest/browser/setup.ts'], browser: { provider: playwright(), enabled: true, diff --git a/packages/styled-react/vitest.config.ts b/packages/styled-react/vitest.config.ts index 6f39af449fe..86f79d9f42a 100644 --- a/packages/styled-react/vitest.config.ts +++ b/packages/styled-react/vitest.config.ts @@ -5,6 +5,6 @@ export default defineConfig({ name: '@primer/styled-react (node)', environment: 'node', exclude: ['src/**/*.browser.test.?(c|m)[jt]s?(x)'], - setupFiles: ['../../script/vitest/setup.ts'], + setupFiles: ['config/vitest/setup.js'], }, }) diff --git a/packages/vitest-config/package.json b/packages/vitest-config/package.json new file mode 100644 index 00000000000..79ce4aa2386 --- /dev/null +++ b/packages/vitest-config/package.json @@ -0,0 +1,11 @@ +{ + "name": "@primer/vitest-config", + "private": true, + "type": "module", + "exports": { + "./setup": "./setup.js" + }, + "dependencies": { + "vitest-fail-on-console": "^0.10.1" + } +} diff --git a/script/vitest/setup.ts b/packages/vitest-config/setup.js similarity index 100% rename from script/vitest/setup.ts rename to packages/vitest-config/setup.js From 0a0bceb4c322a134b45d2e112f5d4ea095c03dfa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 11 May 2026 14:25:36 +0000 Subject: [PATCH 09/14] refactor: use ts vitest setup entrypoints Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- packages/doc-gen/config/vitest/{setup.js => setup.ts} | 0 packages/doc-gen/tsconfig.json | 2 +- packages/doc-gen/vitest.config.mts | 2 +- .../config/vitest/{setup.js => setup.ts} | 0 packages/postcss-preset-primer/tsconfig.json | 3 +-- packages/postcss-preset-primer/vitest.config.ts | 2 +- packages/react/config/vitest/{setup.js => setup.ts} | 0 packages/react/vitest.config.browser.mts | 2 +- packages/react/vitest.config.mts | 2 +- packages/styled-react/config/vitest/{setup.js => setup.ts} | 0 packages/styled-react/vitest.config.browser.ts | 2 +- packages/styled-react/vitest.config.ts | 2 +- packages/vitest-config/package.json | 2 +- packages/vitest-config/{setup.js => setup.ts} | 0 packages/vitest-config/tsconfig.json | 4 ++++ 15 files changed, 13 insertions(+), 10 deletions(-) rename packages/doc-gen/config/vitest/{setup.js => setup.ts} (100%) rename packages/postcss-preset-primer/config/vitest/{setup.js => setup.ts} (100%) rename packages/react/config/vitest/{setup.js => setup.ts} (100%) rename packages/styled-react/config/vitest/{setup.js => setup.ts} (100%) rename packages/vitest-config/{setup.js => setup.ts} (100%) create mode 100644 packages/vitest-config/tsconfig.json diff --git a/packages/doc-gen/config/vitest/setup.js b/packages/doc-gen/config/vitest/setup.ts similarity index 100% rename from packages/doc-gen/config/vitest/setup.js rename to packages/doc-gen/config/vitest/setup.ts diff --git a/packages/doc-gen/tsconfig.json b/packages/doc-gen/tsconfig.json index 564a5990051..84e1eb8860c 100644 --- a/packages/doc-gen/tsconfig.json +++ b/packages/doc-gen/tsconfig.json @@ -1,4 +1,4 @@ { "extends": "../../tsconfig.base.json", - "include": ["src"] + "include": ["src", "config/**/*.ts"] } diff --git a/packages/doc-gen/vitest.config.mts b/packages/doc-gen/vitest.config.mts index 2bfcc04ef53..6b7bcd436e5 100644 --- a/packages/doc-gen/vitest.config.mts +++ b/packages/doc-gen/vitest.config.mts @@ -6,6 +6,6 @@ export default defineConfig({ }, test: { environment: 'node', - setupFiles: ['config/vitest/setup.js'], + setupFiles: ['config/vitest/setup.ts'], }, }) diff --git a/packages/postcss-preset-primer/config/vitest/setup.js b/packages/postcss-preset-primer/config/vitest/setup.ts similarity index 100% rename from packages/postcss-preset-primer/config/vitest/setup.js rename to packages/postcss-preset-primer/config/vitest/setup.ts diff --git a/packages/postcss-preset-primer/tsconfig.json b/packages/postcss-preset-primer/tsconfig.json index fc3b910e566..d246eb6bebd 100644 --- a/packages/postcss-preset-primer/tsconfig.json +++ b/packages/postcss-preset-primer/tsconfig.json @@ -4,6 +4,5 @@ "allowJs": true, "checkJs": true }, - "include": ["src", "vitest.config.ts"] + "include": ["src", "vitest.config.ts", "config/**/*.ts"] } - diff --git a/packages/postcss-preset-primer/vitest.config.ts b/packages/postcss-preset-primer/vitest.config.ts index b227e7de153..673a53f3cd8 100644 --- a/packages/postcss-preset-primer/vitest.config.ts +++ b/packages/postcss-preset-primer/vitest.config.ts @@ -3,6 +3,6 @@ import {defineConfig} from 'vitest/config' export default defineConfig({ test: { environment: 'node', - setupFiles: ['config/vitest/setup.js'], + setupFiles: ['config/vitest/setup.ts'], }, }) diff --git a/packages/react/config/vitest/setup.js b/packages/react/config/vitest/setup.ts similarity index 100% rename from packages/react/config/vitest/setup.js rename to packages/react/config/vitest/setup.ts diff --git a/packages/react/vitest.config.browser.mts b/packages/react/vitest.config.browser.mts index b9d5f9b231c..d027bebdcc8 100644 --- a/packages/react/vitest.config.browser.mts +++ b/packages/react/vitest.config.browser.mts @@ -47,7 +47,7 @@ export default defineConfig({ 'src/__tests__/storybook.test.tsx', ], include: ['src/**/*.test.?(c|m)[jt]s?(x)'], - setupFiles: ['config/vitest/setup.js', 'config/vitest/browser/setup.ts'], + setupFiles: ['config/vitest/setup.ts', 'config/vitest/browser/setup.ts'], css: { include: [/.+/], }, diff --git a/packages/react/vitest.config.mts b/packages/react/vitest.config.mts index 1aae80cef91..47f80446ec1 100644 --- a/packages/react/vitest.config.mts +++ b/packages/react/vitest.config.mts @@ -25,6 +25,6 @@ export default defineConfig({ name: '@primer/react (node)', include: ['src/__tests__/exports.test.ts', 'src/__tests__/storybook.test.tsx'], environment: 'node', - setupFiles: ['config/vitest/setup.js'], + setupFiles: ['config/vitest/setup.ts'], }, }) diff --git a/packages/styled-react/config/vitest/setup.js b/packages/styled-react/config/vitest/setup.ts similarity index 100% rename from packages/styled-react/config/vitest/setup.js rename to packages/styled-react/config/vitest/setup.ts diff --git a/packages/styled-react/vitest.config.browser.ts b/packages/styled-react/vitest.config.browser.ts index f634e1eba3a..b2b74225d7f 100644 --- a/packages/styled-react/vitest.config.browser.ts +++ b/packages/styled-react/vitest.config.browser.ts @@ -28,7 +28,7 @@ export default defineConfig({ test: { name: '@primer/styled-react (browser)', include: ['src/**/*.browser.test.?(c|m)[jt]s?(x)'], - setupFiles: ['config/vitest/setup.js', 'config/vitest/browser/setup.ts'], + setupFiles: ['config/vitest/setup.ts', 'config/vitest/browser/setup.ts'], browser: { provider: playwright(), enabled: true, diff --git a/packages/styled-react/vitest.config.ts b/packages/styled-react/vitest.config.ts index 86f79d9f42a..0dfa8b1e981 100644 --- a/packages/styled-react/vitest.config.ts +++ b/packages/styled-react/vitest.config.ts @@ -5,6 +5,6 @@ export default defineConfig({ name: '@primer/styled-react (node)', environment: 'node', exclude: ['src/**/*.browser.test.?(c|m)[jt]s?(x)'], - setupFiles: ['config/vitest/setup.js'], + setupFiles: ['config/vitest/setup.ts'], }, }) diff --git a/packages/vitest-config/package.json b/packages/vitest-config/package.json index 79ce4aa2386..e26f7739689 100644 --- a/packages/vitest-config/package.json +++ b/packages/vitest-config/package.json @@ -3,7 +3,7 @@ "private": true, "type": "module", "exports": { - "./setup": "./setup.js" + "./setup": "./setup.ts" }, "dependencies": { "vitest-fail-on-console": "^0.10.1" diff --git a/packages/vitest-config/setup.js b/packages/vitest-config/setup.ts similarity index 100% rename from packages/vitest-config/setup.js rename to packages/vitest-config/setup.ts diff --git a/packages/vitest-config/tsconfig.json b/packages/vitest-config/tsconfig.json new file mode 100644 index 00000000000..9960bdbd289 --- /dev/null +++ b/packages/vitest-config/tsconfig.json @@ -0,0 +1,4 @@ +{ + "extends": "../../tsconfig.base.json", + "include": ["setup.ts"] +} From 228e35046ebc65389d9109ac84f640ef6240cbf3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 11 May 2026 15:38:19 +0000 Subject: [PATCH 10/14] fix: scope doc-gen declaration build inputs Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- packages/doc-gen/tsconfig.build.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/doc-gen/tsconfig.build.json b/packages/doc-gen/tsconfig.build.json index f8dbf0c35ca..ddd416c7b80 100644 --- a/packages/doc-gen/tsconfig.build.json +++ b/packages/doc-gen/tsconfig.build.json @@ -4,5 +4,6 @@ "emitDeclarationOnly": true, "outDir": "./dist", "rootDir": "./src" - } + }, + "include": ["src/**/*"] } From 358b5f79346ea2920c2a2a2fe1d673d9646ea2af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 12 May 2026 17:21:58 +0000 Subject: [PATCH 11/14] test: remove vitest console allowlist dependencies Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- packages/react/src/ActionList/Group.test.tsx | 26 ++++++++++++++++++- .../react/src/ActionList/Heading.test.tsx | 8 +++++- .../src/CheckboxGroup/CheckboxGroup.test.tsx | 13 +++++++++- .../src/Details/__tests__/Details.test.tsx | 25 ++++++++++++++++-- .../__tests__/FormControl.test.tsx | 26 ++++++++++++++++--- packages/react/src/Radio/Radio.test.tsx | 15 ++++++++++- .../react/src/RadioGroup/RadioGroup.test.tsx | 13 +++++++++- .../SegmentedControl.test.tsx | 13 ++++++++-- .../src/TooltipV2/__tests__/Tooltip.test.tsx | 12 ++++++++- .../src/UnderlineNav/UnderlineNav.test.tsx | 10 +++++++ .../react/src/UnderlineNav/UnderlineNav.tsx | 2 ++ .../__tests__/CheckboxOrRadioGroup.test.tsx | 17 ++++++++++-- .../UnderlinePanels/UnderlinePanels.test.tsx | 12 +++++++++ .../src/hooks/__tests__/useSlots.test.tsx | 8 ++++++ packages/vitest-config/setup.ts | 16 ------------ 15 files changed, 185 insertions(+), 31 deletions(-) diff --git a/packages/react/src/ActionList/Group.test.tsx b/packages/react/src/ActionList/Group.test.tsx index 7d282da6296..418390eb723 100644 --- a/packages/react/src/ActionList/Group.test.tsx +++ b/packages/react/src/ActionList/Group.test.tsx @@ -1,4 +1,4 @@ -import {describe, it, expect} from 'vitest' +import {describe, it, expect, vi} from 'vitest' import {render as HTMLRender} from '@testing-library/react' import {PlusIcon} from '@primer/octicons-react' import BaseStyles from '../BaseStyles' @@ -22,6 +22,8 @@ describe('ActionList.Group', () => { implementsClassName(ActionList.GroupHeading, classes.GroupHeading) it('should throw an error when ActionList.GroupHeading has an `as` prop when it is used within ActionMenu context', async () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(() => HTMLRender( @@ -40,6 +42,10 @@ describe('ActionList.Group', () => { ).toThrow( "Looks like you are trying to set a heading level to a menu role. Group headings for menu type action lists are for representational purposes, and rendered as divs. Therefore they don't need a heading level.", ) + + expect(consoleErrorSpy).toHaveBeenCalled() + + consoleErrorSpy.mockRestore() }) it('should render the ActionList.GroupHeading component as a heading with the given heading level', async () => { @@ -56,6 +62,8 @@ describe('ActionList.Group', () => { expect(heading).toHaveTextContent('Group Heading') }) it('should throw an error if ActionList.GroupHeading is used without an `as` prop when no role is specified (for list role)', async () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(() => HTMLRender( @@ -69,6 +77,10 @@ describe('ActionList.Group', () => { ).toThrow( "You are setting a heading for a list, that requires a heading level. Please use 'as' prop to set a proper heading level.", ) + + expect(consoleErrorSpy).toHaveBeenCalled() + + consoleErrorSpy.mockRestore() }) it('should render the ActionList.GroupHeading component as a span (not a heading tag) when role is specified as listbox', async () => { const container = HTMLRender( @@ -191,6 +203,8 @@ describe('ActionList.Group', () => { }) it('throws when GroupHeading.TrailingAction is used inside an ActionMenu (menu role) and the feature flag is enabled', () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(() => HTMLRender( @@ -212,9 +226,15 @@ describe('ActionList.Group', () => { , ), ).toThrow(/can not be used inside an ActionList with an ARIA role of "menu"/) + + expect(consoleErrorSpy).toHaveBeenCalled() + + consoleErrorSpy.mockRestore() }) it('throws when GroupHeading.TrailingAction is used inside a listbox role and the feature flag is enabled', () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(() => HTMLRender( @@ -229,6 +249,10 @@ describe('ActionList.Group', () => { , ), ).toThrow(/can not be used inside an ActionList with an ARIA role of "listbox"/) + + expect(consoleErrorSpy).toHaveBeenCalled() + + consoleErrorSpy.mockRestore() }) }) }) diff --git a/packages/react/src/ActionList/Heading.test.tsx b/packages/react/src/ActionList/Heading.test.tsx index 316d4bad06d..937ab2ea4a4 100644 --- a/packages/react/src/ActionList/Heading.test.tsx +++ b/packages/react/src/ActionList/Heading.test.tsx @@ -1,4 +1,4 @@ -import {describe, it, expect} from 'vitest' +import {describe, it, expect, vi} from 'vitest' import {render as HTMLRender} from '@testing-library/react' import BaseStyles from '../BaseStyles' import {ActionList} from '.' @@ -42,6 +42,8 @@ describe('ActionList.Heading', () => { }) it('should throw an error when ActionList.Heading is used within ActionMenu context', async () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(() => HTMLRender( @@ -59,5 +61,9 @@ describe('ActionList.Heading', () => { ).toThrow( "ActionList.Heading shouldn't be used within an ActionMenu container. Menus are labelled by the menu button's name.", ) + + expect(consoleErrorSpy).toHaveBeenCalled() + + consoleErrorSpy.mockRestore() }) }) diff --git a/packages/react/src/CheckboxGroup/CheckboxGroup.test.tsx b/packages/react/src/CheckboxGroup/CheckboxGroup.test.tsx index eba8cea6ca8..a8d99786425 100644 --- a/packages/react/src/CheckboxGroup/CheckboxGroup.test.tsx +++ b/packages/react/src/CheckboxGroup/CheckboxGroup.test.tsx @@ -6,7 +6,18 @@ import {implementsClassName} from '../utils/testing' import classes from '../internal/components/CheckboxOrRadioGroup/CheckboxOrRadioGroup.module.css' describe('CheckboxGroup', () => { - implementsClassName(CheckboxGroup, classes.GroupFieldset) + implementsClassName( + props => ( + + Choices + + + Choice one + + + ), + classes.GroupFieldset, + ) implementsClassName(CheckboxGroup.Caption, classes.CheckboxOrRadioGroupCaption) implementsClassName(CheckboxGroup.Label, classes.RadioGroupLabel) const mockWarningFn = vi.fn() diff --git a/packages/react/src/Details/__tests__/Details.test.tsx b/packages/react/src/Details/__tests__/Details.test.tsx index 61afbccec04..936b2334f05 100644 --- a/packages/react/src/Details/__tests__/Details.test.tsx +++ b/packages/react/src/Details/__tests__/Details.test.tsx @@ -1,4 +1,4 @@ -import {describe, expect, it} from 'vitest' +import {describe, expect, it, vi} from 'vitest' import {render, screen} from '@testing-library/react' import userEvent from '@testing-library/user-event' import {Details, useDetails, Button} from '../..' @@ -7,8 +7,29 @@ import {implementsClassName} from '../../utils/testing' import classes from '../Details.module.css' describe('Details', () => { - implementsClassName(Details, classes.Details) + implementsClassName( + props => ( +
+ summary +
+ ), + classes.Details, + ) implementsClassName(Details.Summary) + + it('warns when rendered without a summary child', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + render(
) + + expect(warnSpy).toHaveBeenCalledWith( + 'Warning:', + 'The
component must have a child component. You can either use or a native element.', + ) + + warnSpy.mockRestore() + }) + it('Toggles when you click outside', async () => { const Component = () => { const {getDetailsProps} = useDetails({closeOnOutsideClick: true}) diff --git a/packages/react/src/FormControl/__tests__/FormControl.test.tsx b/packages/react/src/FormControl/__tests__/FormControl.test.tsx index bd3141abdd6..edb83d8b4df 100644 --- a/packages/react/src/FormControl/__tests__/FormControl.test.tsx +++ b/packages/react/src/FormControl/__tests__/FormControl.test.tsx @@ -59,8 +59,24 @@ const WrappedValidationComponent: FCWithSlotMarker = () => ( WrappedValidationComponent.__SLOT__ = FormControl.Validation.__SLOT__ describe('FormControl', () => { - implementsClassName(FormControl, classes.ControlVerticalLayout) - implementsClassName(props => , classes.ControlHorizontalLayout) + implementsClassName( + props => ( + + {LABEL_TEXT} + + + ), + classes.ControlVerticalLayout, + ) + implementsClassName( + props => ( + + {LABEL_TEXT} + + + ), + classes.ControlHorizontalLayout, + ) implementsClassName(FormControl.Caption, captionClasses.Caption) implementsClassName(FormControl.Label, inputClasses.Label) @@ -405,7 +421,11 @@ describe('FormControl', () => { , ) - expect(spy).toHaveBeenCalledTimes(1) + expect(spy).toHaveBeenCalledWith( + expect.stringMatching( + /^The input field with the id .+ MUST have a FormControl\.Label child\.\n\nIf you want to hide the label, pass the 'visuallyHidden' prop to the FormControl\.Label component\.$/, + ), + ) spy.mockRestore() }) diff --git a/packages/react/src/Radio/Radio.test.tsx b/packages/react/src/Radio/Radio.test.tsx index 854eb350790..fa6b6f4062c 100644 --- a/packages/react/src/Radio/Radio.test.tsx +++ b/packages/react/src/Radio/Radio.test.tsx @@ -5,12 +5,13 @@ import {implementsClassName} from '../utils/testing' import classes from './Radio.module.css' describe('Radio', () => { - implementsClassName(Radio, classes.Radio) const defaultProps = { name: 'mock', value: 'mock value', } + implementsClassName(props => , classes.Radio) + beforeEach(() => { vi.resetAllMocks() }) @@ -49,6 +50,18 @@ describe('Radio', () => { expect(radio.checked).toEqual(true) }) + it('warns when rendered without a name', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + render() + + expect(warnSpy).toHaveBeenCalledWith( + 'A radio input must have a `name` attribute. Pass `name` as a prop directly to each Radio, or nest them in a `RadioGroup` component with a `name` prop', + ) + + warnSpy.mockRestore() + }) + it('accepts a change handler that can alter a single radio state', () => { const handleChange = vi.fn() const {getByRole} = render() diff --git a/packages/react/src/RadioGroup/RadioGroup.test.tsx b/packages/react/src/RadioGroup/RadioGroup.test.tsx index 791e9e5188c..c6c5363ce62 100644 --- a/packages/react/src/RadioGroup/RadioGroup.test.tsx +++ b/packages/react/src/RadioGroup/RadioGroup.test.tsx @@ -6,7 +6,18 @@ import {implementsClassName} from '../utils/testing' import classes from '../internal/components/CheckboxOrRadioGroup/CheckboxOrRadioGroup.module.css' describe('RadioGroup', () => { - implementsClassName(RadioGroup, classes.GroupFieldset) + implementsClassName( + props => ( + + Choices + + + Choice one + + + ), + classes.GroupFieldset, + ) implementsClassName(RadioGroup.Caption, classes.CheckboxOrRadioGroupCaption) implementsClassName(RadioGroup.Label, classes.RadioGroupLabel) const mockWarningFn = vi.fn() diff --git a/packages/react/src/SegmentedControl/SegmentedControl.test.tsx b/packages/react/src/SegmentedControl/SegmentedControl.test.tsx index 6a637d1ba30..c005224183a 100644 --- a/packages/react/src/SegmentedControl/SegmentedControl.test.tsx +++ b/packages/react/src/SegmentedControl/SegmentedControl.test.tsx @@ -32,7 +32,14 @@ const segmentData = [ ] describe('SegmentedControl', () => { - implementsClassName(SegmentedControl, classes.SegmentedControl) + implementsClassName( + props => ( + + Preview + + ), + classes.SegmentedControl, + ) it('renders with a selected segment - controlled', () => { const {getByText} = render( @@ -332,7 +339,9 @@ describe('SegmentedControl', () => { , ) - expect(spy).toHaveBeenCalled() + expect(spy).toHaveBeenCalledWith( + 'Use the `aria-label` or `aria-labelledby` prop to provide an accessible label for assistive technologies', + ) spy.mockRestore() }) diff --git a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx index a2755af56f0..edfa70730e9 100644 --- a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx +++ b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx @@ -1,5 +1,5 @@ import type React from 'react' -import {describe, expect, it} from 'vitest' +import {describe, expect, it, vi} from 'vitest' import type {TooltipProps} from '../Tooltip' import {Tooltip} from '../Tooltip' import {render as HTMLRender} from '@testing-library/react' @@ -125,6 +125,8 @@ describe('Tooltip', () => { expect(triggerEL.getAttribute('aria-describedby')).toContain('custom-tooltip-id') }) it('should throw an error if the trigger element is disabled', () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(() => { HTMLRender( @@ -134,6 +136,14 @@ describe('Tooltip', () => { }).toThrow( 'The `Tooltip` component expects a single React element that contains interactive content. Consider using a `