Update useMergedRefs to be React 19 compatible and public + deprecate useRefObjectAsForwardedRef and useProvidedRefOrCreate (#7672)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: iansan5653

.changeset/early-ravens-visit.md

@@ -0,0 +1,7 @@
+---
+"@primer/react": minor
+---
+
+- New: Exposes new `useMergedRefs` hook that can merge two refs into a single combined ref
+- Deprecates `useRefObjectAsForwardedRef`; see doc comment for migration instructions
+- Deprecates `useProvidedRefOrCreate`; see doc comment for migration instructions

packages/react/src/Banner/Banner.tsx

@@ -3,7 +3,7 @@ import React, {forwardRef, useEffect} from 'react'
 import {AlertIcon, InfoIcon, StopIcon, CheckCircleIcon, XIcon} from '@primer/octicons-react'
 import {Button, IconButton, type ButtonProps} from '../Button'
 import {VisuallyHidden} from '../VisuallyHidden'
-import {useMergedRefs} from '../internal/hooks/useMergedRefs'
+import {useMergedRefs} from '../hooks/useMergedRefs'
 import {useId} from '../hooks/useId'
 import classes from './Banner.module.css'
 import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'

packages/react/src/Details/Details.tsx

@@ -2,7 +2,7 @@ import React, {useEffect, type ComponentPropsWithoutRef, type ReactElement} from
 import {warning} from '../utils/warning'
 import {clsx} from 'clsx'
 import classes from './Details.module.css'
-import {useMergedRefs} from '../internal/hooks/useMergedRefs'
+import {useMergedRefs} from '../hooks/useMergedRefs'
 
 const Root = React.forwardRef<HTMLDetailsElement, DetailsProps>(
   // eslint-disable-next-line @typescript-eslint/no-explicit-any

packages/react/src/__tests__/__snapshots__/exports.test.ts.snap

@@ -218,6 +218,7 @@ exports[`@primer/react > should not update exports without a semver change 1`] =
   "useFormControlForwardedProps",
   "useId",
   "useIsomorphicLayoutEffect",
+  "useMergedRefs",
   "useOnEscapePress",
   "useOnOutsideClick",
   "useOpenAndCloseFocus",

packages/react/src/hooks/__tests__/useMergedRefs.test.tsx

@@ -0,0 +1,153 @@
+import {render, renderHook} from '@testing-library/react'
+import React, {forwardRef, type RefObject} from 'react'
+import {describe, expect, it, vi} from 'vitest'
+import {useMergedRefs} from '../useMergedRefs'
+
+type InputOrButtonRef = RefObject<HTMLInputElement & HTMLButtonElement>
+
+const Component = forwardRef<HTMLInputElement & HTMLButtonElement, {asButton?: boolean}>(({asButton}, forwardedRef) => {
+  const ref: InputOrButtonRef = React.useRef(null)
+
+  const combinedRef = useMergedRefs(forwardedRef, ref)
+
+  return asButton ? <button type="button" ref={combinedRef} /> : <input ref={combinedRef} />
+})
+
+describe('useMergedRefs', () => {
+  describe('combining a forwarded ref with an internal ref', () => {
+    describe('object refs', () => {
+      it('forwards the ref to the underlying element', async () => {
+        const ref: InputOrButtonRef = {current: null}
+
+        render(<Component ref={ref} />)
+
+        expect(ref.current).toBeInstanceOf(HTMLInputElement)
+      })
+
+      it('avoids dangling references by clearing the ref on unmount', () => {
+        const ref: InputOrButtonRef = {current: null}
+
+        const {unmount} = render(<Component ref={ref} />)
+
+        expect(ref.current).toBeInstanceOf(HTMLInputElement)
+
+        unmount()
+
+        expect(ref.current).toBeNull()
+      })
+
+      it('updates the ref if the target changes', () => {
+        const ref: InputOrButtonRef = {current: null}
+
+        const {rerender} = render(<Component ref={ref} />)
+
+        expect(ref.current).toBeInstanceOf(HTMLInputElement)
+
+        rerender(<Component ref={ref} asButton />)
+
+        expect(ref.current).toBeInstanceOf(HTMLButtonElement)
+      })
+
+      it('is correctly set during an initial effect', () => {
+        // This can break if the hook delays setting the initial value until a subsequent render
+
+        const ComponentWithEffect = () => {
+          const ref = React.useRef<HTMLInputElement & HTMLButtonElement>(null)
+
+          React.useEffect(() => {
+            expect(ref.current).toBeInstanceOf(HTMLInputElement)
+          }, [])
+
+          return <Component ref={ref} />
+        }
+
+        render(<ComponentWithEffect />)
+      })
+    })
+
+    describe('callback refs', () => {
+      it('calls the ref on mount and unmount', async () => {
+        const ref = vi.fn()
+
+        const {unmount} = render(<Component ref={ref} />)
+
+        expect(ref).toHaveBeenCalledWith(expect.any(HTMLInputElement))
+
+        unmount()
+
+        expect(ref).toHaveBeenCalledWith(null)
+      })
+
+      it('calls the ref if the target changes', () => {
+        const ref = vi.fn()
+
+        const {rerender} = render(<Component ref={ref} />)
+
+        expect(ref).toHaveBeenCalledWith(expect.any(HTMLInputElement))
+
+        rerender(<Component ref={ref} asButton />)
+
+        expect(ref).toHaveBeenCalledWith(expect.any(HTMLButtonElement))
+      })
+
+      it('does not call the ref on re-render if the target does not change', () => {
+        const ref = vi.fn()
+
+        const {rerender} = render(<Component ref={ref} />)
+
+        rerender(<Component ref={ref} />)
+
+        expect(ref).toHaveBeenCalledExactlyOnceWith(expect.any(HTMLInputElement))
+      })
+    })
+  })
+
+  describe('combining two callback refs', () => {
+    it('calls both refs when the combined ref is called', () => {
+      const refA = vi.fn()
+      const refB = vi.fn()
+
+      const combined = renderHook(() => useMergedRefs(refA, refB))
+
+      combined.result.current('test')
+      expect(refA).toHaveBeenCalledExactlyOnceWith('test')
+      expect(refB).toHaveBeenCalledExactlyOnceWith('test')
+    })
+
+    it('handles React 18 null values correctly', () => {
+      const refA = vi.fn()
+      const refB = vi.fn()
+
+      const combined = renderHook(() => useMergedRefs(refA, refB))
+
+      combined.result.current('test')
+      expect(refA).toHaveBeenCalledWith('test')
+      expect(refB).toHaveBeenCalledWith('test')
+
+      // on React 18, cleanup fn will be ignored and ref will be called with null
+      combined.result.current(null)
+
+      expect(refA).toHaveBeenCalledWith(null)
+      expect(refB).toHaveBeenCalledWith(null)
+    })
+
+    it('handles React 19 cleanup functions correctly and independently', () => {
+      const refA = vi.fn()
+      const cleanupRefB = vi.fn()
+      const refB = vi.fn().mockReturnValue(cleanupRefB)
+
+      const combined = renderHook(() => useMergedRefs(refA, refB))
+
+      const cleanup = combined.result.current('test')
+      expect(refA).toHaveBeenCalledWith('test')
+      expect(refB).toHaveBeenCalledWith('test')
+
+      // React 19 will call cleanup function and not pass null
+      cleanup()
+
+      expect(refA).toHaveBeenCalledWith(null)
+      expect(refB).not.toHaveBeenCalledWith(null)
+      expect(cleanupRefB).toHaveBeenCalledOnce()
+    })
+  })
+})

packages/react/src/hooks/index.ts

@@ -16,3 +16,4 @@ export {useMnemonics} from './useMnemonics'
 export {useRefObjectAsForwardedRef} from './useRefObjectAsForwardedRef'
 export {useId} from './useId'
 export {useIsMacOS} from './useIsMacOS'
+export {useMergedRefs} from './useMergedRefs'

packages/react/src/hooks/useMergedRefs.hookDocs.json

@@ -0,0 +1,22 @@
+{
+  "name": "useMergedRefs",
+  "importPath": "@primer/react",
+  "stories": [],
+  "parameters": [
+    {
+      "name": "refA",
+      "type": "React.Ref<T> | undefined",
+      "required": true,
+      "description": "First ref to combine."
+    },
+    {
+      "name": "refB",
+      "type": "React.Ref<T> | undefined",
+      "required": true,
+      "description": "Second ref to combine."
+    }
+  ],
+  "returns": {
+    "type": "React.RefCallback<T>"
+  }
+}

packages/react/src/hooks/useMergedRefs.ts

@@ -0,0 +1,85 @@
+import type {ForwardedRef, Ref as StandardRef, MutableRefObject} from 'react'
+import {useCallback} from 'react'
+
+/**
+ * Combine two refs of matching type (typically an external or forwarded ref and an internal `useRef` object or
+ * callback ref).
+ *
+ * If you need to combine more than two refs (what are you doing?) just nest the hook:
+ * `useMergedRefs(refA, useMergedRefs(refB, refC))`
+ *
+ * @param refA First ref to merge. The order is not important.
+ * @param refB Second ref to merge. The order is not important.
+ * @returns A new ref which must be passed to the relevant child component. **Important**: do not pass `refA` or
+ * `refB` to the component!
+ *
+ * @example
+ * // React 18
+ * const Example = forwardRef<HTMLButtonElement, {}>((props, forwardedRef) => {
+ *  const ref = useRef<HTMLButtonElement>(null)
+ *  const combinedRef = useMergedRefs(forwardedRef, ref)
+ *
+ *  return <button ref={combinedRef} />
+ * })
+ *
+ * @example
+ * // React 19
+ * const Example = ({ref: externalRef}: {ref?: Ref<HTMLButtonElement>}) => {
+ *  const ref = useRef<HTMLButtonElement>(null)
+ *  const combinedRef = useMergedRefs(externalRef, ref)
+ *
+ *  return <button ref={combinedRef} />
+ * }
+ */
+export function useMergedRefs<T>(refA: Ref<T | null>, refB: Ref<T | null>) {
+  return useCallback(
+    (value: T | null) => {
+      const cleanupA = setRef(refA, value)
+      const cleanupB = setRef(refB, value)
+
+      // Only works in React 19. In React 18, the cleanup function will be ignored and the ref will get called with
+      // `null` which will be passed to each ref as expected.
+      return () => {
+        // For object refs and callback refs that don't return cleanups, we still need to pass `null` on cleanup
+        if (cleanupA) cleanupA()
+        else setRef(refA, null)
+
+        if (cleanupB) cleanupB()
+        else setRef(refB, null)
+      }
+    },
+    [refA, refB],
+  )
+}
+
+type CleanupFunction = () => void
+
+/**
+ * React 19 supports callback refs that can return a cleanup function. If a cleanup function is returned, the
+ * cleanup is called on unmount **instead** of setting the ref to null.
+ */
+// bivarianceHack copied from React types
+type React19RefCallback<T> = {
+  bivarianceHack(instance: T): void | CleanupFunction
+}['bivarianceHack']
+
+/**
+ * Supporting React 18 and 19 while alleviating the need for any hacks or casts in consumers:
+ * - `ForwardedRef` from the React 18 `forwardRef` HOC
+ * - `React19RefCallback` for callback refs that can return a cleanup function (this is included in `Ref` in React 19
+ *   but not in 18)
+ * - `Ref` for standard refs from `useRef` or passed in as React 19 prop
+ * - `undefined` to allow for easy use of optional `ref` props in React 19
+ */
+type Ref<T> = ForwardedRef<T> | React19RefCallback<T> | StandardRef<T> | undefined
+
+function setRef<T>(ref: Ref<T>, value: T) {
+  if (typeof ref === 'function') {
+    return ref(value)
+  } else if (ref) {
+    // `React.Ref` is typed as immutable to protect consumers but it's OK to mutate it here. We could just change the
+    // type to only accept mutable refs, but then it would be harder to accept refs as props in React 19 because we
+    // would have to use the `React.ForwardedRef` type instead of `React.Ref`
+    ;(ref as MutableRefObject<T>).current = value
+  }
+}

packages/react/src/hooks/useProvidedRefOrCreate.ts

@@ -7,6 +7,18 @@ import React from 'react'
  * This hook aims to encapsulate that logic, so the consumer doesn't need to be concerned with violating `rules-of-hooks`.
  * @param providedRef The ref to use - if undefined, will use the ref from a call to React.useRef
  * @type TRef The type of the RefObject which should be created.
+ *
+ * @deprecated This hook is incompatible with forwarded callback refs. Prefer `useMergedRefs` with an internally
+ * created ref instead.
+ *
+ * ```diff
+ * - const ref = useProvidedRefOrCreate(forwardedRef as RefObject<...>)
+ * + const ref = useRef(null)
+ * + const mergedRef = useMergedRefs(forwardedRef, ref)
+ *
+ * - return <div ref={ref} />
+ * + return <div ref={mergedRef} />
+ * ```
  */
 export function useProvidedRefOrCreate<TRef>(providedRef?: React.RefObject<TRef | null>): React.RefObject<TRef | null> {
   const createdRef = React.useRef<TRef>(null)

packages/react/src/hooks/useRefObjectAsForwardedRef.ts

@@ -7,6 +7,18 @@ import {useImperativeHandle} from 'react'
  * instance with `.current`.
  *
  * **NOTE**: The `refObject` should be passed to the underlying element, NOT the `forwardedRef`.
+ *
+ * @deprecated Migrate to `useMergedRefs`. It's safer, faster, and easier to use:
+ *
+ * ```diff
+ *   const ref = useRef(null)
+ *
+ * - useRefObjectAsForwardedRef(forwardedRef, ref)
+ * + const mergedRef = useMergedRefs(forwardedRef, ref)
+ *
+ * - return <div ref={ref} />
+ * + return <div ref={mergedRef} />
+ * ```
  */
 export function useRefObjectAsForwardedRef<T>(forwardedRef: ForwardedRef<T>, refObject: RefObject<T | null>): void {
   useImperativeHandle<T | null, T | null>(forwardedRef, () => refObject.current)