misc: tweak fetch patch restoration timing during HMR to allow for userland fetch patching

[feedthejim]

This PR

In this PR, I update the fetch patching logic that restores the original fetch during development after a request to only happen when a server components gets actually reloaded.

This also makes the fetch patching check more lax so that other users can patch fetch if they want to (seems fair 💀).

Context

• In memory: fix 2 memory leaks in next-dev #43859, I added some logic to restore the original fetch after handling a request during development. I did this because when a server component gets modified in dev, we nuke the require cache to be able to re-instantiate the newly modified component in a fresh environment (temu fast refresh). The problem I found at the time was that we would keep applying the fetch patch on top of the other, leading to the memory of the original fetch environment being retained by the newly patched fetch, which led to huge memory problems.
• MSW is a library that helps people easily mock network requests via patching fetch on the server or using a server worker in the browser

Why

I was investigating how we could make MSW work better with @kettanaito and I wrote a quick example of initialising MSW at the top level of a layout in order to intercept fetch calls.

Nothing complicated, just this:

if (process.env.NEXT_RUNTIME === 'nodejs') {
  const { server } = require('../mocks/node')
  server.listen()
}

Turns out, this breaks on a second page load because the MSW patching gets undone, even though we have not modified the file itself, which is slightly problematic.

I fixed this by changing the timing, but I also realised testing it that any fetch patch that remained would still get overridden by a check in patch-fetch.ts where we verify via a special property that we patched fetch correctly. If you patch fetch on top of that, it breaks because on the second render, we'd patch again.

As a result, I also changed the isPatched check to be a global check that should only be invalidated during dev HMR

test plan

• wrote a simple test that checks that patching is doable
• verified manually that the fetch still does not leak

[ijjk]

Tests Passed

[ijjk]

Stats from current PR

Default Build (Increase detected ⚠️)

General

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
buildDuration	16.9s	18.1s	⚠️ +1.3s
buildDurationCached	8.6s	7.2s	N/A
nodeModulesSize	356 MB	356 MB	N/A
nextStartRea..uration (ms)	424ms	405ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
4958.HASH.js gzip	169 B	168 B	N/A
6405-HASH.js gzip	5.19 kB	5.19 kB	✓
8711-HASH.js gzip	37.9 kB	37.9 kB	N/A
d09468e9-HASH.js gzip	51.9 kB	51.9 kB	N/A
framework-HASH.js gzip	56.8 kB	56.7 kB	N/A
main-app-HASH.js gzip	224 B	225 B	N/A
main-HASH.js gzip	32.5 kB	32.5 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	5.19 kB	5.19 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
_app-HASH.js gzip	194 B	192 B	N/A
_error-HASH.js gzip	191 B	192 B	N/A
amp-HASH.js gzip	512 B	511 B	N/A
css-HASH.js gzip	342 B	344 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	N/A
edge-ssr-HASH.js gzip	265 B	266 B	N/A
head-HASH.js gzip	361 B	364 B	N/A
hooks-HASH.js gzip	390 B	393 B	N/A
image-HASH.js gzip	4.4 kB	4.4 kB	N/A
index-HASH.js gzip	268 B	267 B	N/A
link-HASH.js gzip	2.82 kB	2.81 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	324 B	324 B	✓
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	1.16 kB	1.16 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
_buildManifest.js gzip	747 B	749 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
index.html gzip	523 B	521 B	N/A
link.html gzip	538 B	538 B	✓
withRouter.html gzip	519 B	520 B	N/A
Overall change	538 B	538 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
edge-ssr.js gzip	127 kB	127 kB	N/A
page.js gzip	173 kB	173 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
middleware-b..fest.js gzip	669 B	669 B	✓
middleware-r..fest.js gzip	156 B	156 B	✓
middleware.js gzip	29.7 kB	29.6 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	1.67 kB	1.67 kB	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
928-experime...dev.js gzip	322 B	322 B	✓
928.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	309 kB	307 kB	N/A
app-page-exp..prod.js gzip	122 kB	121 kB	N/A
app-page-tur..prod.js gzip	135 kB	133 kB	N/A
app-page-tur..prod.js gzip	130 kB	129 kB	N/A
app-page.run...dev.js gzip	298 kB	297 kB	N/A
app-page.run..prod.js gzip	118 kB	117 kB	N/A
app-route-ex...dev.js gzip	30.7 kB	30.7 kB	N/A
app-route-ex..prod.js gzip	20.7 kB	20.7 kB	N/A
app-route-tu..prod.js gzip	20.7 kB	20.7 kB	N/A
app-route-tu..prod.js gzip	20.5 kB	20.5 kB	N/A
app-route.ru...dev.js gzip	32.3 kB	32.3 kB	N/A
app-route.ru..prod.js gzip	20.5 kB	20.5 kB	N/A
pages-api-tu..prod.js gzip	9.62 kB	9.6 kB	N/A
pages-api.ru...dev.js gzip	11.5 kB	11.4 kB	N/A
pages-api.ru..prod.js gzip	9.61 kB	9.59 kB	N/A
pages-turbo...prod.js gzip	21.6 kB	21.6 kB	N/A
pages.runtim...dev.js gzip	27.5 kB	27.5 kB	N/A
pages.runtim..prod.js gzip	21.6 kB	21.6 kB	N/A
server.runti..prod.js gzip	56.8 kB	56.9 kB	N/A
Overall change	636 B	636 B	✓

build cache Overall increase ⚠️

	vercel/next.js canary	vercel/next.js feedthejim/fix-fetch-patch-hmr	Change
0.pack gzip	1.49 MB	1.49 MB	⚠️ +767 B
index.pack gzip	127 kB	126 kB	N/A
Overall change	1.49 MB	1.49 MB	⚠️ +767 B

Diff details

Diff for page.js

Diff too large to display

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for image-HASH.js

@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [8358],
   {
-    /***/ 9842: /***/ (
+    /***/ 5684: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(8722);
+          return __webpack_require__(2665);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 5040: /***/ (module, exports, __webpack_require__) => {
+    /***/ 3608: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,17 +40,17 @@
         __webpack_require__(2957)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(5187)
+        __webpack_require__(7569)
       );
-      const _getimgprops = __webpack_require__(1332);
-      const _imageconfig = __webpack_require__(3384);
-      const _imageconfigcontextsharedruntime = __webpack_require__(8875);
-      const _warnonce = __webpack_require__(4516);
-      const _routercontextsharedruntime = __webpack_require__(5002);
+      const _getimgprops = __webpack_require__(4429);
+      const _imageconfig = __webpack_require__(2950);
+      const _imageconfigcontextsharedruntime = __webpack_require__(6587);
+      const _warnonce = __webpack_require__(8719);
+      const _routercontextsharedruntime = __webpack_require__(2062);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(5145)
+        __webpack_require__(232)
       );
-      const _usemergedref = __webpack_require__(4926);
+      const _usemergedref = __webpack_require__(9935);
       // This is replaced by webpack define plugin
       const configEnv = {
         deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
@@ -371,7 +371,7 @@
       /***/
     },
 
-    /***/ 4926: /***/ (module, exports, __webpack_require__) => {
+    /***/ 9935: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -440,7 +440,7 @@
       /***/
     },
 
-    /***/ 1332: /***/ (
+    /***/ 4429: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -456,9 +456,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(4516);
-      const _imageblursvg = __webpack_require__(1569);
-      const _imageconfig = __webpack_require__(3384);
+      const _warnonce = __webpack_require__(8719);
+      const _imageblursvg = __webpack_require__(3207);
+      const _imageconfig = __webpack_require__(2950);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -830,7 +830,7 @@
       /***/
     },
 
-    /***/ 1569: /***/ (__unused_webpack_module, exports) => {
+    /***/ 3207: /***/ (__unused_webpack_module, exports) => {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -885,7 +885,7 @@
       /***/
     },
 
-    /***/ 1674: /***/ (
+    /***/ 3455: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -912,10 +912,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(4345);
-      const _getimgprops = __webpack_require__(1332);
-      const _imagecomponent = __webpack_require__(5040);
+      const _getimgprops = __webpack_require__(4429);
+      const _imagecomponent = __webpack_require__(3608);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(5145)
+        __webpack_require__(232)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -947,7 +947,7 @@
       /***/
     },
 
-    /***/ 5145: /***/ (__unused_webpack_module, exports) => {
+    /***/ 232: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -982,7 +982,7 @@
       /***/
     },
 
-    /***/ 8722: /***/ (
+    /***/ 2665: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -999,8 +999,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@19.0.0-rc-187dd6a7-20240806/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(8548);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+main-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-187dd6a7-20240806_re_6tugpsz4gl2xsqrwds5bwcbi3y/node_modules/next/image.js
-      var next_image = __webpack_require__(6629);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+diff-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-187dd6a7-20240806_re_wxf3je55uhqnotjiz3wkaonabu/node_modules/next/image.js
+      var next_image = __webpack_require__(2856);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // CONCATENATED MODULE: ./pages/nextjs.png
       /* harmony default export */ const nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
@@ -1030,12 +1030,12 @@
       /***/
     },
 
-    /***/ 6629: /***/ (
+    /***/ 2856: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(1674);
+      module.exports = __webpack_require__(3455);
 
       /***/
     },
@@ -1045,7 +1045,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [2888, 9774, 179], () =>
-      __webpack_exec__(9842)
+      __webpack_exec__(5684)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;

Diff for 8711-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

failed to diff

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

failed to diff

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-api-tu..time.prod.js

Diff too large to display

Diff for pages-api.runtime.dev.js

Diff too large to display

Diff for pages-api.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: 6f3b8f7

[unstubbable]

• misc: tweak fetch patch restoration timing during HMR to allow for userland fetch patching #68193 👈
• Remove obsolete deleteAppClientCache() call from webpack plugin #68535 : 1 other dependent PR (#68743 )
• canary

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @feedthejim and the rest of your teammates on Graphite

[eps1lon]

How do we know the HMR update is applied from this assertion? Both expect(magicNumber).not.toEqual(magicNumber3) and expect(magicNumber).toBe(magicNumber2) can be true at the same time. So just from the assertions, I don't see how this verifies that anything changed. Shouldn't we assert that "touch to trigger HMR 2" is now part of the rendered HTML?

[eps1lon]

Shouldn't we assert that "touch to trigger HMR 2" is now part of the rendered HTML?

Doing that now. expect(magicNumber).not.toEqual(magicNumber3) was testing if the module was actually re-evaluated. Leaving that in since it's an interesting property to check for (module state is reset).

[kettanaito]

Update

I've got a chance to try out these changes, and I confirm that they fix two issues:

• Server-side HMR (the fetch patch now persists correctly).
• Issue when the app crashed on refresh due to the fetch being unpatched on incremental server-side requests.

🎉

next-02.mov

The issue with the client-side HMR not taking effect is still there, but this change was never meant to affect that.

Awesome work, @feedthejim 👍 Can we get any ETA on when this can be merged?

[kettanaito]

I filed the client-side bug in #69098. It doesn't actually swallow the HMR as I initially thought. The event listener for the UI element that does a fetch request on click gets applied more times than necessary during the component's re-rendering during HMR.

[eps1lon]

@kettanaito Thank you for confirming. This should be good to go

[kettanaito]

Incredible work! Thank you, everyone 👏