-
Notifications
You must be signed in to change notification settings - Fork 248
feat: support assetPrefix in next.config #474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
00c8711
8b5071a
d99cc97
04e09e1
6949427
290a2aa
ef6d83a
eca4295
dd8364e
6150056
f0cee58
752b5e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,12 +4,13 @@ | |
| * Loads the Next.js config file (if present) and extracts supported options. | ||
| * Unsupported options are logged as warnings. | ||
| */ | ||
| import path from "node:path"; | ||
| import { createRequire } from "node:module"; | ||
| import fs from "node:fs"; | ||
|
|
||
| import { randomUUID } from "node:crypto"; | ||
| import { PHASE_DEVELOPMENT_SERVER } from "../shims/constants.js"; | ||
| import fs from "node:fs"; | ||
| import { createRequire } from "node:module"; | ||
| import path from "node:path"; | ||
| import { normalizePageExtensions } from "../routing/file-matcher.js"; | ||
| import { PHASE_DEVELOPMENT_SERVER } from "../shims/constants.js"; | ||
| import { isExternalUrl } from "./config-matchers.js"; | ||
|
|
||
| /** | ||
|
|
@@ -127,6 +128,8 @@ export interface NextConfig { | |
| env?: Record<string, string>; | ||
| /** Base URL path prefix */ | ||
| basePath?: string; | ||
| /** CDN URL prefix for all static assets (JS chunks, CSS, fonts). Trailing slash stripped. */ | ||
| assetPrefix?: string; | ||
| /** Whether to add trailing slashes */ | ||
| trailingSlash?: boolean; | ||
| /** Internationalization routing config */ | ||
|
|
@@ -204,6 +207,8 @@ export interface NextConfig { | |
| export interface ResolvedNextConfig { | ||
| env: Record<string, string>; | ||
| basePath: string; | ||
| /** Resolved CDN URL prefix for static assets. Empty string if not set. */ | ||
| assetPrefix: string; | ||
| trailingSlash: boolean; | ||
| output: "" | "export" | "standalone"; | ||
| pageExtensions: string[]; | ||
|
|
@@ -393,6 +398,7 @@ export async function resolveNextConfig( | |
| const resolved: ResolvedNextConfig = { | ||
| env: {}, | ||
| basePath: "", | ||
| assetPrefix: "", | ||
| trailingSlash: false, | ||
| output: "", | ||
| pageExtensions: normalizePageExtensions(), | ||
|
|
@@ -526,6 +532,8 @@ export async function resolveNextConfig( | |
| const resolved: ResolvedNextConfig = { | ||
| env: config.env ?? {}, | ||
| basePath: config.basePath ?? "", | ||
| assetPrefix: | ||
| typeof config.assetPrefix === "string" ? config.assetPrefix.replace(/\/$/, "") : "", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| trailingSlash: config.trailingSlash ?? false, | ||
| output: output === "export" || output === "standalone" ? output : "", | ||
| pageExtensions, | ||
|
|
@@ -543,6 +551,13 @@ export async function resolveNextConfig( | |
| buildId, | ||
| }; | ||
|
|
||
| // If assetPrefix is empty and basePath is set, inherit basePath as assetPrefix. | ||
| // This matches Next.js behavior: static assets are served under basePath when | ||
| // no explicit CDN prefix is configured. | ||
| if (resolved.assetPrefix === "" && resolved.basePath !== "") { | ||
| resolved.assetPrefix = resolved.basePath; | ||
| } | ||
|
|
||
| // Auto-detect next-intl (lowest priority — explicit aliases from | ||
| // webpack/turbopack already in `aliases` take precedence) | ||
| detectNextIntlConfig(root, resolved); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,6 +102,7 @@ export async function generateServerEntry( | |
| // so prod-server.ts can apply them without loading next.config.js at runtime. | ||
| const vinextConfigJson = JSON.stringify({ | ||
| basePath: nextConfig?.basePath ?? "", | ||
| assetPrefix: nextConfig?.assetPrefix ?? "", | ||
| trailingSlash: nextConfig?.trailingSlash ?? false, | ||
| redirects: nextConfig?.redirects ?? [], | ||
| rewrites: nextConfig?.rewrites ?? { beforeFiles: [], afterFiles: [], fallback: [] }, | ||
|
|
@@ -421,6 +422,17 @@ function collectAssetTags(manifest, moduleIds) { | |
| const tags = []; | ||
| const seen = new Set(); | ||
| // Only prepend assetPrefix when it is an absolute CDN URL (https://, http://, | ||
| // or protocol-relative //). When assetPrefix is a same-origin path like | ||
| // "/docs", Vite already embeds basePath into every emitted asset path, so | ||
| // prepending it again would produce a double-prefix like /docs/docs/assets/. | ||
| var _assetHrefPrefix = (vinextConfig.assetPrefix && | ||
james-elicx marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The CDN-only guard here is the key design decision and it's correct. When The
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The CDN-only guard is the right design decision. One observation: if someone sets In practice this is unlikely — |
||
| (vinextConfig.assetPrefix.startsWith("https://") || | ||
| vinextConfig.assetPrefix.startsWith("http://") || | ||
| vinextConfig.assetPrefix.startsWith("//"))) | ||
| ? vinextConfig.assetPrefix | ||
| : ""; | ||
| // Load the set of lazy chunk filenames (only reachable via dynamic imports). | ||
| // These should NOT get <link rel="modulepreload"> or <script type="module"> | ||
| // tags — they are fetched on demand when the dynamic import() executes (e.g. | ||
|
|
@@ -432,8 +444,8 @@ function collectAssetTags(manifest, moduleIds) { | |
| if (typeof globalThis !== "undefined" && globalThis.__VINEXT_CLIENT_ENTRY__) { | ||
| const entry = globalThis.__VINEXT_CLIENT_ENTRY__; | ||
| seen.add(entry); | ||
| tags.push('<link rel="modulepreload" href="/' + entry + '" />'); | ||
| tags.push('<script type="module" src="/' + entry + '" crossorigin></script>'); | ||
| tags.push('<link rel="modulepreload" href="' + _assetHrefPrefix + '/' + entry + '" />'); | ||
| tags.push('<script type="module" src="' + _assetHrefPrefix + '/' + entry + '" crossorigin></script>'); | ||
| } | ||
| if (m) { | ||
| // Always inject shared chunks (framework, vinext runtime, entry) and | ||
|
|
@@ -508,13 +520,13 @@ function collectAssetTags(manifest, moduleIds) { | |
| if (seen.has(tf)) continue; | ||
| seen.add(tf); | ||
| if (tf.endsWith(".css")) { | ||
| tags.push('<link rel="stylesheet" href="/' + tf + '" />'); | ||
| tags.push('<link rel="stylesheet" href="' + _assetHrefPrefix + '/' + tf + '" />'); | ||
| } else if (tf.endsWith(".js")) { | ||
| // Skip lazy chunks — they are behind dynamic import() boundaries | ||
| // (React.lazy, next/dynamic) and should only be fetched on demand. | ||
| if (lazySet && lazySet.has(tf)) continue; | ||
| tags.push('<link rel="modulepreload" href="/' + tf + '" />'); | ||
| tags.push('<script type="module" src="/' + tf + '" crossorigin></script>'); | ||
| tags.push('<link rel="modulepreload" href="' + _assetHrefPrefix + '/' + tf + '" />'); | ||
| tags.push('<script type="module" src="' + _assetHrefPrefix + '/' + tf + '" crossorigin></script>'); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1269,6 +1269,30 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |
| define: defines, | ||
| // Set base path if configured | ||
| ...(nextConfig.basePath ? { base: nextConfig.basePath + "/" } : {}), | ||
| // When assetPrefix is set (and differs from basePath inheritance), | ||
| // rewrite built asset/chunk URLs to the configured prefix. | ||
| // SSR environments stay relative so server-side imports resolve correctly. | ||
| ...(nextConfig.assetPrefix && nextConfig.assetPrefix !== nextConfig.basePath | ||
| ? { | ||
| experimental: { | ||
| ...config.experimental, | ||
| renderBuiltUrl(filename: string, ctx: { type: string; ssr: boolean }) { | ||
| if (ctx.ssr) return { relative: true }; | ||
| if (ctx.type === "asset" || ctx.type === "chunk") { | ||
| return nextConfig.assetPrefix + "/" + filename; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Subtle detail worth calling out: But in This means JS chunks loaded via |
||
| } | ||
| const userRenderBuiltUrl = config.experimental?.renderBuiltUrl; | ||
| if (typeof userRenderBuiltUrl === "function") { | ||
| return userRenderBuiltUrl( | ||
| filename, | ||
| ctx as Parameters<typeof userRenderBuiltUrl>[1], | ||
| ); | ||
| } | ||
| return { relative: true }; | ||
| }, | ||
| }, | ||
| } | ||
| : {}), | ||
| // Inject resolved PostCSS plugins if string names were found | ||
| ...(postcssOverride ? { css: { postcss: postcssOverride } } : {}), | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -732,6 +732,10 @@ async function startPagesRouterServer(options: PagesRouterServerOptions) { | |
|
|
||
| // Extract config values (embedded at build time in the server entry) | ||
| const basePath: string = vinextConfig?.basePath ?? ""; | ||
| // assetBase is used only for internal manifest path lookups and dedup | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good clarifying comment. This was a source of confusion in the first review (potential dedup mismatch between |
||
| // (e.g. mapping SSR manifest keys to lazy-chunk filenames). It is NOT | ||
| // used to construct href attributes in HTML — those are handled by | ||
| // collectAssetTags in the generated server entry, which applies assetPrefix. | ||
| const assetBase = basePath ? `${basePath}/` : "/"; | ||
| const trailingSlash: boolean = vinextConfig?.trailingSlash ?? false; | ||
| const configRedirects = vinextConfig?.redirects ?? []; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -323,10 +323,18 @@ function collectFontPreloads(options: LocalFontOptions): void { | |
|
|
||
| for (const src of sources) { | ||
| const href = src.path; | ||
| // Only collect URLs that are absolute (start with /) — relative paths | ||
| // Only collect URLs that are absolute or CDN-prefixed — relative paths | ||
| // would resolve incorrectly from different page URLs. The vinext:local-fonts | ||
| // Vite transform should have already resolved them to absolute URLs. | ||
| if (href && href.startsWith("/") && !ssrFontPreloadHrefs.has(href)) { | ||
| // Accept https://, http://, and protocol-relative // URLs for assetPrefix CDN cases. | ||
| if ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix. This was a genuine silent bug — CDN-prefixed font URLs were being dropped because the guard only accepted Minor: |
||
| href && | ||
| (href.startsWith("/") || | ||
| href.startsWith("https://") || | ||
| href.startsWith("http://") || | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a small helper or Not blocking — the current implementation is correct for the |
||
| href.startsWith("//")) && | ||
| !ssrFontPreloadHrefs.has(href) | ||
| ) { | ||
| ssrFontPreloadHrefs.add(href); | ||
| ssrFontPreloads.push({ href, type: getFontMimeType(href) }); | ||
| } | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex
replace(/\/$/, "")strips a single trailing slash, which matches Next.js behavior. Worth noting thatassetPrefix: "https://cdn.example.com//"(double slash) would only strip one, leaving a trailing/. Next.js has the same behavior, so this is consistent.