From 958269fdda7890b045b3718d868cfecd52ff13d2 Mon Sep 17 00:00:00 2001 From: wo-o29 Date: Tue, 16 Sep 2025 17:19:45 +0900 Subject: [PATCH 1/4] refactor: add support for cleanup functions in mergeRefs callback ref for React 19+ --- src/utils/mergeRefs/mergeRefs.ts | 45 ++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/src/utils/mergeRefs/mergeRefs.ts b/src/utils/mergeRefs/mergeRefs.ts index b643558b..5a992ea5 100644 --- a/src/utils/mergeRefs/mergeRefs.ts +++ b/src/utils/mergeRefs/mergeRefs.ts @@ -1,4 +1,7 @@ -import { RefCallback, RefObject } from 'react'; +import { Ref, RefCallback } from 'react'; + +type StrictRef = NonNullable>; +type RefCleanup = ReturnType>; /** * @description @@ -7,7 +10,7 @@ import { RefCallback, RefObject } from 'react'; * * @template T - The type of target to be referenced. * - * @param {Array | RefCallback | null | undefined>} refs - An array of refs to be merged. Each ref can be either a RefObject or RefCallback. + * @param {Array | undefined>} refs - An array of refs to be merged. Each ref can be either a RefObject or RefCallback. * * @returns {RefCallback} A single ref callback that updates all provided refs. * @@ -34,19 +37,39 @@ import { RefCallback, RefObject } from 'react'; * return
; * } */ -export function mergeRefs(...refs: Array | RefCallback | null | undefined>): RefCallback { + +function assignRef(ref: StrictRef, value: T | null): RefCleanup { + if (typeof ref === 'function') { + return ref(value); + } + + ref.current = value; +} + +export function mergeRefs(...refs: Array | undefined>): RefCallback { + const availableRefs = refs.filter(ref => ref != null); + const cleanupMap = new Map, Exclude, void>>(); + return value => { - for (const ref of refs) { - if (ref == null) { - continue; + for (const ref of availableRefs) { + const cleanup = assignRef(ref, value); + if (cleanup) { + cleanupMap.set(ref, cleanup); } + } - if (typeof ref === 'function') { - ref(value); - continue; + return () => { + for (const ref of availableRefs) { + const cleanup = cleanupMap.get(ref); + if (cleanup && typeof cleanup === 'function') { + cleanup(); + continue; + } + + assignRef(ref, null); } - (ref as RefObject).current = value; - } + cleanupMap.clear(); + }; }; } From 7104d1072d7bd6af20e18715cfdea7b19760d72a Mon Sep 17 00:00:00 2001 From: wo-o29 Date: Tue, 16 Sep 2025 17:19:58 +0900 Subject: [PATCH 2/4] docs: update mergeRefs type definitions for consistency --- src/utils/mergeRefs/ko/mergeRefs.md | 4 ++-- src/utils/mergeRefs/mergeRefs.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utils/mergeRefs/ko/mergeRefs.md b/src/utils/mergeRefs/ko/mergeRefs.md index 0cdefcbf..6aea8447 100644 --- a/src/utils/mergeRefs/ko/mergeRefs.md +++ b/src/utils/mergeRefs/ko/mergeRefs.md @@ -6,7 +6,7 @@ ```ts function mergeRefs( - ...refs: Array | RefCallback | null | undefined> + ...refs: Array | undefined> ): RefCallback; ``` @@ -15,7 +15,7 @@ function mergeRefs( diff --git a/src/utils/mergeRefs/mergeRefs.md b/src/utils/mergeRefs/mergeRefs.md index 35242c45..74f964a5 100644 --- a/src/utils/mergeRefs/mergeRefs.md +++ b/src/utils/mergeRefs/mergeRefs.md @@ -6,7 +6,7 @@ This function takes multiple refs (RefObject or RefCallback) and returns a singl ```ts function mergeRefs( - ...refs: Array | RefCallback | null | undefined> + ...refs: Array | undefined> ): RefCallback; ``` @@ -15,7 +15,7 @@ function mergeRefs( From 6ad1fcff30fd30d29e4e6fce0984705ab793507b Mon Sep 17 00:00:00 2001 From: wo-o29 Date: Tue, 16 Sep 2025 17:20:21 +0900 Subject: [PATCH 3/4] test: added tests related to cleanup functions --- src/utils/mergeRefs/mergeRefs.spec.ts | 52 ++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/src/utils/mergeRefs/mergeRefs.spec.ts b/src/utils/mergeRefs/mergeRefs.spec.ts index f27cf243..e74e5637 100644 --- a/src/utils/mergeRefs/mergeRefs.spec.ts +++ b/src/utils/mergeRefs/mergeRefs.spec.ts @@ -1,6 +1,6 @@ import { useCallback, useRef } from 'react'; import { act } from '@testing-library/react'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { renderHookSSR } from '../../_internal/test-utils/renderHookSSR.tsx'; @@ -75,4 +75,54 @@ describe('mergeRefs', () => { expect(result.current.ref1.current).toBe(value); expect(ref3Value).toBe(value); }); + + it('should call cleanup functions returned by callback refs', () => { + const cleanupCalls: string[] = []; + + const callbackRef1 = vi.fn(() => { + return () => { + cleanupCalls.push('cleanup1'); + }; + }); + + const callbackRef2 = vi.fn(() => { + return () => { + cleanupCalls.push('cleanup2'); + }; + }); + + const mergedRef = mergeRefs(callbackRef1, callbackRef2); + const value = 'test-value'; + + act(() => { + mergedRef(value); + }); + + const cleanupFn = mergedRef(null); + if (cleanupFn) { + cleanupFn(); + } + + expect(cleanupCalls).toEqual(['cleanup1', 'cleanup2']); + expect(callbackRef1).toHaveBeenCalledWith(value); + expect(callbackRef2).toHaveBeenCalledWith(value); + }); + + it('verifies that object refs initialize correctly without cleanup functions', () => { + const refObj = { current: 'initial' }; + const mergedRef = mergeRefs(refObj); + + act(() => { + mergedRef('new-value'); + }); + expect(refObj.current).toBe('new-value'); + + const cleanupFn = mergedRef(null); + expect(cleanupFn).toBeInstanceOf(Function); + + if (cleanupFn) { + cleanupFn(); + } + expect(refObj.current).toBeNull(); + }); }); From 276774ef272db56f0784f58c426ba6efa4bfa6f5 Mon Sep 17 00:00:00 2001 From: wo-o29 Date: Wed, 17 Sep 2025 15:32:29 +0900 Subject: [PATCH 4/4] fix: update cleanupMap type to use StrictRef for better type safety --- src/utils/mergeRefs/mergeRefs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/mergeRefs/mergeRefs.ts b/src/utils/mergeRefs/mergeRefs.ts index 5a992ea5..79bf65e5 100644 --- a/src/utils/mergeRefs/mergeRefs.ts +++ b/src/utils/mergeRefs/mergeRefs.ts @@ -48,7 +48,7 @@ function assignRef(ref: StrictRef, value: T | null): RefCleanup { export function mergeRefs(...refs: Array | undefined>): RefCallback { const availableRefs = refs.filter(ref => ref != null); - const cleanupMap = new Map, Exclude, void>>(); + const cleanupMap = new Map, Exclude, void>>(); return value => { for (const ref of availableRefs) {