From 3c0c5e43f21e29f34a829f669ca54435a7cbe6f7 Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Wed, 7 Aug 2024 19:30:46 +0900 Subject: [PATCH 1/6] test: add e2e test for assetPrefix with full url --- .../asset-prefix-full-url/app/layout.tsx | 7 +++++ .../asset-prefix-full-url/app/page.tsx | 3 ++ .../asset-prefix.test.ts | 31 +++++++++++++++++++ .../asset-prefix-full-url/next.config.js | 4 +++ 4 files changed, 45 insertions(+) create mode 100644 test/e2e/app-dir/asset-prefix-full-url/app/layout.tsx create mode 100644 test/e2e/app-dir/asset-prefix-full-url/app/page.tsx create mode 100644 test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts create mode 100644 test/e2e/app-dir/asset-prefix-full-url/next.config.js diff --git a/test/e2e/app-dir/asset-prefix-full-url/app/layout.tsx b/test/e2e/app-dir/asset-prefix-full-url/app/layout.tsx new file mode 100644 index 0000000000000..a3a86a5ca1e12 --- /dev/null +++ b/test/e2e/app-dir/asset-prefix-full-url/app/layout.tsx @@ -0,0 +1,7 @@ +export default function Root({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/asset-prefix-full-url/app/page.tsx b/test/e2e/app-dir/asset-prefix-full-url/app/page.tsx new file mode 100644 index 0000000000000..fb9b4085fcd27 --- /dev/null +++ b/test/e2e/app-dir/asset-prefix-full-url/app/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return

before edit

+} diff --git a/test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts b/test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts new file mode 100644 index 0000000000000..7b83db727548d --- /dev/null +++ b/test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts @@ -0,0 +1,31 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('app-dir assetPrefix full URL', () => { + const { next, isNextDev } = nextTestSetup({ + files: __dirname, + skipDeployment: true, + }) + + if (isNextDev) { + it('should not break HMR', async () => { + await next.patchFile('next.config.js', (content) => + content.replace('undefined', `'http://localhost:${next.appPort}'`) + ) + + const $ = await next.render$('/') + expect($('p').text()).toBe('before edit') + + await next.patchFile('app/page.tsx', (content) => + content.replace('before', 'after') + ) + + const $2 = await next.render$('/') + expect($2('p').text()).toBe('after edit') + }) + } + + it('should build correctly when assetPrefix is undefined', async () => { + const $ = await next.render$('/') + expect($('p').text()).toBe('before edit') + }) +}) diff --git a/test/e2e/app-dir/asset-prefix-full-url/next.config.js b/test/e2e/app-dir/asset-prefix-full-url/next.config.js new file mode 100644 index 0000000000000..226290d1bf86f --- /dev/null +++ b/test/e2e/app-dir/asset-prefix-full-url/next.config.js @@ -0,0 +1,4 @@ +/** @type {import('next').NextConfig} */ +module.exports = { + assetPrefix: undefined, +} From 63a840035ed391afbbd006e4fdfb5b094671e0bf Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Wed, 7 Aug 2024 19:33:56 +0900 Subject: [PATCH 2/6] chore: add warn if assetprefix is url in dev --- packages/next/src/server/config.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/next/src/server/config.ts b/packages/next/src/server/config.ts index c1eedc43bce69..e2240be056748 100644 --- a/packages/next/src/server/config.ts +++ b/packages/next/src/server/config.ts @@ -1069,6 +1069,16 @@ export default async function loadConfig( } } + if ( + phase === PHASE_DEVELOPMENT_SERVER && + URL.canParse(userConfig.assetPrefix ?? '') + ) { + curLog.warn( + `Absolute URL assetPrefix "${userConfig.assetPrefix}" may disrupt development HMR.\n` + + 'See more info here https://nextjs.org/docs/app/api-reference/next-config-js/assetPrefix' + ) + } + if (userConfig.target && userConfig.target !== 'server') { throw new Error( `The "target" property is no longer supported in ${configFileName}.\n` + From 5f9da291c9b83c834c8dee06ac0ba4edc1367efb Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Thu, 8 Aug 2024 02:54:25 +0900 Subject: [PATCH 3/6] test: move to development --- .../hmr-asset-prefix-full-url}/app/layout.tsx | 0 .../hmr-asset-prefix-full-url}/app/page.tsx | 0 .../asset-prefix.test.ts | 30 ++++++++++++++++++ .../asset-prefix.test.ts | 31 ------------------- .../asset-prefix-full-url/next.config.js | 4 --- 5 files changed, 30 insertions(+), 35 deletions(-) rename test/{e2e/app-dir/asset-prefix-full-url => development/app-dir/hmr-asset-prefix-full-url}/app/layout.tsx (100%) rename test/{e2e/app-dir/asset-prefix-full-url => development/app-dir/hmr-asset-prefix-full-url}/app/page.tsx (100%) create mode 100644 test/development/app-dir/hmr-asset-prefix-full-url/asset-prefix.test.ts delete mode 100644 test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts delete mode 100644 test/e2e/app-dir/asset-prefix-full-url/next.config.js diff --git a/test/e2e/app-dir/asset-prefix-full-url/app/layout.tsx b/test/development/app-dir/hmr-asset-prefix-full-url/app/layout.tsx similarity index 100% rename from test/e2e/app-dir/asset-prefix-full-url/app/layout.tsx rename to test/development/app-dir/hmr-asset-prefix-full-url/app/layout.tsx diff --git a/test/e2e/app-dir/asset-prefix-full-url/app/page.tsx b/test/development/app-dir/hmr-asset-prefix-full-url/app/page.tsx similarity index 100% rename from test/e2e/app-dir/asset-prefix-full-url/app/page.tsx rename to test/development/app-dir/hmr-asset-prefix-full-url/app/page.tsx 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..30bc0b88343b0 --- /dev/null +++ b/test/development/app-dir/hmr-asset-prefix-full-url/asset-prefix.test.ts @@ -0,0 +1,30 @@ +import { nextTestSetup } from 'e2e-utils' +import { retry } from 'next-test-utils' + +// TODO: findPort not working anyway how +const forcedPort = String(Math.floor(10000 + Math.random() * 90000)) + +describe('app-dir assetPrefix full URL', () => { + const { next } = nextTestSetup({ + files: __dirname, + forcedPort, + skipDeployment: true, + nextConfig: { + assetPrefix: `http://localhost:${forcedPort}`, + }, + }) + + it('should not break when renaming a folder', 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()).toContain('after edit') + }) + }) +}) diff --git a/test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts b/test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts deleted file mode 100644 index 7b83db727548d..0000000000000 --- a/test/e2e/app-dir/asset-prefix-full-url/asset-prefix.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { nextTestSetup } from 'e2e-utils' - -describe('app-dir assetPrefix full URL', () => { - const { next, isNextDev } = nextTestSetup({ - files: __dirname, - skipDeployment: true, - }) - - if (isNextDev) { - it('should not break HMR', async () => { - await next.patchFile('next.config.js', (content) => - content.replace('undefined', `'http://localhost:${next.appPort}'`) - ) - - const $ = await next.render$('/') - expect($('p').text()).toBe('before edit') - - await next.patchFile('app/page.tsx', (content) => - content.replace('before', 'after') - ) - - const $2 = await next.render$('/') - expect($2('p').text()).toBe('after edit') - }) - } - - it('should build correctly when assetPrefix is undefined', async () => { - const $ = await next.render$('/') - expect($('p').text()).toBe('before edit') - }) -}) diff --git a/test/e2e/app-dir/asset-prefix-full-url/next.config.js b/test/e2e/app-dir/asset-prefix-full-url/next.config.js deleted file mode 100644 index 226290d1bf86f..0000000000000 --- a/test/e2e/app-dir/asset-prefix-full-url/next.config.js +++ /dev/null @@ -1,4 +0,0 @@ -/** @type {import('next').NextConfig} */ -module.exports = { - assetPrefix: undefined, -} From e8627905532822a7dec6c00da1ee514c01a7e29d Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Thu, 8 Aug 2024 02:59:50 +0900 Subject: [PATCH 4/6] fix: check for hmr only for pathname --- packages/next/src/server/lib/router-server.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts index fc9dfd76a0678..eeae595eb0f36 100644 --- a/packages/next/src/server/lib/router-server.ts +++ b/packages/next/src/server/lib/router-server.ts @@ -668,7 +668,14 @@ export async function initialize(opts: { // assetPrefix overrides basePath for HMR path if (assetPrefix) { hmrPrefix = normalizedAssetPrefix(assetPrefix) + + if (URL.canParse(hmrPrefix)) { + // pathname without trailing slash + // or if is '/', replace to '' to not conflict + hmrPrefix = new URL(hmrPrefix).pathname.replace(/\/$/, '') + } } + const isHMRRequest = req.url.startsWith( ensureLeadingSlash(`${hmrPrefix}/_next/webpack-hmr`) ) From 9a770f8fc02e867b7ce46d7194e75d69f2995163 Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Thu, 8 Aug 2024 12:11:48 +0900 Subject: [PATCH 5/6] test: update to use createNext Co-authored-by: JJ Kasper --- .../asset-prefix.test.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) 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 index 30bc0b88343b0..16b582dcf5295 100644 --- 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 @@ -1,20 +1,22 @@ -import { nextTestSetup } from 'e2e-utils' -import { retry } from 'next-test-utils' - -// TODO: findPort not working anyway how -const forcedPort = String(Math.floor(10000 + Math.random() * 90000)) +import { createNext } from 'e2e-utils' +import { findPort, retry } from 'next-test-utils' describe('app-dir assetPrefix full URL', () => { - const { next } = nextTestSetup({ - files: __dirname, - forcedPort, - skipDeployment: true, - nextConfig: { - assetPrefix: `http://localhost:${forcedPort}`, - }, + 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 when renaming a folder', async () => { + 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') From d691ea996b7a078ccdbe2e43450f2088423d5168 Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Thu, 8 Aug 2024 12:18:42 +0900 Subject: [PATCH 6/6] misc: better comment --- packages/next/src/server/lib/router-server.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts index eeae595eb0f36..70be360d51035 100644 --- a/packages/next/src/server/lib/router-server.ts +++ b/packages/next/src/server/lib/router-server.ts @@ -670,8 +670,9 @@ export async function initialize(opts: { hmrPrefix = normalizedAssetPrefix(assetPrefix) if (URL.canParse(hmrPrefix)) { - // pathname without trailing slash - // or if is '/', replace to '' to not conflict + // remove trailing slash from pathname + // return empty string if pathname is '/' + // to avoid conflicts with '/_next' below hmrPrefix = new URL(hmrPrefix).pathname.replace(/\/$/, '') } }