fix: be more defensive in useMergedRef (#75088)

- Fixes a crash when a Link unmounts (under some VERY specific
conditions)
- Reverts #75012, which implemented a workaround for the crash

### Short explanation

The crash happens because we use a cleanup-returning ref in (the app-dir
version of) Link, and `<Link legacyBehavior>` "leaks" that ref into user
code. If user code does ref-merging incorrectly, i.e. it treats all
callback refs as the old style that gets called with `null`, then our
ref can get called with `null` even though it shouldn't, and crashes.

Technically this crash is not a Next.js bug. But I think a lot user code
uses ref-merging libraries that haven't been updated for 19, and can
crash because of this. So i think the safest course of action is to be
defensive, and change `useMergedRef` to always convert cleanup refs to
normal function refs, so that userspace can't mess up our refs.

### Full explanation

- `observeLinkVisibilityOnMount` inside our Link is a cleanup-returning
ref
- i.e. the new thing added in react 19, `(el: Element) => () =>
void` instead of the old `(el: Element | null) => void`
- user code does  `<Link legacyBehavior><CustomComponent /></Link>`
- `legacyBehavior` means that props get spread onto the child,
including `ref`, so we'll pass the ref created inside Link
to  `CustomComponent`
- AND no `ref` is passed on the `<CustomComponent>`  itself
- with `legacyBehavior`, that'd get extracted and merged with
`observeLinkVisibilityOnMount`
- if there's no user ref, the old implementation of `useMergedRef` would
return `observeLinkVisibilityOnMount` unchanged, thus
passing `CustomComponent` a cleanup ref
- `CustomComponent` uses a buggy ref merging library, i.e. one that
wasn't updated to handle cleanup refs.
- **so `observeLinkVisibilityOnMount` gets treated as an old-style
callback ref, i.e. it gets called with`null`. and stuff explodes,
because it doesn't expect that**

The test in `test/production/next-link-legacybehavior-ref-merging`
reproduces the crash. You can check out the commit that adds it to see
it happen -- I put it before the fix.

packages/next/src/client/app-dir/link.tsx

@@ -640,17 +640,12 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
     // currently mounted <Link> instances, e.g. so we can re-prefetch them after
     // a revalidation or refresh.
     const observeLinkVisibilityOnMount = React.useCallback(
-      (element: HTMLAnchorElement | SVGAElement | null) => {
+      (element: HTMLAnchorElement | SVGAElement) => {
         if (prefetchEnabled && router !== null) {
-          // FIXME: element still can be null here in some cases. Require further investigation.
-          if (element) {
-            mountLinkInstance(element, href, router, appPrefetchKind)
-          }
+          mountLinkInstance(element, href, router, appPrefetchKind)
         }
         return () => {
-          if (element) {
-            unmountLinkInstance(element)
-          }
+          unmountLinkInstance(element)
         }
       },
       [prefetchEnabled, href, router, appPrefetchKind]

packages/next/src/client/link.tsx

@@ -466,7 +466,7 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
     })
 
     const setIntersectionWithResetRef = React.useCallback(
-      (el: Element) => {
+      (el: Element | null) => {
         // Before the link getting observed, check if visible state need to be reset
         if (previousAs.current !== as || previousHref.current !== href) {
           resetVisible()

packages/next/src/client/use-merged-ref.ts

@@ -1,4 +1,4 @@
-import { useMemo, useRef, type Ref } from 'react'
+import { useCallback, useRef, type Ref } from 'react'
 
 // This is a compatibility hook to support React 18 and 19 refs.
 // In 19, a cleanup function from refs may be returned.
@@ -11,24 +11,40 @@ export function useMergedRef<TElement>(
   refA: Ref<TElement>,
   refB: Ref<TElement>
 ): Ref<TElement> {
-  const cleanupA = useRef<() => void>(() => {})
-  const cleanupB = useRef<() => void>(() => {})
+  const cleanupA = useRef<(() => void) | null>(null)
+  const cleanupB = useRef<(() => void) | null>(null)
 
-  return useMemo(() => {
-    if (!refA || !refB) {
-      return refA || refB
-    }
-
-    return (current: TElement | null): void => {
+  // NOTE: In theory, we could skip the wrapping if only one of the refs is non-null.
+  // (this happens often if the user doesn't pass a ref to Link/Form/Image)
+  // But this can cause us to leak a cleanup-ref into user code (e.g. via `<Link legacyBehavior>`),
+  // and the user might pass that ref into ref-merging library that doesn't support cleanup refs
+  // (because it hasn't been updated for React 19)
+  // which can then cause things to blow up, because a cleanup-returning ref gets called with `null`.
+  // So in practice, it's safer to be defensive and always wrap the ref, even on React 19.
+  return useCallback(
+    (current: TElement | null): void => {
       if (current === null) {
-        cleanupA.current()
-        cleanupB.current()
+        const cleanupFnA = cleanupA.current
+        if (cleanupFnA) {
+          cleanupA.current = null
+          cleanupFnA()
+        }
+        const cleanupFnB = cleanupB.current
+        if (cleanupFnB) {
+          cleanupB.current = null
+          cleanupFnB()
+        }
       } else {
-        cleanupA.current = applyRef(refA, current)
-        cleanupB.current = applyRef(refB, current)
+        if (refA) {
+          cleanupA.current = applyRef(refA, current)
+        }
+        if (refB) {
+          cleanupB.current = applyRef(refB, current)
+        }
       }
-    }
-  }, [refA, refB])
+    },
+    [refA, refB]
+  )
 }
 
 function applyRef<TElement>(

test/production/next-link-legacybehavior-ref-merging/app/layout.tsx

@@ -0,0 +1,13 @@
+import * as React from 'react'
+
+export default function RootLayout({
+  children,
+}: {
+  children: React.ReactNode
+}) {
+  return (
+    <html lang="en">
+      <body>{children}</body>
+    </html>
+  )
+}

test/production/next-link-legacybehavior-ref-merging/app/link-target/page.tsx

@@ -0,0 +1,11 @@
+import * as React from 'react'
+import Link from 'next/link'
+
+export default function Page() {
+  return (
+    <>
+      <h1>Navigation worked!</h1>
+      <Link href="/">Go back</Link>
+    </>
+  )
+}

test/production/next-link-legacybehavior-ref-merging/app/page.tsx

@@ -0,0 +1,87 @@
+'use client'
+import Link from 'next/link'
+import * as React from 'react'
+import {
+  useCallback,
+  useState,
+  type RefCallback,
+  type Ref,
+  type ComponentPropsWithRef,
+  type ReactNode,
+} from 'react'
+
+export default function Page() {
+  return (
+    <>
+      <h1>Home</h1>
+      <ToggleVisibility>
+        <Link href="/link-target" legacyBehavior>
+          <AnchorThatDoesRefMerging id="test-link">
+            Go to /link-target
+          </AnchorThatDoesRefMerging>
+        </Link>
+      </ToggleVisibility>
+    </>
+  )
+}
+
+function ToggleVisibility({ children }: { children: ReactNode }) {
+  const [isVisible, setIsVisible] = useState(true)
+  return (
+    <>
+      <div>
+        <button type="button" onClick={() => setIsVisible((prev) => !prev)}>
+          {isVisible ? 'Hide content' : 'Show content'}
+        </button>
+      </div>
+      {isVisible ? children : null}
+    </>
+  )
+}
+
+function AnchorThatDoesRefMerging({
+  ref,
+  children,
+  ...anchorProps
+}: ComponentPropsWithRef<'a'>) {
+  const customRef: RefCallback<HTMLAnchorElement> = useCallback((el) => {
+    if (el) {
+      console.log('hello friends i am here')
+    } else {
+      console.log('goodbye friends i am gone')
+    }
+  }, [])
+
+  const finalRef = useBuggyRefMerge(customRef, ref ?? null)
+  return (
+    <a ref={finalRef} {...anchorProps}>
+      {children}
+    </a>
+  )
+}
+
+/** A ref-merging function that doesn't account for cleanup refs (added in React 19)
+ * https://react.dev/blog/2024/12/05/react-19#cleanup-functions-for-refs
+ */
+function useBuggyRefMerge<TElement>(
+  refA: Ref<TElement>,
+  refB: Ref<TElement>
+): RefCallback<TElement> {
+  return useCallback(
+    (current: TElement | null) => {
+      for (const ref of [refA, refB]) {
+        if (!ref) {
+          continue
+        }
+        if (typeof ref === 'object') {
+          ref.current = current
+        } else {
+          // BUG!!!
+          // This would work in 18, but in 19 it can return a cleanup which will get swallowed here
+          ref(current)
+        }
+      }
+    },
+    [refA, refB]
+  )
+}

test/production/next-link-legacybehavior-ref-merging/index.test.ts

@@ -0,0 +1,25 @@
+import { nextTestSetup } from 'e2e-utils'
+
+// NOTE: this test is checking for a bug in prefetching code,
+// so we only enable it in production
+
+describe('Link with legacyBehavior - handles buggy userspace ref merging', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+  it('does not crash when Link unmounts', async () => {
+    const browser = await next.browser('/')
+    expect(await browser.elementByCss('h1').text()).toEqual('Home')
+    expect(await browser.hasElementByCssSelector('#test-link')).toBe(true)
+
+    // hide the link, unmounting it
+    await browser.elementByCss('button').click()
+    expect(await browser.hasElementByCssSelector('#test-link')).toBe(false)
+
+    // shouldn't cause a crash
+    expect(await browser.elementByCss('h1').text()).toEqual('Home')
+    expect(await browser.elementByCss('body').text()).not.toContain(
+      'Application error: a client-side exception has occurred (see the browser console for more information).'
+    )
+  })
+})

test/production/next-link-legacybehavior-ref-merging/next.config.mjs

@@ -0,0 +1,2 @@
+/** @type {import('next').NextConfig} */
+export default {}