misc: tweak fetch patch restoration timing during HMR to allow for userland fetch patching (#68193)

Author: feedthejim

packages/next/src/server/dev/hot-reloader-turbopack.ts

@@ -92,7 +92,8 @@ const sessionId = Math.floor(Number.MAX_SAFE_INTEGER * Math.random())
 export async function createHotReloaderTurbopack(
   opts: SetupOpts,
   serverFields: ServerFields,
-  distDir: string
+  distDir: string,
+  resetFetch: () => void
 ): Promise<NextJsHotReloaderInterface> {
   const buildId = 'development'
   const { nextConfig, dir } = opts
@@ -236,6 +237,8 @@ export async function createHotReloaderTurbopack(
       }
     }
 
+    resetFetch()
+
     const hasAppPaths = writtenEndpoint.serverPaths.some(({ path: p }) =>
       p.startsWith('server/app')
     )

packages/next/src/server/dev/hot-reloader-webpack.ts

@@ -249,6 +249,7 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
   private pagesMapping: { [key: string]: string } = {}
   private appDir?: string
   private telemetry: Telemetry
+  private resetFetch: () => void
   private versionInfo: VersionInfo = {
     staleness: 'unknown',
     installed: '0.0.0',
@@ -274,6 +275,7 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
       rewrites,
       appDir,
       telemetry,
+      resetFetch,
     }: {
       config: NextConfigComplete
       pagesDir?: string
@@ -284,6 +286,7 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
       rewrites: CustomRoutes['rewrites']
       appDir?: string
       telemetry: Telemetry
+      resetFetch: () => void
     }
   ) {
     this.hasAmpEntrypoints = false
@@ -301,6 +304,7 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
     this.edgeServerStats = null
     this.serverPrevDocumentHash = null
     this.telemetry = telemetry
+    this.resetFetch = resetFetch
 
     this.config = config
     this.previewProps = previewProps
@@ -1365,6 +1369,7 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface {
         changedCSSImportPages.size ||
         reloadAfterInvalidation
       ) {
+        this.resetFetch()
         this.refreshServerComponents()
       }
 

packages/next/src/server/dev/next-dev-server.ts

@@ -162,7 +162,6 @@ export default class DevServer extends Server {
     this.bundlerService = options.bundlerService
     this.startServerSpan =
       options.startServerSpan ?? trace('start-next-dev-server')
-    this.storeGlobals()
     this.renderOpts.dev = true
     this.renderOpts.ErrorDebug = ReactDevOverlay
     this.staticPathsCache = new LRUCache({
@@ -294,9 +293,6 @@ export default class DevServer extends Server {
     await super.prepareImpl()
     await this.matchers.reload()
 
-    // Store globals again to preserve changes made by the instrumentation hook.
-    this.storeGlobals()
-
     this.ready?.resolve()
     this.ready = undefined
 
@@ -825,14 +821,6 @@ export default class DevServer extends Server {
     return nextInvoke as NonNullable<typeof result>
   }
 
-  private storeGlobals(): void {
-    this.originalFetch = global.fetch
-  }
-
-  private restorePatchedGlobals(): void {
-    global.fetch = this.originalFetch ?? global.fetch
-  }
-
   protected async ensurePage(opts: {
     page: string
     clientOnly: boolean
@@ -880,11 +868,6 @@ export default class DevServer extends Server {
       }
 
       this.nextFontManifest = super.getNextFontManifest()
-      // before we re-evaluate a route module, we want to restore globals that might
-      // have been patched previously to their original state so that we don't
-      // patch on top of the previous patch, which would keep the context of the previous
-      // patched global in memory, creating a memory leak.
-      this.restorePatchedGlobals()
 
       return await super.findPageComponents({
         page,

packages/next/src/server/lib/patch-fetch.ts

@@ -34,10 +34,10 @@ type PatchedFetcher = Fetcher & {
   readonly _nextOriginalFetch: Fetcher
 }
 
-function isPatchedFetch(
-  fetch: Fetcher | PatchedFetcher
-): fetch is PatchedFetcher {
-  return '__nextPatched' in fetch && fetch.__nextPatched === true
+export const NEXT_PATCH_SYMBOL = Symbol.for('next-patch')
+
+function isFetchPatched() {
+  return (globalThis as Record<symbol, unknown>)[NEXT_PATCH_SYMBOL] === true
 }
 
 export function validateRevalidate(
@@ -804,18 +804,21 @@ export function createPatchedFetcher(
   }
 
   // Attach the necessary properties to the patched fetch function.
+  // We don't use this to determine if the fetch function has been patched,
+  // but for external consumers to determine if the fetch function has been
+  // patched.
   patched.__nextPatched = true as const
   patched.__nextGetStaticStore = () => staticGenerationAsyncStorage
   patched._nextOriginalFetch = originFetch
+  ;(globalThis as Record<symbol, unknown>)[NEXT_PATCH_SYMBOL] = true
 
   return patched
 }
-
 // we patch fetch to collect cache information used for
 // determining if a page is static or not
 export function patchFetch(options: PatchableModule) {
   // If we've already patched fetch, we should not patch it again.
-  if (isPatchedFetch(globalThis.fetch)) return
+  if (isFetchPatched()) return
 
   // Grab the original fetch function. We'll attach this so we can use it in
   // the patched fetch function.

packages/next/src/server/lib/router-server.ts

@@ -46,6 +46,7 @@ import {
   type AppIsrManifestAction,
 } from '../dev/hot-reloader-types'
 import { normalizedAssetPrefix } from '../../shared/lib/normalized-asset-prefix'
+import { NEXT_PATCH_SYMBOL } from './patch-fetch'
 
 const debug = setupDebug('next:router-server:main')
 const isNextFont = (pathname: string | null) =>
@@ -109,6 +110,8 @@ export async function initialize(opts: {
 
   let devBundlerService: DevBundlerService | undefined
 
+  let originalFetch = globalThis.fetch
+
   if (opts.dev) {
     const { Telemetry } =
       require('../../telemetry/storage') as typeof import('../../telemetry/storage')
@@ -121,6 +124,11 @@ export async function initialize(opts: {
     const { setupDevBundler } =
       require('./router-utils/setup-dev-bundler') as typeof import('./router-utils/setup-dev-bundler')
 
+    const resetFetch = () => {
+      globalThis.fetch = originalFetch
+      ;(globalThis as Record<symbol, unknown>)[NEXT_PATCH_SYMBOL] = false
+    }
+
     const setupDevBundlerSpan = opts.startServerSpan
       ? opts.startServerSpan.traceChild('setup-dev-bundler')
       : trace('setup-dev-bundler')
@@ -138,6 +146,7 @@ export async function initialize(opts: {
         turbo: !!process.env.TURBOPACK,
         port: opts.port,
         onCleanup: opts.onCleanup,
+        resetFetch,
       })
     )
 
@@ -591,12 +600,12 @@ export async function initialize(opts: {
   let requestHandler: WorkerRequestHandler = requestHandlerImpl
   if (config.experimental.testProxy) {
     // Intercept fetch and other testmode apis.
-    const {
-      wrapRequestHandlerWorker,
-      interceptTestApis,
-    } = require('next/dist/experimental/testmode/server')
+    const { wrapRequestHandlerWorker, interceptTestApis } =
+      require('next/dist/experimental/testmode/server') as typeof import('next/src/experimental/testmode/server')
     requestHandler = wrapRequestHandlerWorker(requestHandler)
     interceptTestApis()
+    // We treat the intercepted fetch as "original" fetch that should be reset to during HMR.
+    originalFetch = globalThis.fetch
   }
   requestHandlers[opts.dir] = requestHandler
 

packages/next/src/server/lib/router-utils/setup-dev-bundler.ts

@@ -104,6 +104,7 @@ export type SetupOpts = {
   nextConfig: NextConfigComplete
   port: number
   onCleanup: (listener: () => Promise<void>) => void
+  resetFetch: () => void
 }
 
 export type ServerFields = {
@@ -122,6 +123,7 @@ export type ServerFields = {
     typeof import('./filesystem').buildCustomRoute
   >[]
   setAppIsrStatus?: (key: string, value: false | number | null) => void
+  resetFetch?: () => void
 }
 
 async function verifyTypeScript(opts: SetupOpts) {
@@ -152,7 +154,7 @@ export async function propagateServerField(
 }
 
 async function startWatcher(opts: SetupOpts) {
-  const { nextConfig, appDir, pagesDir, dir } = opts
+  const { nextConfig, appDir, pagesDir, dir, resetFetch } = opts
   const { useFileSystemPublicRoutes } = nextConfig
   const usingTypeScript = await verifyTypeScript(opts)
 
@@ -182,7 +184,7 @@ async function startWatcher(opts: SetupOpts) {
   })
 
   const hotReloader: NextJsHotReloaderInterface = opts.turbo
-    ? await createHotReloaderTurbopack(opts, serverFields, distDir)
+    ? await createHotReloaderTurbopack(opts, serverFields, distDir, resetFetch)
     : new HotReloaderWebpack(opts.dir, {
         appDir,
         pagesDir,
@@ -193,6 +195,7 @@ async function startWatcher(opts: SetupOpts) {
         telemetry: opts.telemetry,
         rewrites: opts.fsChecker.rewrites,
         previewProps: opts.fsChecker.prerenderManifest.preview,
+        resetFetch,
       })
 
   await hotReloader.start()

test/development/app-dir/dev-fetch-hmr/app/layout.tsx

@@ -0,0 +1,36 @@
+import React from 'react'
+import { ReactNode } from 'react'
+
+const magicNumber = Math.random()
+const originalFetch = globalThis.fetch
+
+if (originalFetch.name === 'monkeyPatchedFetch') {
+  throw new Error(
+    'Patching over already patched fetch. This creates a memory leak.'
+  )
+}
+
+globalThis.fetch = async function monkeyPatchedFetch(
+  resource: URL | RequestInfo,
+  options?: RequestInit
+) {
+  const request = new Request(resource)
+
+  if (request.url === 'http://fake.url/secret') {
+    return new Response('monkey patching is fun')
+  }
+
+  if (request.url === 'http://fake.url/magic-number') {
+    return new Response(magicNumber.toString())
+  }
+
+  return originalFetch(resource, options)
+}
+
+export default function Root({ children }: { children: ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

test/development/app-dir/dev-fetch-hmr/app/page.tsx

@@ -0,0 +1,16 @@
+export default async function Page() {
+  const secret = (await fetch('http://fake.url/secret').then((res) =>
+    res.text()
+  )) as any
+  const magicNumber = (await fetch('http://fake.url/magic-number').then((res) =>
+    res.text()
+  )) as any
+
+  return (
+    <>
+      <div id="update">touch to trigger HMR</div>
+      <div id="secret">{secret}</div>
+      <div id="magic-number">{magicNumber}</div>
+    </>
+  )
+}

test/development/app-dir/dev-fetch-hmr/dev-fetch-hmr.test.ts

@@ -0,0 +1,40 @@
+import { nextTestSetup } from 'e2e-utils'
+import { retry } from 'next-test-utils'
+
+import cheerio from 'cheerio'
+
+describe('dev-fetch-hmr', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  it('should retain module level fetch patching', async () => {
+    const html = await next.render('/')
+    expect(html).toContain('monkey patching is fun')
+
+    const magicNumber = cheerio.load(html)('#magic-number').text()
+
+    const html2 = await next.render('/')
+    expect(html2).toContain('monkey patching is fun')
+    const magicNumber2 = cheerio.load(html2)('#magic-number').text()
+    // Module was not re-evaluated
+    expect(magicNumber2).toBe(magicNumber)
+    const update = cheerio.load(html2)('#update').text()
+    expect(update).toBe('touch to trigger HMR')
+
+    // trigger HMR
+    await next.patchFile('app/page.tsx', (content) =>
+      content.replace('touch to trigger HMR', 'touch to trigger HMR 2')
+    )
+
+    await retry(async () => {
+      const html3 = await next.render('/')
+      const update2 = cheerio.load(html3)('#update').text()
+      expect(update2).toBe('touch to trigger HMR 2')
+      const magicNumber3 = cheerio.load(html3)('#magic-number').text()
+      expect(html3).toContain('monkey patching is fun')
+      // Module was re-evaluated
+      expect(magicNumber3).not.toEqual(magicNumber)
+    })
+  })
+})

test/development/app-dir/dev-fetch-hmr/next.config.js

@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig

test/development/app-dir/dev-fetch-hmr/tsconfig.json

@@ -0,0 +1,24 @@
+{
+  "compilerOptions": {
+    "target": "ES2017",
+    "lib": ["dom", "dom.iterable", "esnext"],
+    "allowJs": true,
+    "skipLibCheck": true,
+    "strict": false,
+    "noEmit": true,
+    "incremental": true,
+    "module": "esnext",
+    "esModuleInterop": true,
+    "moduleResolution": "node",
+    "resolveJsonModule": true,
+    "isolatedModules": true,
+    "jsx": "preserve",
+    "plugins": [
+      {
+        "name": "next"
+      }
+    ]
+  },
+  "include": ["next-env.d.ts", ".next/types/**/*.ts", "**/*.ts", "**/*.tsx"],
+  "exclude": ["node_modules"]
+}