-
Notifications
You must be signed in to change notification settings - Fork 646
chore - Fix type compatibility with react 19 refs #7246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 8283d7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/7651 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates ref types throughout the @primer/react codebase to be compatible with React 19's stricter type requirements. React 19 enforces that RefObject<T> types must be RefObject<T | null> to accurately reflect that refs can be null.
Key changes:
- Updated hook signatures to use
RefObject<T | null>instead ofRefObject<T>across 14+ custom hooks - Added
@ts-expect-errorcomments in component implementations where refs are passed to elements that haven't been updated to accept nullable refs - Updated type definitions for overlay, dialog, focus management, and positioning utilities
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/hooks/useScrollFlash.ts | Updated parameter type to accept nullable HTMLElement refs |
| packages/react/src/hooks/useResizeObserver.ts | Updated target ref parameter to nullable type |
| packages/react/src/hooks/useRefObjectAsForwardedRef.ts | Updated refObject parameter to nullable type |
| packages/react/src/hooks/useProvidedRefOrCreate.ts | Updated function signature to accept and return nullable RefObjects |
| packages/react/src/hooks/useOverlay.tsx | Updated UseOverlaySettings types to use nullable refs; has redundant type parameter issue |
| packages/react/src/hooks/useOpenAndCloseFocus.ts | Updated all ref types in settings interface to nullable |
| packages/react/src/hooks/useOnOutsideClick.tsx | Updated container and ignore refs to nullable types |
| packages/react/src/hooks/useMnemonics.ts | Updated providedRef parameter to nullable type |
| packages/react/src/hooks/useMenuKeyboardNavigation.ts | Updated containerRef and anchorRef parameters to nullable across multiple functions |
| packages/react/src/hooks/useMenuInitialFocus.ts | Updated containerRef and anchorRef parameters to nullable |
| packages/react/src/hooks/useFocusZone.ts | Updated containerRef, activeDescendantFocus, and return types to nullable |
| packages/react/src/hooks/useFocusTrap.ts | Updated containerRef, initialFocusRef, and returnFocusRef to nullable |
| packages/react/src/hooks/useDialog.ts | Updated all ref types in UseDialogParameters to nullable |
| packages/react/src/hooks/useAnchoredPosition.ts | Updated floatingElementRef and anchorElementRef to nullable types |
| packages/react/src/experimental/Tabs/Tabs.tsx | Updated ref type in useTabList return type; added ts-expect-error for div element |
| packages/react/src/experimental/Tabs/Tabs.examples.stories.tsx | Added ts-expect-error comment for ActionList ref; has inconsistent comment formatting |
| packages/react/src/experimental/SelectPanel2/SelectPanel.tsx | Updated anchorRef prop type to nullable |
| packages/react/src/deprecated/DialogV1/Dialog.tsx | Updated initialFocusRef and returnFocusRef props to nullable |
| packages/react/src/TooltipV2/Tooltip.tsx | Added ts-expect-error for triggerRef assignment |
| packages/react/src/TextInput/TextInput.tsx | Updated inputRef cast to nullable type; added ts-expect-error |
| packages/react/src/SelectPanel/SelectPanel.tsx | Added ts-expect-error for onInputRefChanged prop |
| packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx | Added VirtualItem type import and explicit type annotation; unrelated to ref changes |
| packages/react/src/PageHeader/PageHeader.tsx | Added ts-expect-error for titleAreaRef assignment |
| packages/react/src/Overlay/Overlay.tsx | Updated ignoreClickRefs, initialFocusRef, and returnFocusRef to nullable |
| packages/react/src/FilteredActionList/useAnnouncements.tsx | Updated listRef and inputRef parameters to nullable |
| packages/react/src/FilteredActionList/FilteredActionList.tsx | Updated onInputRefChanged callback and inputRef prop to nullable; added ts-expect-error comments |
| packages/react/src/Dialog/Dialog.tsx | Updated DialogButtonProps ref and Dialog initialFocusRef/returnFocusRef to nullable; added ts-expect-error |
| packages/react/src/Checkbox/Checkbox.tsx | Added ts-expect-error with inconsistent comment message |
| packages/react/src/ButtonGroup/ButtonGroup.tsx | Updated buttonRef cast to nullable; added ts-expect-error |
| packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx | Updated anchorRef prop type to nullable in both interface variants |
| packages/react/src/ActionList/List.tsx | Added ts-expect-error comment for Component ref |
| examples/nextjs/src/app/products/page.tsx | New example file; unrelated to ref type changes |
| .changeset/three-years-film.md | Documents the ref type compatibility changes |
| import React from 'react' | ||
| import {Button} from '@primer/react' | ||
|
|
||
| // Minimal products page to satisfy type generation and provide a simple sample. | ||
| export default function ProductsPage() { | ||
| return ( | ||
| <main style={{padding: '1rem'}}> | ||
| <h1>Products</h1> | ||
| <p>This is a placeholder Products page for the Next.js example.</p> | ||
| <Button variant="primary">Primary action</Button> | ||
| </main> | ||
| ) | ||
| } |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new file appears unrelated to the React 19 ref type compatibility changes described in the PR description. The addition of a products page example doesn't connect to ref type changes. Consider moving this to a separate PR to keep changes focused and easier to review.
| import React from 'react' | |
| import {Button} from '@primer/react' | |
| // Minimal products page to satisfy type generation and provide a simple sample. | |
| export default function ProductsPage() { | |
| return ( | |
| <main style={{padding: '1rem'}}> | |
| <h1>Products</h1> | |
| <p>This is a placeholder Products page for the Next.js example.</p> | |
| <Button variant="primary">Primary action</Button> | |
| </main> | |
| ) | |
| } |
|
🟢 ci completed with status |
francinelucca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! just one question
| @@ -0,0 +1,13 @@ | |||
| import React from 'react' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this? 👀 is this necessary for the React 19 changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me remove it and see if I'm still getting the errors I was
|
👋 Hi, there are new commits since the last successful integration test. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
This makes only type related changes.
RefObject<T>toRefObject<T|null>@ts-expect-error