-
Notifications
You must be signed in to change notification settings - Fork 57
refactor(mergeRefs): Enhance mergeRefs to handle optional cleanup functions returned by callback refs #281
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
base: main
Are you sure you want to change the base?
Conversation
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 enhances the mergeRefs utility to support cleanup functions returned by callback refs, a new React 19 feature that enables better resource management and prevents memory leaks.
- Adds support for cleanup functions returned by callback refs
- Maintains backward compatibility with existing ref patterns
- Updates type definitions to use React's
Ref<T>type for better type safety
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils/mergeRefs/mergeRefs.ts | Core implementation updated to handle cleanup functions and improved type safety |
| src/utils/mergeRefs/mergeRefs.spec.ts | Added comprehensive tests for cleanup function behavior |
| src/utils/mergeRefs/mergeRefs.md | Updated documentation to reflect new type signature |
| src/utils/mergeRefs/ko/mergeRefs.md | Updated Korean documentation with new type signature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return ref(value); | ||
| } | ||
|
|
||
| ref.current = value; |
Copilot
AI
Sep 16, 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.
The assignRef function should return undefined when handling object refs to match the expected RefCleanup<T> return type. Add an explicit return statement.
| ref.current = value; | |
| ref.current = value; | |
| return undefined; |
src/utils/mergeRefs/mergeRefs.ts
Outdated
| const availableRefs = refs.filter(ref => ref != null); | ||
| const cleanupMap = new Map<Ref<T>, Exclude<RefCleanup<T>, void>>(); |
Copilot
AI
Sep 16, 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.
The availableRefs array and cleanupMap are recreated on every call to mergeRefs. Consider moving these outside the returned function or using a more efficient approach to avoid repeated allocations.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #281 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 37 37
Lines 1093 1108 +15
Branches 324 329 +5
=========================================
+ Hits 1093 1108 +15 🚀 New features to boost your workflow:
|
Overview
This PR updates the
mergeRefsutility to supportcleanup functionsreturned bycallback refs, a new behavior introduced in React 19.React 19 allows ref callbacks to return a
cleanup function, which React invokes when the ref is detached or the component unmounts.This enhancement enables better resource management and cleanup (e.g., event listener removal, timers cleanup) tightly coupled to the lifecycle of the referenced element.
To maintain backward compatibility, if a callback ref does not return a cleanup function, React calls the ref with
nullupon detachment. This legacy behavior will be deprecated in future React releases in favor of cleanup functions.This change future-proofs the utility and aligns it with modern React best practices, improving reliability and developer experience.
Why this matters
Checklist
yarn run fixto format and lint the code and docs?yarn run test:coverageto make sure there is no uncovered line?