From 276ddf32b19b39d162ff7065cd9ab48053cf86a1 Mon Sep 17 00:00:00 2001 From: Jiwon Choi Date: Fri, 13 Sep 2024 03:24:43 +0900 Subject: [PATCH] fix: setting `assetPrefix` to URL format breaks HMR (#70040) Backporting: - https://github.com/vercel/next.js/pull/68622 - https://github.com/vercel/next.js/pull/68681 - https://github.com/vercel/next.js/pull/68518 --- .../05-next-config-js/assetPrefix.mdx | 25 +++++++---- .../internal/helpers/get-socket-url.ts | 16 +++---- packages/next/src/server/lib/router-server.ts | 8 ++++ .../lib/normalized-asset-prefix.test.ts | 44 +++++++++++++++++++ .../src/shared/lib/normalized-asset-prefix.ts | 19 ++++---- .../hmr-asset-prefix-full-url/app/layout.tsx | 7 +++ .../hmr-asset-prefix-full-url/app/page.tsx | 3 ++ .../asset-prefix.test.ts | 32 ++++++++++++++ 8 files changed, 130 insertions(+), 24 deletions(-) create mode 100644 packages/next/src/shared/lib/normalized-asset-prefix.test.ts create mode 100644 test/development/app-dir/hmr-asset-prefix-full-url/app/layout.tsx create mode 100644 test/development/app-dir/hmr-asset-prefix-full-url/app/page.tsx create mode 100644 test/development/app-dir/hmr-asset-prefix-full-url/asset-prefix.test.ts diff --git a/docs/02-app/02-api-reference/05-next-config-js/assetPrefix.mdx b/docs/02-app/02-api-reference/05-next-config-js/assetPrefix.mdx index 1f9d6ef40d094..40a1db9d946e9 100644 --- a/docs/02-app/02-api-reference/05-next-config-js/assetPrefix.mdx +++ b/docs/02-app/02-api-reference/05-next-config-js/assetPrefix.mdx @@ -23,16 +23,25 @@ description: Learn how to use the assetPrefix config option to configure your CD > suited for hosting your application on a sub-path like `/docs`. > We do not suggest you use a custom Asset Prefix for this use case. -To set up a [CDN](https://en.wikipedia.org/wiki/Content_delivery_network), you can set up an asset prefix and configure your CDN's origin to resolve to the domain that Next.js is hosted on. - -Open `next.config.js` and add the `assetPrefix` config: +## Set up a CDN -```js filename="next.config.js" -const isProd = process.env.NODE_ENV === 'production' +To set up a [CDN](https://en.wikipedia.org/wiki/Content_delivery_network), you can set up an asset prefix and configure your CDN's origin to resolve to the domain that Next.js is hosted on. -module.exports = { - // Use the CDN in production and localhost for development. - assetPrefix: isProd ? 'https://cdn.mydomain.com' : undefined, +Open `next.config.mjs` and add the `assetPrefix` config based on the [phase](/docs/app/api-reference/next-config-js#async-configuration): + +```js filename="next.config.mjs" +// @ts-check +import { PHASE_DEVELOPMENT_SERVER } from 'next/constants' + +export default (phase) => { + const isDev = phase === PHASE_DEVELOPMENT_SERVER + /** + * @type {import('next').NextConfig} + */ + const nextConfig = { + assetPrefix: isDev ? undefined : 'https://cdn.mydomain.com', + } + return nextConfig } ``` 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 index b9a01f9bcc71e..3a3b87ac28a5b 100644 --- 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 @@ -8,19 +8,19 @@ function getSocketProtocol(assetPrefix: string): string { protocol = new URL(assetPrefix).protocol } catch {} - return protocol === 'http:' ? 'ws' : 'wss' + return protocol === 'http:' ? 'ws:' : 'wss:' } export function getSocketUrl(assetPrefix: string | undefined): string { - const { hostname, port } = window.location - const protocol = getSocketProtocol(assetPrefix || '') const prefix = normalizedAssetPrefix(assetPrefix) + const protocol = getSocketProtocol(assetPrefix || '') - // 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}` + if (URL.canParse(prefix)) { + // since normalized asset prefix is ensured to be a URL format, + // we can safely replace the protocol + return prefix.replace(/^http/, 'ws') } - return `${protocol}://${hostname}:${port}${prefix}` + const { hostname, port } = window.location + return `${protocol}//${hostname}${port ? `:${port}` : ''}${prefix}` } diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts index 37c87d3aa2f96..1739d21bc5112 100644 --- a/packages/next/src/server/lib/router-server.ts +++ b/packages/next/src/server/lib/router-server.ts @@ -657,7 +657,15 @@ export async function initialize(opts: { // assetPrefix overrides basePath for HMR path if (assetPrefix) { hmrPrefix = normalizedAssetPrefix(assetPrefix) + + if (URL.canParse(hmrPrefix)) { + // remove trailing slash from pathname + // return empty string if pathname is '/' + // to avoid conflicts with '/_next' below + hmrPrefix = new URL(hmrPrefix).pathname.replace(/\/$/, '') + } } + const isHMRRequest = req.url.startsWith( ensureLeadingSlash(`${hmrPrefix}/_next/webpack-hmr`) ) diff --git a/packages/next/src/shared/lib/normalized-asset-prefix.test.ts b/packages/next/src/shared/lib/normalized-asset-prefix.test.ts new file mode 100644 index 0000000000000..2c6171c0eadde --- /dev/null +++ b/packages/next/src/shared/lib/normalized-asset-prefix.test.ts @@ -0,0 +1,44 @@ +import { normalizedAssetPrefix } from './normalized-asset-prefix' + +describe('normalizedAssetPrefix', () => { + it('should return an empty string when assetPrefix is nullish', () => { + expect(normalizedAssetPrefix(undefined)).toBe('') + }) + + it('should return an empty string when assetPrefix is an empty string', () => { + expect(normalizedAssetPrefix('')).toBe('') + }) + + it('should return an empty string when assetPrefix is a single slash', () => { + expect(normalizedAssetPrefix('/')).toBe('') + }) + + // we expect an empty string because it could be an unnecessary trailing slash + it('should remove leading slash(es) when assetPrefix has more than one', () => { + expect(normalizedAssetPrefix('///path/to/asset')).toBe('/path/to/asset') + }) + + it('should not remove the leading slash when assetPrefix has only one', () => { + expect(normalizedAssetPrefix('/path/to/asset')).toBe('/path/to/asset') + }) + + it('should add a leading slash when assetPrefix is missing one', () => { + expect(normalizedAssetPrefix('path/to/asset')).toBe('/path/to/asset') + }) + + it('should remove all trailing slash(es) when assetPrefix has one', () => { + expect(normalizedAssetPrefix('/path/to/asset///')).toBe('/path/to/asset') + }) + + it('should return the URL when assetPrefix is a URL', () => { + expect(normalizedAssetPrefix('https://example.com/path/to/asset')).toBe( + 'https://example.com/path/to/asset' + ) + }) + + it('should not leave a trailing slash when assetPrefix is a URL with no pathname', () => { + expect(normalizedAssetPrefix('https://example.com')).toBe( + 'https://example.com' + ) + }) +}) diff --git a/packages/next/src/shared/lib/normalized-asset-prefix.ts b/packages/next/src/shared/lib/normalized-asset-prefix.ts index b7da7771915a6..352e836698c5c 100644 --- a/packages/next/src/shared/lib/normalized-asset-prefix.ts +++ b/packages/next/src/shared/lib/normalized-asset-prefix.ts @@ -1,16 +1,19 @@ export function normalizedAssetPrefix(assetPrefix: string | undefined): string { - const escapedAssetPrefix = assetPrefix?.replace(/^\/+/, '') || false + // remove all leading slashes and trailing slashes + const escapedAssetPrefix = assetPrefix?.replace(/^\/+|\/+$/g, '') || false - // assetPrefix as a url - if (escapedAssetPrefix && escapedAssetPrefix.startsWith('://')) { - return escapedAssetPrefix.split('://', 2)[1] - } - - // assetPrefix is set to `undefined` or '/' + // if an assetPrefix was '/', we return empty string + // because it could be an unnecessary trailing slash if (!escapedAssetPrefix) { return '' } - // assetPrefix is a common path but escaped so let's add one leading slash + if (URL.canParse(escapedAssetPrefix)) { + const url = new URL(escapedAssetPrefix).toString() + return url.endsWith('/') ? url.slice(0, -1) : url + } + + // assuming assetPrefix here is a pathname-style, + // restore the leading slash return `/${escapedAssetPrefix}` } diff --git a/test/development/app-dir/hmr-asset-prefix-full-url/app/layout.tsx b/test/development/app-dir/hmr-asset-prefix-full-url/app/layout.tsx new file mode 100644 index 0000000000000..a3a86a5ca1e12 --- /dev/null +++ b/test/development/app-dir/hmr-asset-prefix-full-url/app/layout.tsx @@ -0,0 +1,7 @@ +export default function Root({ children }) { + return ( + + {children} + + ) +} diff --git a/test/development/app-dir/hmr-asset-prefix-full-url/app/page.tsx b/test/development/app-dir/hmr-asset-prefix-full-url/app/page.tsx new file mode 100644 index 0000000000000..fb9b4085fcd27 --- /dev/null +++ b/test/development/app-dir/hmr-asset-prefix-full-url/app/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return

before edit

+} diff --git a/test/development/app-dir/hmr-asset-prefix-full-url/asset-prefix.test.ts b/test/development/app-dir/hmr-asset-prefix-full-url/asset-prefix.test.ts new file mode 100644 index 0000000000000..42bb67e99d429 --- /dev/null +++ b/test/development/app-dir/hmr-asset-prefix-full-url/asset-prefix.test.ts @@ -0,0 +1,32 @@ +import { createNext } from 'e2e-utils' +import { findPort, retry } from 'next-test-utils' + +describe('app-dir assetPrefix full URL', () => { + let next, forcedPort + beforeAll(async () => { + forcedPort = ((await findPort()) ?? '54321').toString() + + next = await createNext({ + files: __dirname, + forcedPort, + nextConfig: { + assetPrefix: `http://localhost:${forcedPort}`, + }, + }) + }) + afterAll(() => next.destroy()) + + it('should not break HMR when asset prefix set to full URL', async () => { + const browser = await next.browser('/') + const text = await browser.elementByCss('p').text() + expect(text).toBe('before edit') + + await next.patchFile('app/page.tsx', (content) => { + return content.replace('before', 'after') + }) + + await retry(async () => { + expect(await browser.elementByCss('p').text()).toBe('after edit') + }) + }) +})