Skip to content

Replace useRefObjectAsForwardedRef with useMergedRefs internally#8065

Closed
llastflowers wants to merge 18 commits into
mainfrom
create-use-combined-refs-2
Closed

Replace useRefObjectAsForwardedRef with useMergedRefs internally#8065
llastflowers wants to merge 18 commits into
mainfrom
create-use-combined-refs-2

Conversation

@llastflowers

@llastflowers llastflowers commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

recreate: #7638

revert-revert: #7737

Closes: https://github.com/github/primer/issues/6787

After #7672 deprecates useRefObjectAsForwardedRef, we need to replace internal calls with the new approach, which has several advantages:

  1. Fixes the bug identified in Revert "perf(useRefObjectAsForwardedRef): add dependency array to useImperativeHandle" #7635 (comment) where callback refs would be called on every render, by using a callback ref itself as the trigger for updating the refs. This prevents possible infinite render loops where a callback ref is used to set state.
  2. Using a callback ref, only updates the refs when the target changes, gaining the perf improvement from perf(useRefObjectAsForwardedRef): add dependency array to useImperativeHandle #7553 without the bug that came with it.
  3. Doesn't care about the order of passed arguments; the current hook will silently fail to work if you switch the arguments.
  4. Can work with two callback refs, allowing for components to use callback refs internally while still accepting external refs.
  5. Is React 19 forward-compatible with callback ref cleanup functions while remaining backwards-compatible with React 18.
  6. Has better type definitions to reduce the need for casting.
  7. Accepts undefined, a significant DX improvement for using optional React 19 ref props without having to default to null.

However, the new hook is a breaking change from the old one because it returns a combined ref that must be passed to the underlying child component. So I've still left the old hook in place (because it's public) but deprecated it so we can remove it in a future major release.

Internally, we can migrate everything over to the new approach now, which is what I've done in this PR.

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

…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 <tylerjdev@github.com>
@changeset-bot

changeset-bot Bot commented Jun 25, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b18bf21

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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

@github-actions 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 Jun 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ 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. If this doesn't work, you can also use the original workflow here. 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.

@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 20:11 Abandoned
iansan5653 and others added 2 commits June 25, 2026 13:12
…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 <tylerjdev@github.com>
@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 20:16 Abandoned
@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 20:21 Abandoned
@primer

primer Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

@primer primer Bot had a problem deploying to github-pages June 25, 2026 20:26 Failure
@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 20:26 Abandoned
@primer

primer Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 20:45 Abandoned
@primer

primer Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 20:53 Abandoned
@primer

primer Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 20:59 Abandoned
@primer

primer Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

@github-actions github-actions Bot temporarily deployed to storybook-preview-8065 June 25, 2026 21:12 Inactive
@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 25, 2026 21:21 Abandoned
@github-actions github-actions Bot temporarily deployed to storybook-preview-8065 June 25, 2026 21:31 Inactive
@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 26, 2026 17:38 Abandoned
@primer

primer Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

@github-actions github-actions Bot requested a deployment to storybook-preview-8065 June 26, 2026 17:43 Abandoned
@github-actions github-actions Bot temporarily deployed to storybook-preview-8065 June 26, 2026 17:53 Inactive
@llastflowers llastflowers added the Canary Release Apply this label when you want CI to create a canary release of the current PR label Jun 26, 2026
@primer-integration

primer-integration Bot commented Jun 26, 2026

Copy link
Copy Markdown

Integration test results from github/github-ui PR:

Failed  CI   Failed
Waiting  VRT   Waiting
Waiting  Projects   Waiting

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.

@llastflowers llastflowers marked this pull request as ready for review June 26, 2026 18:48
@llastflowers llastflowers requested a review from a team as a code owner June 26, 2026 18:48
@llastflowers llastflowers requested review from TylerJDev, Copilot and iansan5653 and removed request for Copilot June 26, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary Release Apply this label when you want CI to create a canary release of the current PR integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants