Wait for pending Webpack Hot Updates before evaluating JS from RSC responses (#67673)

Author: eps1lon

packages/next/src/client/components/react-dev-overlay/app/hot-reloader-client.tsx

@@ -47,17 +47,35 @@ let __nextDevClientId = Math.round(Math.random() * 100 + Date.now())
 let reloading = false
 let startLatency: number | null = null
 
-function onBeforeFastRefresh(dispatcher: Dispatcher, hasUpdates: boolean) {
+let pendingHotUpdateWebpack = Promise.resolve()
+let resolvePendingHotUpdateWebpack: () => void = () => {}
+function setPendingHotUpdateWebpack() {
+  pendingHotUpdateWebpack = new Promise((resolve) => {
+    resolvePendingHotUpdateWebpack = () => {
+      resolve()
+    }
+  })
+}
+
+export function waitForWebpackRuntimeHotUpdate() {
+  return pendingHotUpdateWebpack
+}
+
+function handleBeforeHotUpdateWebpack(
+  dispatcher: Dispatcher,
+  hasUpdates: boolean
+) {
   if (hasUpdates) {
     dispatcher.onBeforeRefresh()
   }
 }
 
-function onFastRefresh(
+function handleSuccessfulHotUpdateWebpack(
   dispatcher: Dispatcher,
   sendMessage: (message: string) => void,
   updatedModules: ReadonlyArray<string>
 ) {
+  resolvePendingHotUpdateWebpack()
   dispatcher.onBuildOk()
   reportHmrLatency(sendMessage, updatedModules)
 
@@ -159,6 +177,7 @@ function tryApplyUpdates(
   dispatcher: Dispatcher
 ) {
   if (!isUpdateAvailable() || !canApplyUpdates()) {
+    resolvePendingHotUpdateWebpack()
     dispatcher.onBuildOk()
     reportHmrLatency(sendMessage, [])
     return
@@ -281,12 +300,16 @@ function processMessage(
     } else {
       tryApplyUpdates(
         function onBeforeHotUpdate(hasUpdates: boolean) {
-          onBeforeFastRefresh(dispatcher, hasUpdates)
+          handleBeforeHotUpdateWebpack(dispatcher, hasUpdates)
         },
         function onSuccessfulHotUpdate(webpackUpdatedModules: string[]) {
           // Only dismiss it when we're sure it's a hot update.
           // Otherwise it would flicker right before the reload.
-          onFastRefresh(dispatcher, sendMessage, webpackUpdatedModules)
+          handleSuccessfulHotUpdateWebpack(
+            dispatcher,
+            sendMessage,
+            webpackUpdatedModules
+          )
         },
         sendMessage,
         dispatcher
@@ -320,6 +343,9 @@ function processMessage(
     }
     case HMR_ACTIONS_SENT_TO_BROWSER.BUILDING: {
       startLatency = Date.now()
+      if (!process.env.TURBOPACK) {
+        setPendingHotUpdateWebpack()
+      }
       console.log('[Fast Refresh] rebuilding')
       break
     }
@@ -426,6 +452,7 @@ function processMessage(
         reloading = true
         return window.location.reload()
       }
+      resolvePendingHotUpdateWebpack()
       startTransition(() => {
         router.hmrRefresh()
         dispatcher.onRefresh()

packages/next/src/client/components/router-reducer/fetch-server-response.ts

@@ -29,6 +29,7 @@ import {
 import { callServer } from '../../app-call-server'
 import { PrefetchKind } from './router-reducer-types'
 import { hexHash } from '../../../shared/lib/hash'
+import { waitForWebpackRuntimeHotUpdate } from '../react-dev-overlay/app/hot-reloader-client'
 
 export interface FetchServerResponseOptions {
   readonly flightRouterState: FlightRouterState
@@ -180,6 +181,14 @@ export async function fetchServerResponse(
       return doMpaNavigation(responseUrl.toString())
     }
 
+    // We may navigate to a page that requires a different Webpack runtime.
+    // In prod, every page will have the same Webpack runtime.
+    // In dev, the Webpack runtime is minimal for each page.
+    // We need to ensure the Webpack runtime is updated before executing client-side JS of the new page.
+    if (process.env.NODE_ENV !== 'production' && !process.env.TURBOPACK) {
+      await waitForWebpackRuntimeHotUpdate()
+    }
+
     // Handle the `fetch` readable stream that can be unwrapped by `React.use`.
     const response: NavigationFlightResponse = await createFromFetch(
       Promise.resolve(res),

test/development/app-hmr/app/bundler-runtime-changes/new-runtime-functionality/page.tsx

@@ -0,0 +1,8 @@
+'use client'
+// requires
+import * as React from 'react'
+
+export default function RuntimeChangesNewRuntimeFunctionalityPage() {
+  React.useEffect(() => {}, [])
+  return <div data-testid="new-runtime-functionality-page" />
+}

test/development/app-hmr/app/bundler-runtime-changes/page.tsx

@@ -0,0 +1,9 @@
+import Link from 'next/link'
+
+export default function RuntimeChangesPage() {
+  return (
+    <Link href="/bundler-runtime-changes/new-runtime-functionality">
+      Click me
+    </Link>
+  )
+}

test/development/app-hmr/app/favicon.ico

@@ -99,7 +99,7 @@ describe(`app-dir-hmr`, () => {
                 'window.__TEST_NO_RELOAD === undefined'
               )
               // Used to be flaky but presumably no longer is.
-              // If this flakes again, please add the received value as a commnet.
+              // If this flakes again, please add the received value as a comment.
               expect({ envValue, mpa }).toEqual({
                 envValue: 'ipad',
                 mpa: false,
@@ -245,5 +245,73 @@ describe(`app-dir-hmr`, () => {
     it('should have no unexpected action error for hmr', async () => {
       expect(next.cliOutput).not.toContain('Unexpected action')
     })
+
+    it('can navigate cleanly to a page that requires a change in the Webpack runtime', async () => {
+      // This isn't a very accurate test since the Webpack runtime is somewhat an implementation detail.
+      // To ensure this is still valid, check the `*/webpack.*.hot-update.js` network response content when the navigation is triggered.
+      // If there is new functionality added, the test is still valid.
+      // If not, the test doesn't cover anything new.
+      // TODO: Enforce console.error assertions or MPA navigation assertions in all tests instead.
+      const browser = await next.browser('/bundler-runtime-changes')
+      await browser.eval('window.__TEST_NO_RELOAD = true')
+
+      await browser
+        .elementByCss('a')
+        .click()
+        .waitForElementByCss('[data-testid="new-runtime-functionality-page"]')
+
+      const logs = await browser.log()
+      // TODO: Should assert on all logs but these are cluttered with logs from our test utils (e.g. playwright tracing or webdriver)
+      if (process.env.TURBOPACK) {
+        // FIXME: logging "rebuilding" multiple times instead of closing it of with "done in"
+        // Should just not branch here and have the same logs as Webpack.
+        expect(logs).toEqual(
+          expect.arrayContaining([
+            {
+              message: '[Fast Refresh] rebuilding',
+              source: 'log',
+            },
+            {
+              message: '[Fast Refresh] rebuilding',
+              source: 'log',
+            },
+            {
+              message: '[Fast Refresh] rebuilding',
+              source: 'log',
+            },
+          ])
+        )
+        expect(logs).not.toEqual(
+          expect.arrayContaining([
+            {
+              message: expect.stringContaining('[Fast Refresh] done in'),
+              source: 'log',
+            },
+          ])
+        )
+      } else {
+        expect(logs).toEqual(
+          expect.arrayContaining([
+            {
+              message: '[Fast Refresh] rebuilding',
+              source: 'log',
+            },
+            {
+              message: expect.stringContaining('[Fast Refresh] done in'),
+              source: 'log',
+            },
+          ])
+        )
+        expect(logs).not.toEqual(
+          expect.arrayContaining([
+            expect.objectContaining({
+              source: 'error',
+            }),
+          ])
+        )
+      }
+      // No MPA navigation triggered
+      expect(await browser.eval('window.__TEST_NO_RELOAD')).toEqual(true)
+    })
   })
 })

test/development/app-hmr/hmr.test.ts

@@ -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"]
+}