fix: be more defensive in useMergedRef #75088

lubieowoce · 2025-01-20T02:22:09Z

Fixes a crash when a Link unmounts (under some VERY specific conditions)
Reverts fix: always ensure element before set to weakmap #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 withnull. 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.

lubieowoce · 2025-01-20T02:29:37Z

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) => {


unrelated to the issue, just a drive-by fix

ijjk · 2025-01-20T02:37:20Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
buildDuration	18.2s	16.3s	N/A
buildDurationCached	15.5s	13s	N/A
nodeModulesSize	418 MB	418 MB	⚠️ +5.83 kB
nextStartRea..uration (ms)	427ms	407ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
5306-HASH.js gzip	54 kB	54 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.44 kB	5.46 kB	N/A
bccd1874-HASH.js gzip	52.9 kB	52.9 kB	✓
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	238 B	243 B	N/A
main-HASH.js gzip	34.4 kB	34.4 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	52.9 kB	52.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.57 kB	4.58 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.35 kB	✓
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	5.94 kB	5.94 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
_buildManifest.js gzip	749 B	747 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
index.html gzip	524 B	524 B	✓
link.html gzip	539 B	538 B	N/A
withRouter.html gzip	520 B	520 B	✓
Overall change	1.04 kB	1.04 kB	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
edge-ssr.js gzip	129 kB	129 kB	N/A
page.js gzip	208 kB	208 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
middleware-b..fest.js gzip	671 B	666 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.3 kB	31.3 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
274-experime...dev.js gzip	322 B	322 B	✓
274.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	375 kB	375 kB	✓
app-page-exp..prod.js gzip	130 kB	130 kB	✓
app-page-tur..prod.js gzip	143 kB	143 kB	✓
app-page-tur..prod.js gzip	139 kB	139 kB	✓
app-page.run...dev.js gzip	363 kB	363 kB	✓
app-page.run..prod.js gzip	126 kB	126 kB	✓
app-route-ex...dev.js gzip	37.6 kB	37.6 kB	✓
app-route-ex..prod.js gzip	25.6 kB	25.6 kB	✓
app-route-tu..prod.js gzip	25.6 kB	25.6 kB	✓
app-route-tu..prod.js gzip	25.4 kB	25.4 kB	✓
app-route.ru...dev.js gzip	39.2 kB	39.2 kB	✓
app-route.ru..prod.js gzip	25.4 kB	25.4 kB	✓
pages-api-tu..prod.js gzip	9.69 kB	9.69 kB	✓
pages-api.ru...dev.js gzip	11.6 kB	11.6 kB	✓
pages-api.ru..prod.js gzip	9.68 kB	9.68 kB	✓
pages-turbo...prod.js gzip	21.9 kB	21.9 kB	✓
pages.runtim...dev.js gzip	27.7 kB	27.7 kB	✓
pages.runtim..prod.js gzip	21.9 kB	21.9 kB	✓
server.runti..prod.js gzip	916 kB	916 kB	✓
Overall change	2.47 MB	2.47 MB	✓

build cache Overall increase ⚠️

	vercel/next.js canary	vercel/next.js lubieowoce/fix-link-ref-crash	Change
0.pack gzip	2.09 MB	2.1 MB	⚠️ +3.39 kB
index.pack gzip	75.1 kB	75.2 kB	N/A
Overall change	2.09 MB	2.1 MB	⚠️ +3.39 kB

Diff details

Diff for image-HASH.js

@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [2983],
   {
-    /***/ 3705: /***/ (
+    /***/ 8255: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(8448);
+          return __webpack_require__(8926);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 7342: /***/ (module, exports, __webpack_require__) => {
+    /***/ 4369: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,17 +40,17 @@
         __webpack_require__(6049)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(7196)
+        __webpack_require__(956)
       );
-      const _getimgprops = __webpack_require__(2661);
-      const _imageconfig = __webpack_require__(72);
-      const _imageconfigcontextsharedruntime = __webpack_require__(6386);
-      const _warnonce = __webpack_require__(4496);
-      const _routercontextsharedruntime = __webpack_require__(6443);
+      const _getimgprops = __webpack_require__(485);
+      const _imageconfig = __webpack_require__(4664);
+      const _imageconfigcontextsharedruntime = __webpack_require__(1250);
+      const _warnonce = __webpack_require__(1648);
+      const _routercontextsharedruntime = __webpack_require__(907);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(3433)
+        __webpack_require__(6489)
       );
-      const _usemergedref = __webpack_require__(1942);
+      const _usemergedref = __webpack_require__(5942);
       // This is replaced by webpack define plugin
       const configEnv = {
         deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
@@ -377,7 +377,7 @@
       /***/
     },
 
-    /***/ 1942: /***/ (module, exports, __webpack_require__) => {
+    /***/ 5942: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -391,22 +391,39 @@
       });
       const _react = __webpack_require__(8101);
       function useMergedRef(refA, refB) {
-        const cleanupA = (0, _react.useRef)(() => {});
-        const cleanupB = (0, _react.useRef)(() => {});
-        return (0, _react.useMemo)(() => {
-          if (!refA || !refB) {
-            return refA || refB;
-          }
-          return (current) => {
+        const cleanupA = (0, _react.useRef)(null);
+        const cleanupB = (0, _react.useRef)(null);
+        // 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 (0, _react.useCallback)(
+          (current) => {
             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(refA, current) {
         if (typeof refA === "function") {
@@ -438,7 +455,7 @@
       /***/
     },
 
-    /***/ 2661: /***/ (
+    /***/ 485: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -454,9 +471,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(4496);
-      const _imageblursvg = __webpack_require__(4796);
-      const _imageconfig = __webpack_require__(72);
+      const _warnonce = __webpack_require__(1648);
+      const _imageblursvg = __webpack_require__(4812);
+      const _imageconfig = __webpack_require__(4664);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -864,7 +881,7 @@
       /***/
     },
 
-    /***/ 4796: /***/ (__unused_webpack_module, exports) => {
+    /***/ 4812: /***/ (__unused_webpack_module, exports) => {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -919,7 +936,7 @@
       /***/
     },
 
-    /***/ 1969: /***/ (
+    /***/ 8273: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -946,10 +963,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(173);
-      const _getimgprops = __webpack_require__(2661);
-      const _imagecomponent = __webpack_require__(7342);
+      const _getimgprops = __webpack_require__(485);
+      const _imagecomponent = __webpack_require__(4369);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(3433)
+        __webpack_require__(6489)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -981,7 +998,7 @@
       /***/
     },
 
-    /***/ 3433: /***/ (__unused_webpack_module, exports) => {
+    /***/ 6489: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -1028,7 +1045,7 @@
       /***/
     },
 
-    /***/ 8448: /***/ (
+    /***/ 8926: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -1045,8 +1062,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@19.0.0/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(5105);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+main-repo+packages+next+next-packed.tgz_react-dom@19.0.0_react@19.0.0__react@19.0.0/node_modules/next/image.js
-      var next_image = __webpack_require__(8140);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+diff-repo+packages+next+next-packed.tgz_react-dom@19.0.0_react@19.0.0__react@19.0.0/node_modules/next/image.js
+      var next_image = __webpack_require__(5434);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // ./pages/nextjs.png
       /* harmony default export */ const nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
@@ -1076,12 +1093,12 @@
       /***/
     },
 
-    /***/ 8140: /***/ (
+    /***/ 5434: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(1969);
+      module.exports = __webpack_require__(8273);
 
       /***/
     },
@@ -1091,7 +1108,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [636, 6593, 8792], () =>
-      __webpack_exec__(3705)
+      __webpack_exec__(8255)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;

Diff for link-HASH.js

@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [4672],
   {
-    /***/ 3679: /***/ (
+    /***/ 6613: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/link",
         function () {
-          return __webpack_require__(8562);
+          return __webpack_require__(2488);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 7597: /***/ (module, exports, __webpack_require__) => {
+    /***/ 58: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -30,7 +30,7 @@
           return getDomainLocale;
         },
       });
-      const _normalizetrailingslash = __webpack_require__(4858);
+      const _normalizetrailingslash = __webpack_require__(8650);
       const basePath =
         /* unused pure expression or super */ null && (false || "");
       function getDomainLocale(path, locale, locales, domainLocales) {
@@ -54,7 +54,7 @@
       /***/
     },
 
-    /***/ 3000: /***/ (module, exports, __webpack_require__) => {
+    /***/ 9400: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -71,16 +71,16 @@
       const _react = /*#__PURE__*/ _interop_require_default._(
         __webpack_require__(8101)
       );
-      const _resolvehref = __webpack_require__(9978);
-      const _islocalurl = __webpack_require__(1762);
-      const _formaturl = __webpack_require__(9835);
-      const _utils = __webpack_require__(4781);
-      const _addlocale = __webpack_require__(5244);
-      const _routercontextsharedruntime = __webpack_require__(6443);
-      const _useintersection = __webpack_require__(1587);
-      const _getdomainlocale = __webpack_require__(7597);
-      const _addbasepath = __webpack_require__(7439);
-      const _usemergedref = __webpack_require__(1942);
+      const _resolvehref = __webpack_require__(5066);
+      const _islocalurl = __webpack_require__(5266);
+      const _formaturl = __webpack_require__(4507);
+      const _utils = __webpack_require__(3085);
+      const _addlocale = __webpack_require__(8172);
+      const _routercontextsharedruntime = __webpack_require__(907);
+      const _useintersection = __webpack_require__(9347);
+      const _getdomainlocale = __webpack_require__(58);
+      const _addbasepath = __webpack_require__(943);
+      const _usemergedref = __webpack_require__(5942);
       const prefetched = new Set();
       function prefetch(router, href, as, options) {
         if (false) {
@@ -433,7 +433,7 @@
       /***/
     },
 
-    /***/ 1587: /***/ (module, exports, __webpack_require__) => {
+    /***/ 9347: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -446,7 +446,7 @@
         },
       });
       const _react = __webpack_require__(8101);
-      const _requestidlecallback = __webpack_require__(7512);
+      const _requestidlecallback = __webpack_require__(5832);
       const hasIntersectionObserver =
         typeof IntersectionObserver === "function";
       const observers = new Map();
@@ -559,7 +559,7 @@
       /***/
     },
 
-    /***/ 1942: /***/ (module, exports, __webpack_require__) => {
+    /***/ 5942: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -573,22 +573,39 @@
       });
       const _react = __webpack_require__(8101);
       function useMergedRef(refA, refB) {
-        const cleanupA = (0, _react.useRef)(() => {});
-        const cleanupB = (0, _react.useRef)(() => {});
-        return (0, _react.useMemo)(() => {
-          if (!refA || !refB) {
-            return refA || refB;
-          }
-          return (current) => {
+        const cleanupA = (0, _react.useRef)(null);
+        const cleanupB = (0, _react.useRef)(null);
+        // 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 (0, _react.useCallback)(
+          (current) => {
             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(refA, current) {
         if (typeof refA === "function") {
@@ -620,7 +637,7 @@
       /***/
     },
 
-    /***/ 8562: /***/ (
+    /***/ 2488: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -635,7 +652,7 @@
       /* harmony import */ var react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__ =
         __webpack_require__(5105);
       /* harmony import */ var next_link__WEBPACK_IMPORTED_MODULE_1__ =
-        __webpack_require__(4101);
+        __webpack_require__(4779);
       /* harmony import */ var next_link__WEBPACK_IMPORTED_MODULE_1___default =
         /*#__PURE__*/ __webpack_require__.n(
           next_link__WEBPACK_IMPORTED_MODULE_1__
@@ -666,12 +683,12 @@
       /***/
     },
 
-    /***/ 4101: /***/ (
+    /***/ 4779: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(3000);
+      module.exports = __webpack_require__(9400);
 
       /***/
     },
@@ -681,7 +698,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [636, 6593, 8792], () =>
-      __webpack_exec__(3679)
+      __webpack_exec__(6613)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;

Diff for 8377-HASH.js

@@ -1,8 +1,8 @@
 "use strict";
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
-  [8377],
+  [2727],
   {
-    /***/ 8377: /***/ (module, exports, __webpack_require__) => {
+    /***/ 2727: /***/ (module, exports, __webpack_require__) => {
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
         value: true,
@@ -13,27 +13,27 @@
           return Image;
         },
       });
-      const _interop_require_default = __webpack_require__(2952);
-      const _interop_require_wildcard = __webpack_require__(333);
-      const _jsxruntime = __webpack_require__(3094);
+      const _interop_require_default = __webpack_require__(384);
+      const _interop_require_wildcard = __webpack_require__(4261);
+      const _jsxruntime = __webpack_require__(7048);
       const _react = /*#__PURE__*/ _interop_require_wildcard._(
-        __webpack_require__(1446)
+        __webpack_require__(228)
       );
       const _reactdom = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(8307)
+        __webpack_require__(9221)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(8050)
+        __webpack_require__(1116)
       );
-      const _getimgprops = __webpack_require__(3201);
-      const _imageconfig = __webpack_require__(6678);
-      const _imageconfigcontextsharedruntime = __webpack_require__(578);
-      const _warnonce = __webpack_require__(1971);
-      const _routercontextsharedruntime = __webpack_require__(7795);
+      const _getimgprops = __webpack_require__(5763);
+      const _imageconfig = __webpack_require__(6224);
+      const _imageconfigcontextsharedruntime = __webpack_require__(6720);
+      const _warnonce = __webpack_require__(894);
+      const _routercontextsharedruntime = __webpack_require__(9093);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(1315)
+        __webpack_require__(2809)
       );
-      const _usemergedref = __webpack_require__(592);
+      const _usemergedref = __webpack_require__(1329);
       // This is replaced by webpack define plugin
       const configEnv = {
         deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
@@ -361,7 +361,7 @@
       /***/
     },
 
-    /***/ 592: /***/ (module, exports, __webpack_require__) => {
+    /***/ 1329: /***/ (module, exports, __webpack_require__) => {
       Object.defineProperty(exports, "__esModule", {
         value: true,
       });
@@ -371,24 +371,41 @@
           return useMergedRef;
         },
       });
-      const _react = __webpack_require__(1446);
+      const _react = __webpack_require__(228);
       function useMergedRef(refA, refB) {
-        const cleanupA = (0, _react.useRef)(() => {});
-        const cleanupB = (0, _react.useRef)(() => {});
-        return (0, _react.useMemo)(() => {
-          if (!refA || !refB) {
-            return refA || refB;
-          }
-          return (current) => {
+        const cleanupA = (0, _react.useRef)(null);
+        const cleanupB = (0, _react.useRef)(null);
+        // 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 (0, _react.useCallback)(
+          (current) => {
             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(refA, current) {
         if (typeof refA === "function") {
@@ -420,7 +437,7 @@
       /***/
     },
 
-    /***/ 5318: /***/ (
+    /***/ 272: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -434,9 +451,9 @@
           return AmpStateContext;
         },
       });
-      const _interop_require_default = __webpack_require__(2952);
+      const _interop_require_default = __webpack_require__(384);
       const _react = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(1446)
+        __webpack_require__(228)
       );
       const AmpStateContext = _react.default.createContext({});
       if (false) {
@@ -445,7 +462,7 @@
       /***/
     },
 
-    /***/ 1210: /***/ (__unused_webpack_module, exports) => {
+    /***/ 1467: /***/ (__unused_webpack_module, exports) => {
       Object.defineProperty(exports, "__esModule", {
         value: true,
       });
@@ -467,7 +484,7 @@
       /***/
     },
 
-    /***/ 3201: /***/ (
+    /***/ 5763: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -481,9 +498,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(1971);
-      const _imageblursvg = __webpack_require__(3482);
-      const _imageconfig = __webpack_require__(6678);
+      const _warnonce = __webpack_require__(894);
+      const _imageblursvg = __webpack_require__(5868);
+      const _imageconfig = __webpack_require__(6224);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -891,8 +908,8 @@
       /***/
     },
 
-    /***/ 8050: /***/ (module, exports, __webpack_require__) => {
-      /* provided dependency */ var process = __webpack_require__(6611);
+    /***/ 1116: /***/ (module, exports, __webpack_require__) => {
+      /* provided dependency */ var process = __webpack_require__(9829);
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
         value: true,
@@ -913,19 +930,19 @@
           return defaultHead;
         },
       });
-      const _interop_require_default = __webpack_require__(2952);
-      const _interop_require_wildcard = __webpack_require__(333);
-      const _jsxruntime = __webpack_require__(3094);
+      const _interop_require_default = __webpack_require__(384);
+      const _interop_require_wildcard = __webpack_require__(4261);
+      const _jsxruntime = __webpack_require__(7048);
       const _react = /*#__PURE__*/ _interop_require_wildcard._(
-        __webpack_require__(1446)
+        __webpack_require__(228)
       );
       const _sideeffect = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(4607)
+        __webpack_require__(4101)
       );
-      const _ampcontextsharedruntime = __webpack_require__(5318);
-      const _headmanagercontextsharedruntime = __webpack_require__(7060);
-      const _ampmode = __webpack_require__(1210);
-      const _warnonce = __webpack_require__(1971);
+      const _ampcontextsharedruntime = __webpack_require__(272);
+      const _headmanagercontextsharedruntime = __webpack_require__(1790);
+      const _ampmode = __webpack_require__(1467);
+      const _warnonce = __webpack_require__(894);
       function defaultHead(inAmpMode) {
         if (inAmpMode === void 0) inAmpMode = false;
         const head = [
@@ -1109,7 +1126,7 @@
       /***/
     },
 
-    /***/ 3482: /***/ (__unused_webpack_module, exports) => {
+    /***/ 5868: /***/ (__unused_webpack_module, exports) => {
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
        */
@@ -1163,7 +1180,7 @@
       /***/
     },
 
-    /***/ 578: /***/ (
+    /***/ 6720: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -1177,11 +1194,11 @@
           return ImageConfigContext;
         },
       });
-      const _interop_require_default = __webpack_require__(2952);
+      const _interop_require_default = __webpack_require__(384);
       const _react = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(1446)
+        __webpack_require__(228)
       );
-      const _imageconfig = __webpack_require__(6678);
+      const _imageconfig = __webpack_require__(6224);
       const ImageConfigContext = _react.default.createContext(
         _imageconfig.imageConfigDefault
       );
@@ -1191,7 +1208,7 @@
       /***/
     },
 
-    /***/ 6678: /***/ (__unused_webpack_module, exports) => {
+    /***/ 6224: /***/ (__unused_webpack_module, exports) => {
       Object.defineProperty(exports, "__esModule", {
         value: true,
       });
@@ -1240,7 +1257,7 @@
       /***/
     },
 
-    /***/ 1315: /***/ (__unused_webpack_module, exports) => {
+    /***/ 2809: /***/ (__unused_webpack_module, exports) => {
       Object.defineProperty(exports, "__esModule", {
         value: true,
       });
@@ -1285,7 +1302,7 @@
       /***/
     },
 
-    /***/ 7795: /***/ (
+    /***/ 9093: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -1299,9 +1316,9 @@
           return RouterContext;
         },
       });
-      const _interop_require_default = __webpack_require__(2952);
+      const _interop_require_default = __webpack_require__(384);
       const _react = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(1446)
+        __webpack_require__(228)
       );
       const RouterContext = _react.default.createContext(null);
       if (false) {
@@ -1310,7 +1327,7 @@
       /***/
     },
 
-    /***/ 4607: /***/ (
+    /***/ 4101: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -1324,7 +1341,7 @@
           return SideEffect;
         },
       });
-      const _react = __webpack_require__(1446);
+      const _react = __webpack_require__(228);
       const isServer = typeof window === "undefined";
       const useClientOnlyLayoutEffect = isServer
         ? () => {}

Diff for main-HASH.js

Diff too large to display

Commit: 7bf5d12

ijjk · 2025-01-20T02:39:02Z

Tests Passed

lubieowoce · 2025-01-20T15:54:13Z

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

-  return useMemo(() => {
-    if (!refA || !refB) {
-      return refA || refB


the removal of this fast path is the real change here -- this is what was causing us to return observeLinkVisibilityOnMount as-is. the rest just comes as a consequence of allowing null refs in the wrapper below

This reverts commit 7229934.

lubieowoce · 2025-01-20T16:36:43Z

An alternative approach would be to leave the useMergedRef fast-path in, and just avoid using cleanup-refs in our code. So we'd change observeLinkVisibilityOnMount to be old-style (possibly via a converter util, that'd probably be cleanest).

But i don't like that option very much, because then there's nothing guarding us against reintroducing a similar bug elsewhere. And the overhead of wrapping seems pretty negligible anyway

ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Jan 20, 2025

lubieowoce force-pushed the lubieowoce/fix-link-ref-crash branch from 6eb16d7 to d104439 Compare January 20, 2025 02:23

lubieowoce commented Jan 20, 2025

View reviewed changes

lubieowoce force-pushed the lubieowoce/fix-link-ref-crash branch from 58868f0 to 5916cb4 Compare January 20, 2025 15:53

ijjk added the tests label Jan 20, 2025

lubieowoce commented Jan 20, 2025

View reviewed changes

lubieowoce requested review from eps1lon and acdlite January 20, 2025 16:08

lubieowoce changed the title ~~fix: don't skip back-compat-wrapping in useMergedRef~~ fix: be more defensive in useMergedRef Jan 20, 2025

lubieowoce marked this pull request as ready for review January 20, 2025 16:09

lubieowoce added 2 commits January 20, 2025 17:10

Revert "fix: always ensure element before set to weakmap (#75012)"

5daba1c

This reverts commit 7229934.

test: reproduce link crash

274d918

lubieowoce force-pushed the lubieowoce/fix-link-ref-crash branch 2 times, most recently from 5a0ca8f to 4125ebe Compare January 20, 2025 16:15

lubieowoce requested a review from huozhi January 20, 2025 16:16

lubieowoce force-pushed the lubieowoce/fix-link-ref-crash branch from 4125ebe to 7dc9500 Compare January 20, 2025 16:21

lubieowoce added 2 commits January 20, 2025 17:23

fix useMergedRef

87b10a6

random type fix

7bf5d12

lubieowoce force-pushed the lubieowoce/fix-link-ref-crash branch from 7dc9500 to 7bf5d12 Compare January 20, 2025 16:23

huozhi approved these changes Jan 21, 2025

View reviewed changes

lubieowoce merged commit 32bc355 into canary Jan 21, 2025
135 checks passed

lubieowoce deleted the lubieowoce/fix-link-ref-crash branch January 21, 2025 15:14

github-actions bot added the locked label Feb 5, 2025

github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: be more defensive in useMergedRef #75088

fix: be more defensive in useMergedRef #75088

lubieowoce commented Jan 20, 2025 •

edited

Loading

lubieowoce Jan 20, 2025

ijjk commented Jan 20, 2025 •

edited

Loading

ijjk commented Jan 20, 2025 •

edited

Loading

lubieowoce Jan 20, 2025 •

edited

Loading

lubieowoce commented Jan 20, 2025 •

edited

Loading

fix: be more defensive in useMergedRef #75088

fix: be more defensive in useMergedRef #75088

Conversation

lubieowoce commented Jan 20, 2025 • edited Loading

Short explanation

Full explanation

lubieowoce Jan 20, 2025

Choose a reason for hiding this comment

ijjk commented Jan 20, 2025 • edited Loading

Stats from current PR

ijjk commented Jan 20, 2025 • edited Loading

Tests Passed

lubieowoce Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

lubieowoce commented Jan 20, 2025 • edited Loading

lubieowoce commented Jan 20, 2025 •

edited

Loading

ijjk commented Jan 20, 2025 •

edited

Loading

ijjk commented Jan 20, 2025 •

edited

Loading

lubieowoce Jan 20, 2025 •

edited

Loading

lubieowoce commented Jan 20, 2025 •

edited

Loading