Skip to content

Conversation

@wo-o29
Copy link
Contributor

@wo-o29 wo-o29 commented Sep 16, 2025

Overview

This PR updates the mergeRefs utility to support cleanup functions returned by callback 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 null upon 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

  • Cleanup function support in callback refs simplifies complex component lifecycle management.
  • Helps prevent memory leaks by automatically cleaning up attached resources.
  • Avoids deprecated React warnings related to ref detachment behavior.
  • Ensures compatibility with upcoming React versions.

Checklist

  • Did you write the test code?
  • Have you run yarn run fix to format and lint the code and docs?
  • Have you run yarn run test:coverage to make sure there is no uncovered line?
  • Did you write the JSDoc?

Copilot AI review requested due to automatic review settings September 16, 2025 08:24
Copy link

Copilot AI left a 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;
Copy link

Copilot AI Sep 16, 2025

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.

Suggested change
ref.current = value;
ref.current = value;
return undefined;

Copilot uses AI. Check for mistakes.
Comment on lines 50 to 51
const availableRefs = refs.filter(ref => ref != null);
const cleanupMap = new Map<Ref<T>, Exclude<RefCleanup<T>, void>>();
Copy link

Copilot AI Sep 16, 2025

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.

Copilot uses AI. Check for mistakes.
@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (5bdb4ff) to head (276774e).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants