Revert "Replace `useRefObjectAsForwardedRef` with `useMergedRefs` internally" by liuliu-dev · Pull Request #7737 · primer/react

liuliu-dev · 2026-04-08T16:38:39Z

Reverts #7638

…ernally …" This reverts commit f04e85d.

changeset-bot · 2026-04-08T16:38:42Z

⚠️ No Changeset found

Latest commit: c5f47ce

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2026-04-08T16:39:11Z

⚠️ Action required

👋 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. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

Copilot

Pull request overview

Reverts #7638 by switching internal “combined refs” usage back from useMergedRefs to useRefObjectAsForwardedRef across multiple components in @primer/react.

Changes:

Replace useMergedRefs usage with useRefObjectAsForwardedRef and pass local ref objects directly to underlying elements.
Update useMergedRefs docs/tests variable naming (mergedRef → combinedRef).
Remove the changeset that described the (now-reverted) internal ref-handling update.

Show a summary per file

File	Description
packages/react/src/TextInputWithTokens/TextInputWithTokens.tsx	Reverts forwarded-ref wiring back to `useRefObjectAsForwardedRef` + local `ref`.
packages/react/src/PageLayout/PageLayout.tsx	Reverts Pane/Sidebar forwarded-ref wiring back to `useRefObjectAsForwardedRef`.
packages/react/src/Overlay/Overlay.tsx	Reverts overlay ref wiring back to `useRefObjectAsForwardedRef`.
packages/react/src/Link/Link.tsx	Reverts ref wiring and introduces a ref cast to satisfy polymorphic typing.
packages/react/src/hooks/useMergedRefs.ts	Renames variables in JSDoc examples (`mergedRef` → `combinedRef`).
packages/react/src/hooks/tests/useMergedRefs.test.tsx	Renames test variable usage accordingly.
packages/react/src/Heading/Heading.tsx	Reverts forwarded-ref wiring back to `useRefObjectAsForwardedRef`.
packages/react/src/Dialog/Dialog.tsx	Reverts forwarded-ref wiring back to `useRefObjectAsForwardedRef`.
packages/react/src/deprecated/DialogV1/Dialog.tsx	Reverts forwarded-ref wiring back to `useRefObjectAsForwardedRef`.
packages/react/src/Button/ButtonBase.tsx	Reverts forwarded-ref wiring and changes ref assignment on polymorphic component.
packages/react/src/Autocomplete/AutocompleteOverlay.tsx	Reverts combined ref handling for overlay scroll container.
packages/react/src/Autocomplete/AutocompleteInput.tsx	Reverts forwarded-ref wiring back to `useRefObjectAsForwardedRef`.
packages/react/src/Autocomplete/Autocomplete.test.tsx	Removes an explicit focus call in a menu-open test.
packages/react/src/ActionList/Heading.tsx	Reverts forwarded-ref wiring back to `useRefObjectAsForwardedRef`.
packages/react/src/ActionBar/ActionBar.tsx	Reverts forwarded-ref wiring back to `useRefObjectAsForwardedRef` for icon button.
.changeset/combined-refs-hook.md	Deletes the changeset introduced by the reverted PR.

Copilot's findings

Files reviewed: 16/16 changed files
Comments generated: 4

Copilot · 2026-04-08T16:48:29Z

packages/react/src/Link/Link.tsx

      data-inline={inline}
      data-hover-color={hoverColor}
      {...restProps}
-      ref={mergedRef}
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      ref={innerRef as any}


ref={innerRef as any} (and disabling the lint rule) bypasses type-safety for the polymorphic as prop and can mask cases where the rendered component doesn’t support refs. Prefer keeping the ref fully typed (e.g., by using useMergedRefs again, or by expressing the polymorphic ref type explicitly) so TS can enforce that ref is valid for As without resorting to any.

Copilot · 2026-04-08T16:48:30Z

packages/react/src/Button/ButtonBase.tsx

+        // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent
+        ref={innerRef}


Avoid introducing @ts-ignore on the ref assignment here; it hides a real typing mismatch between the polymorphic as element and innerRef’s declared element type. Consider typing innerRef to match the union of supported elements (e.g., button/anchor) or switching back to useMergedRefs so forwardedRef can be attached without suppressing TypeScript errors.

Suggested change

// @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent

ref={innerRef}

ref={

Component === 'a'

? (innerRef as React.Ref<HTMLAnchorElement>)

: (innerRef as React.Ref<HTMLButtonElement>)

}

Copilot · 2026-04-08T16:48:30Z

packages/react/src/Autocomplete/Autocomplete.test.tsx

      const inputNode = getByLabelText(AUTOCOMPLETE_LABEL)

      expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
-      inputNode.focus()
      fireEvent.click(inputNode)
      fireEvent.keyDown(inputNode, {key: 'ArrowDown'})



Removing inputNode.focus() means this test now relies on fireEvent.click(inputNode) to focus/open the menu, which can make it pass even if the intended “ArrowDown when focused opens menu” behavior regresses (click may open the menu by itself). To keep the assertion targeted, focus the input (or assert the menu remains closed after click) before the ArrowDown keyDown.

Copilot · 2026-04-08T16:48:30Z

packages/react/src/Overlay/Overlay.tsx

    const featureFlagMaxHeightClampToViewport = useFeatureFlag('primer_react_overlay_max_height_clamp_to_viewport')
    const overlayRef = useRef<HTMLDivElement>(null)
-    const mergedRef = useMergedRefs(forwardedRef, overlayRef)
+    useRefObjectAsForwardedRef(forwardedRef, overlayRef)
    const slideAnimationDistance = 8 // var(--base-size-8), hardcoded to do some math


Switching back to useRefObjectAsForwardedRef reintroduces its known behavior of invoking callback refs on every re-render (see useRefObjectAsForwardedRef tests’ TODO), which can cause unnecessary work/side effects and is especially risky with React 19 callback refs that return cleanup functions. Prefer useMergedRefs here so refs are only updated on mount/unmount/target changes and React 19 cleanup refs are handled correctly.

primer-integration · 2026-04-08T17:11:38Z

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/18110

primer-integration · 2026-04-08T17:20:16Z

Integration test results from github/github-ui:

CI Failed
VRT Running
Projects Passed

CI check runs linting, type checking, and unit tests. Check the workflow logs for specific failures.

Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance.

Revert "Replace useRefObjectAsForwardedRef with useMergedRefs int…

c5f47ce

…ernally …" This reverts commit f04e85d.

github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Apr 8, 2026

liuliu-dev marked this pull request as ready for review April 8, 2026 16:43

liuliu-dev requested a review from a team as a code owner April 8, 2026 16:43

liuliu-dev requested review from Copilot and siddharthkp April 8, 2026 16:43

Copilot started reviewing on behalf of liuliu-dev April 8, 2026 16:43 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

liuliu-dev deployed to github-pages April 8, 2026 16:49 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-7737 April 8, 2026 16:50 View deployment

liuliu-dev added the Canary Release Apply this label when you want CI to create a canary release of the current PR label Apr 8, 2026

liuliu-dev added integration-tests: skipped manually Changes in this PR do not require an integration test and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Apr 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "Replace `useRefObjectAsForwardedRef` with `useMergedRefs` internally"#7737

Revert "Replace `useRefObjectAsForwardedRef` with `useMergedRefs` internally"#7737
liuliu-dev wants to merge 1 commit intomainfrom
revert-7638-create-use-combined-refs

liuliu-dev commented Apr 8, 2026

Uh oh!

changeset-bot bot commented Apr 8, 2026

Uh oh!

github-actions bot commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

primer-integration bot commented Apr 8, 2026

Uh oh!

primer-integration bot commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent
		ref={innerRef}

-        // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent
-        ref={innerRef}
+        ref={
+          Component === 'a'
+            ? (innerRef as React.Ref<HTMLAnchorElement>)
+            : (innerRef as React.Ref<HTMLButtonElement>)
+        }

Conversation

liuliu-dev commented Apr 8, 2026

Uh oh!

changeset-bot bot commented Apr 8, 2026

⚠️ No Changeset found

Uh oh!

github-actions bot commented Apr 8, 2026

⚠️ Action required

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

primer-integration bot commented Apr 8, 2026

Uh oh!

primer-integration bot commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants