perf(useRefObjectAsForwardedRef): add dependency array to useImperativeHandle#7553
Conversation
🦋 Changeset detectedLatest commit: 5026569 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a performance optimization to the useRefObjectAsForwardedRef hook by adding a dependency array to the useImperativeHandle call. The hook is used by 13 components throughout the codebase to synchronize a local ref object with a forwarded ref, enabling both local and external access to DOM elements.
Changes:
- Added
[refObject]dependency array touseImperativeHandleinuseRefObjectAsForwardedRef - Added explanatory comments documenting why the dependency array is needed
- Created changeset for patch release
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/react/src/hooks/useRefObjectAsForwardedRef.ts | Added dependency array [refObject] to useImperativeHandle to prevent factory function from re-running on every render, since ref objects from useRef are stable |
| .changeset/use-ref-object-add-deps.md | Added changeset documenting the performance improvement as a patch release |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/14135 |
Closes #
useRefObjectAsForwardedRefcallsuseImperativeHandlewithout a dependency array, causing the factory function to re-run on every render. Since the ref object (fromuseRef) is stable, adding[refObject]as the dependency means the factory runs once after mount and is skipped on all subsequent re-renders.13 components use this hook (~90-340 instances on a typical GitHub page):
With the
[refObject]dependency array, ~100-300 unnecessary factory calls per re-render cycle drop to 0 after mount.Measurements (500 Buttons, changing count prop)
The ~1ms improvement per 500 components is expected given the factory is trivial (
() => refObject.current). The real benefit is eliminating unnecessary work in React's commit phase across all 13 consuming components on every re-render.Changelog
New
N/A
Changed
useImperativeHandlenow has a[refObject]dependency array, preventing the factory from re-running every renderRemoved
N/A
Rollout strategy
Testing & Reviewing
ref.currentshould point to the correct DOM element after mount and after re-rendersMerge checklist