From 2437c4e78871154a646ef41aa7346741706d084f Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Mon, 30 Mar 2026 13:10:22 -0400 Subject: [PATCH 01/18] Replace `useRefObjectAsForwardedRef` with `useMergedRefs` internally (#7638) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: iansan5653 <2294248+iansan5653@users.noreply.github.com> Co-authored-by: Tyler Jones --- .changeset/combined-refs-hook.md | 5 ++++ packages/react/src/ActionBar/ActionBar.tsx | 4 ++- .../src/Autocomplete/Autocomplete.test.tsx | 1 + .../src/Autocomplete/AutocompleteOverlay.tsx | 1 + packages/react/src/Button/ButtonBase.tsx | 6 ++-- packages/react/src/Dialog/Dialog.tsx | 1 + packages/react/src/Link/Link.tsx | 28 +++++++++++++++++-- packages/react/src/Overlay/Overlay.tsx | 1 + packages/react/src/PageLayout/PageLayout.tsx | 1 + .../TextInputWithTokens.tsx | 2 ++ .../hooks/__tests__/useMergedRefs.test.tsx | 4 +-- packages/react/src/hooks/useMergedRefs.ts | 8 +++--- 12 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 .changeset/combined-refs-hook.md diff --git a/.changeset/combined-refs-hook.md b/.changeset/combined-refs-hook.md new file mode 100644 index 00000000000..e8e9c96e0cc --- /dev/null +++ b/.changeset/combined-refs-hook.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Update internal implementations of combined refs to improve performance and add support for React 19 callback refs diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 299ff7ffb6c..b063fbf6c1d 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -10,6 +10,7 @@ import {useFocusZone, FocusKeys} from '../hooks/useFocusZone' import styles from './ActionBar.module.css' import {clsx} from 'clsx' import {useMergedRefs} from '../hooks' +import {useMergedRefs} from '../hooks' import {createDescendantRegistry} from '../utils/descendant-registry' type ChildProps = @@ -352,7 +353,7 @@ function useActionBarItem(ref: React.RefObject, registryProp export const ActionBarIconButton = forwardRef( ({disabled, onClick, ...props}: ActionBarIconButtonProps, forwardedRef) => { const ref = useRef(null) - const mergedRef = useMergedRefs(forwardedRef, ref) + const mergedRef = useMergedRefs(ref, forwardedRef) const {size} = React.useContext(ActionBarContext) @@ -384,6 +385,7 @@ export const ActionBarIconButton = forwardRef( { const inputNode = getByLabelText(AUTOCOMPLETE_LABEL) expect(inputNode.getAttribute('aria-expanded')).not.toBe('true') + inputNode.focus() fireEvent.click(inputNode) fireEvent.keyDown(inputNode, {key: 'ArrowDown'}) diff --git a/packages/react/src/Autocomplete/AutocompleteOverlay.tsx b/packages/react/src/Autocomplete/AutocompleteOverlay.tsx index f330153c2ba..72c277a1ea2 100644 --- a/packages/react/src/Autocomplete/AutocompleteOverlay.tsx +++ b/packages/react/src/Autocomplete/AutocompleteOverlay.tsx @@ -6,6 +6,7 @@ import Overlay from '../Overlay' import type {ComponentProps} from '../utils/types' import {AutocompleteContext} from './AutocompleteContext' import {useMergedRefs} from '../hooks/useMergedRefs' +import {useMergedRefs} from '../hooks/useMergedRefs' import VisuallyHidden from '../_VisuallyHidden' import classes from './AutocompleteOverlay.module.css' diff --git a/packages/react/src/Button/ButtonBase.tsx b/packages/react/src/Button/ButtonBase.tsx index 19357e9ae43..d8b989bcff2 100644 --- a/packages/react/src/Button/ButtonBase.tsx +++ b/packages/react/src/Button/ButtonBase.tsx @@ -2,6 +2,7 @@ import React, {forwardRef, type JSX} from 'react' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import type {ButtonProps} from './types' import {useMergedRefs} from '../hooks/useMergedRefs' +import {useMergedRefs} from '../hooks/useMergedRefs' import {VisuallyHidden} from '../VisuallyHidden' import Spinner from '../Spinner' import CounterLabel from '../CounterLabel' @@ -51,7 +52,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f } = props const innerRef = React.useRef(null) - const mergedRef = useMergedRefs(forwardedRef, innerRef) + const combinedRefs = useMergedRefs(forwardedRef, innerRef) const uuid = useId(id) const loadingAnnouncementID = `${uuid}-loading-announcement` @@ -88,8 +89,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f aria-disabled={loading ? true : undefined} data-component="Button" {...rest} - // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent - ref={mergedRef} + ref={combinedRefs} className={clsx(classes.ButtonBase, className)} data-block={block ? 'block' : null} data-inactive={inactive ? true : undefined} diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index de9cc4f8794..2e7c5d13bf6 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -7,6 +7,7 @@ import {XIcon} from '@primer/octicons-react' import {useFocusZone} from '../hooks/useFocusZone' import {FocusKeys} from '@primer/behaviors' import Portal from '../Portal' +import {useMergedRefs} from '../hooks/useMergedRefs' import {useId} from '../hooks/useId' import {ScrollableRegion} from '../ScrollableRegion' import type {ResponsiveValue} from '../hooks/useResponsiveValue' diff --git a/packages/react/src/Link/Link.tsx b/packages/react/src/Link/Link.tsx index 5ae1e74f5ba..309519be46f 100644 --- a/packages/react/src/Link/Link.tsx +++ b/packages/react/src/Link/Link.tsx @@ -23,6 +23,31 @@ export const UnwrappedLink = ( const innerRef = React.useRef>(null) const mergedRef = useMergedRefs(ref, innerRef) + if (__DEV__) { + /** + * The Linter yells because it thinks this conditionally calls an effect, + * but since this is a compile-time flag and not a runtime conditional + * this is safe, and ensures the entire effect is kept out of prod builds + * shaving precious bytes from the output, and avoiding mounting a noop effect + */ + // eslint-disable-next-line react-hooks/rules-of-hooks + useEffect(() => { + if ( + innerRef.current && + !(innerRef.current instanceof HTMLButtonElement) && + !(innerRef.current instanceof HTMLAnchorElement) + ) { + // eslint-disable-next-line no-console + console.error( + 'Error: Found `Link` component that renders an inaccessible element', + innerRef.current, + 'Please ensure `Link` always renders as or + , ) const inputNode = getByLabelText(AUTOCOMPLETE_LABEL) + const outsideButton = screen.getByRole('button', {name: 'outside'}) expect(inputNode.getAttribute('aria-expanded')).not.toBe('true') - inputNode.focus() - fireEvent.click(inputNode) + await user.click(inputNode) fireEvent.keyDown(inputNode, {key: 'ArrowDown'}) expect(inputNode.getAttribute('aria-expanded')).toBe('true') - // `userEvent.tab()` is unreliable in browser-mode Vitest for this case; blur is deterministic. - // eslint-disable-next-line github/no-blur - fireEvent.blur(inputNode) + await user.click(outsideButton) await waitFor(() => expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')) })