Skip to content

Commit

Permalink
fix: setting assetPrefix to URL format breaks HMR (#70040)
Browse files Browse the repository at this point in the history
Backporting:
- #68622
- #68681
- #68518
  • Loading branch information
devjiwonchoi committed Sep 12, 2024
1 parent 9954a21 commit 276ddf3
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 24 deletions.
25 changes: 17 additions & 8 deletions docs/02-app/02-api-reference/05-next-config-js/assetPrefix.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
}
8 changes: 8 additions & 0 deletions packages/next/src/server/lib/router-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
)
Expand Down
44 changes: 44 additions & 0 deletions packages/next/src/shared/lib/normalized-asset-prefix.test.ts
Original file line number Diff line number Diff line change
@@ -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'
)
})
})
19 changes: 11 additions & 8 deletions packages/next/src/shared/lib/normalized-asset-prefix.ts
Original file line number Diff line number Diff line change
@@ -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}`
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Root({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>before edit</p>
}
Original file line number Diff line number Diff line change
@@ -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')
})
})
})

0 comments on commit 276ddf3

Please sign in to comment.