Fix hmr assetPrefix escaping and reuse logic from other files (#67983)

fixes #63820 fixes #55320 fixes #52931 In one of the issues, there was a suggestion to delete `assetPrefix` from `next.config`. It helps if you have `assetPrefix: '/'`. Otherwise, you probably won't face this problem. **Explanation:** The problem lies where the browser tries to connect and where websocket is available on the server. Adjust the condition for `prefix` in `get-socket-url` as it can return `''` which will lead to `//` in the url. The browser wants to connect to `ws://localhost:3000/_next/webpack-hmr` While internally next exposes the web socket under `ws://localhost:3000//_next/webpack-hmr` - you can connect to it via wscat, postman or whatever. As the path is different it does not handle HMR requests in the browser. In addition to that - Reuse logic and extract separate files as a helper in shared/lib. | Before | After | | ------ | ------- | | ![before - latest canary branch](https://github.com/user-attachments/assets/c26c8b20-1352-49c6-a099-101394438ba0) before - latest canary branch - we can't establish a connection to HMR ws | ![after](https://github.com/user-attachments/assets/26500e68-bc4d-44ca-b418-f9bda6bc9aa6) same example with local changes - connected to ws | --------- Co-authored-by: Jiwon Choi <devjiwonchoi@gmail.com> Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Aug 26, 2024 · 6b839ed · 6b839ed
1 parent a1a006b
commit 6b839ed
Show file tree

Hide file tree

Showing 5 changed files with 842 additions and 902 deletions.
diff --git a/packages/next/src/client/components/react-dev-overlay/internal/helpers/get-socket-url.ts b/packages/next/src/client/components/react-dev-overlay/internal/helpers/get-socket-url.ts
@@ -1,3 +1,5 @@
+import { normalizedAssetPrefix } from '../../../../../shared/lib/normalized-asset-prefix'
+
 function getSocketProtocol(assetPrefix: string): string {
   let protocol = window.location.protocol
 
@@ -9,18 +11,16 @@ function getSocketProtocol(assetPrefix: string): string {
   return protocol === 'http:' ? 'ws' : 'wss'
 }
 
-export function getSocketUrl(assetPrefix: string): string {
+export function getSocketUrl(assetPrefix: string | undefined): string {
   const { hostname, port } = window.location
-  const protocol = getSocketProtocol(assetPrefix)
-  const normalizedAssetPrefix = assetPrefix.replace(/^\/+/, '')
-
-  let url = `${protocol}://${hostname}:${port}${
-    normalizedAssetPrefix ? `/${normalizedAssetPrefix}` : ''
-  }`
+  const protocol = getSocketProtocol(assetPrefix || '')
+  const prefix = normalizedAssetPrefix(assetPrefix)
 
-  if (normalizedAssetPrefix.startsWith('http')) {
-    url = `${protocol}://${normalizedAssetPrefix.split('://', 2)[1]}`
+  // if original assetPrefix is a full URL with protocol
+  // we just update to use the correct `ws` protocol
+  if (assetPrefix?.replace(/^\/+/, '').includes('://')) {
+    return `${protocol}://${prefix}`
   }
 
-  return url
+  return `${protocol}://${hostname}:${port}${prefix}`
 }
diff --git a/packages/next/src/client/components/react-dev-overlay/pages/websocket.ts b/packages/next/src/client/components/react-dev-overlay/pages/websocket.ts
@@ -1,22 +1,12 @@
 import type { HMR_ACTION_TYPES } from '../../../../server/dev/hot-reloader-types'
+import { getSocketUrl } from '../internal/helpers/get-socket-url'
 
 let source: WebSocket
 
 type ActionCallback = (action: HMR_ACTION_TYPES) => void
 
 const eventCallbacks: Array<ActionCallback> = []
 
-function getSocketProtocol(assetPrefix: string): string {
-  let protocol = location.protocol
-
-  try {
-    // assetPrefix is a url
-    protocol = new URL(assetPrefix).protocol
-  } catch {}
-
-  return protocol === 'http:' ? 'ws' : 'wss'
-}
-
 export function addMessageListener(callback: ActionCallback) {
   eventCallbacks.push(callback)
 }
@@ -62,17 +52,7 @@ export function connectHMR(options: { path: string; assetPrefix: string }) {
       timer = setTimeout(init, reconnections > 5 ? 5000 : 1000)
     }
 
-    const { hostname, port } = location
-    const protocol = getSocketProtocol(options.assetPrefix || '')
-    const assetPrefix = options.assetPrefix.replace(/^\/+/, '')
-
-    let url = `${protocol}://${hostname}:${port}${
-      assetPrefix ? `/${assetPrefix}` : ''
-    }`
-
-    if (assetPrefix.startsWith('http')) {
-      url = `${protocol}://${assetPrefix.split('://', 2)[1]}`
-    }
+    const url = getSocketUrl(options.assetPrefix)
 
     source = new window.WebSocket(`${url}${options.path}`)
     source.onopen = handleOnline

diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts
@@ -40,6 +40,7 @@ import { ensureLeadingSlash } from '../../shared/lib/page-path/ensure-leading-sl
 import { getNextPathnameInfo } from '../../shared/lib/router/utils/get-next-pathname-info'
 import { getHostname } from '../../shared/lib/get-hostname'
 import { detectDomainLocale } from '../../shared/lib/i18n/detect-domain-locale'
+import { normalizedAssetPrefix } from '../../shared/lib/normalized-asset-prefix'
 
 const debug = setupDebug('next:router-server:main')
 const isNextFont = (pathname: string | null) =>
@@ -651,8 +652,14 @@ export async function initialize(opts: {
       if (opts.dev && developmentBundler && req.url) {
         const { basePath, assetPrefix } = config
 
+        let hmrPrefix = basePath
+
+        // assetPrefix overrides basePath for HMR path
+        if (assetPrefix) {
+          hmrPrefix = normalizedAssetPrefix(assetPrefix)
+        }
         const isHMRRequest = req.url.startsWith(
-          ensureLeadingSlash(`${assetPrefix || basePath}/_next/webpack-hmr`)
+          ensureLeadingSlash(`${hmrPrefix}/_next/webpack-hmr`)
         )
 
         // only handle HMR requests if the basePath in the request

diff --git a/packages/next/src/shared/lib/normalized-asset-prefix.ts b/packages/next/src/shared/lib/normalized-asset-prefix.ts
@@ -0,0 +1,16 @@
+export function normalizedAssetPrefix(assetPrefix: string | undefined): string {
+  const escapedAssetPrefix = assetPrefix?.replace(/^\/+/, '') || false
+
+  // assetPrefix as a url
+  if (escapedAssetPrefix && escapedAssetPrefix.startsWith('://')) {
+    return escapedAssetPrefix.split('://', 2)[1]
+  }
+
+  // assetPrefix is set to `undefined` or '/'
+  if (!escapedAssetPrefix) {
+    return ''
+  }
+
+  // assetPrefix is a common path but escaped so let's add one leading slash
+  return `/${escapedAssetPrefix}`
+}