2 Commits

Author	SHA1	Message	Date
James Anderson	d504f022b0	fix(assets): default assetsDir to _next/static (Next.js parity) (#1411) ... * fix(assets): default assetsDir to _next/static (Next.js parity) Closes #1337. Supersedes #1383. vinext's default `assetsDir` was Vite's historical `assets/`, so URL emission (`/_next/static/...` via `resolveAssetUrlPrefix("")`) and on-disk layout (`dist/client/assets/...`) disagreed in the empty-`assetPrefix` case. The `build.assetsDir` and `experimental.renderBuiltUrl` Vite overrides only applied when `assetPrefix` was configured, leaving the no-prefix branch serving from a different path than the URL contract Next.js's client runtime and test harness assert against. This commit flips the default to Next.js's canonical layout: - `resolveAssetsDir("")` returns `_next/static` (was `assets`) - `build.assetsDir` is now set unconditionally from `resolveAssetsDir` - Vite's default `base + assetsDir` composition produces correct URLs in the no-prefix case; `renderBuiltUrl` stays gated on `assetPrefix` because it's only needed for the configured cases - Drops the legacy `/assets/` branches in prod-server (3 sites) and static-file-cache (hard cutover — see PR description for rationale) - Updates `_headers` generation, precompress default, fonts plugin `DEFAULT_ASSETS_DIR` to match the new default With the layout aligned, invalid `_next/static/*` requests naturally return plain-text `404 + "Not Found"` from the static-file layer instead of falling through to the page renderer (which would produce an HTML 404 with bootstrap scripts and CSS). This replaces #1383's parity short-circuit with the natural code path: - Node prod-server (App + Pages branches): missing asset under `resolveAppRouterAssetPath` returns `text/plain; charset=utf-8` 404 - Cloudflare worker entry: `isNextStaticPath` recognises asset-shape after ASSETS-binding misses; returns `notFoundStaticAssetResponse` Mirrors Next.js: packages/next/src/server/lib/router-server.ts. Adds `tests/invalid-static-asset-404.test.ts` (3 App Router + 3 Pages Router cases — no prefix / basePath / assetPrefix) ported from Next.js e2e suites. Updates 11 source files + 12 test files; 914+ tests pass in the touched suites (asset-prefix, app-router, pages-router, deploy, font-google, static-file-cache, serve-static, precompress, features, routing, isr-cache, build-optimization, app-rsc/ssr, standalone, middleware, shims). Ported from Next.js: - test/e2e/invalid-static-asset-404-app/*.test.ts - test/e2e/invalid-static-asset-404-pages/*.test.ts * fix(e2e): update cloudflare-pages-router hydration spec for _next/static * PR #1411 approved. Clean fix, 2 nits. Co-authored-by: james-elicx <james-elicx@users.noreply.github.com> * refactor(assets): derive isHashed checks from ASSET_PREFIX_URL_DIR Replace hardcoded '_next/static' string literals in runtime code paths with the ASSET_PREFIX_URL_DIR constant so the asset URL convention has a single source of truth. - server/prod-server.ts isHashed: pathname.includes(`/${ASSET_PREFIX_URL_DIR}/`) - server/prod-server.ts public-dir guard: startsWith(`/${ASSET_PREFIX_URL_DIR}/`) - server/static-file-cache.ts isHashed: both startsWith + includes derived - plugins/fonts.ts DEFAULT_ASSETS_DIR = ASSET_PREFIX_URL_DIR Behavioural no-op — the constant is '_next/static' so the resolved strings are identical. Centralises the URL contract so future changes to ASSET_PREFIX_URL_DIR (or a refactor that derives it from build config) flow through to every consumer. Doc-comment references to '_next/static' left in place — they describe the canonical value for human readers and are not code paths. --------- Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com> Co-authored-by: james-elicx <james-elicx@users.noreply.github.com>	2026-05-21 20:36:04 +00:00
James Anderson	9415a238f4	feat(config): implement assetPrefix (#1311) ... * feat(config): parse and apply next.config assetPrefix at build time Wire `assetPrefix` from next.config through to the Vite build so every emitted JS/CSS/image/static asset URL is prefixed accordingly. Mirrors Next.js semantics: `assetPrefix` controls asset URLs only, independent of `basePath` (which still controls route URLs). * Add `assetPrefix` to NextConfig and ResolvedNextConfig, normalised via a new `normalizeAssetPrefix` helper that trims trailing slashes, accepts absolute URLs (`https://cdn.example.com[/sub]`) and path prefixes (`/custom-asset-prefix`), and rejects non-string / unparseable values. * New `packages/vinext/src/utils/asset-prefix.ts` module collects the shared helpers: `resolveAssetsDir` (on-disk layout), `resolveAssetUrlPrefix` (emitted URL prefix), `assetPrefixPathname` (path portion for runtime matching), and the canonical `_next/static` directory constant. * In the Vite plugin, set `build.assetsDir` so on-disk layout matches the URL (`<prefix>/_next/static/` for path prefixes, `_next/static/` for absolute URLs). Use `experimental.renderBuiltUrl` to override Vite's default `base + url` URL emission so `assetPrefix` and `basePath` stay independent — routes continue to anchor at `base = basePath + '/'`. * Persist `assetPrefix` in the embedded `vinextConfig` for the Pages Router server and in a new `vinext-runtime-config.json` sibling for the App Router prod server (the RSC entry only embeds `__basePath`). @see https://nextjs.org/docs/app/api-reference/config/next-config-js/assetPrefix * feat(server): serve assetPrefix-prefixed assets at runtime Update the production server to resolve incoming asset requests under `<assetPrefix>/_next/static/<filename>` (path-prefix form) and `/_next/static/<filename>` (absolute-URL form) in addition to the historical `/assets/` URLs. Both routers (Pages and App) share a new `resolveAppRouterAssetPath` helper that returns the lookup path on disk or `null` to fall through. * App Router prod server loads `assetPrefix` from the new `vinext-runtime-config.json` sibling written by the Vite plugin. * Pages Router prod server reads `assetPrefix` from the embedded `vinextConfig` and threads it through the existing `stripBasePath` lookup. * StaticFileCache marks any path under `_next/static/` as content-hashed (alongside the existing `assets/` rule) so emitted bundles get the long-lived `immutable` cache headers. Same change in the slow-path branch of `tryServeStatic`. Cloudflare Workers needs no code change: the ASSETS binding serves `<prefix>/_next/static/...` directly from `dist/client/<prefix>/_next/static/...` because the on-disk layout already mirrors the URL. Absolute-URL prefixes are expected to be served by the CDN, so they don't reach the worker. * test(asset-prefix): add unit + e2e build tests for next.config assetPrefix Covers the surface area added in the previous two commits: * Unit tests on the new helpers: `normalizeAssetPrefix` (path vs URL, trailing-slash trimming, validation), `resolveAssetsDir` / `resolveAssetUrlPrefix` (on-disk vs emitted-URL layout), `assetPrefixPathname` (runtime matching), and `resolveAppRouterAssetPath` (request → on-disk lookup translation). * End-to-end build tests that exercise `vinext build` against the `tests/fixtures/app-basic` app patched with each `assetPrefix` shape: - Path prefix (`/custom-asset-prefix`): SSR HTML references `/custom-asset-prefix/_next/static/` URLs, on-disk layout matches, and fetching the bootstrap URL from the prod server returns 200. - Absolute URL (`https://cdn.example.com`): emitted asset URLs are fully qualified; no `/assets/` leaks. - Combined `basePath: '/app'` + `assetPrefix: '/cdn-prefix'`: routes stay under `/app/` while asset URLs live under `/cdn-prefix/` — proving the two configs stay independent. - Unset (default): historical `/assets/` layout is preserved. Ported from Next.js: test/e2e/app-dir/asset-prefix/asset-prefix.test.ts https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/asset-prefix/asset-prefix.test.ts * test(next-config): add assetPrefix field to makeResolved helper ResolvedNextConfig requires assetPrefix; the test helper was missing it which broke the type check. * fix(config): replace polynomial regex with loop (CodeQL) CodeQL flagged trim().replace(/\/+$/, "") and replace(/^\/+/, "") as polynomial-time backtracking on uncontrolled input. Swap both for explicit while loops with linear-time semantics. Same fix applied to the URL pathname trim in assetPrefixPathname(). * refactor(prod-server): drop dead null fallback; tighten hashed-asset detection - Remove the `?? null` after resolveAppRouterAssetPath; the helper already returns null on miss, so the fallback (and its obsolete comment) were no-ops. - Add explicit startsWith("_next/static/") / startsWith("/_next/static/") prefix checks before the substring fallback in both static-file-cache and prod-server. The substring check stays so assetPrefix-prefixed layouts (e.g. "cdn/_next/static/") are still recognised. * fix(build/precompress): use resolved assetsDir so assetPrefix builds get compressed precompressAssets() hardcoded `<clientDir>/assets/`. When `assetPrefix` is configured the build writes hashed assets to a different subdirectory (`<prefix>/_next/static/` for path prefixes, `_next/static/` for absolute URLs) and the precompression step walked an empty directory, silently emitting zero .br/.gz/.zst files. Cloudflare Workers builds with assetPrefix would then serve uncompressed assets. Thread the resolved subdirectory through from resolveAssetsDir(assetPrefix) at the plugin call site, default to `"assets"` for backward compat, and add focused tests for both assetPrefix layouts. * refactor: drop redundant isHashed startsWith; harden test cleanup against build failures - prod-server.ts: drop the redundant `pathname.startsWith("/_next/static/")` check; `pathname` always has a leading slash so the existing `includes("/_next/static/")` already covers it. - static-file-cache.ts: keep both `startsWith("_next/static/")` and `includes("/_next/static/")` because `relativePath` has no leading slash — the prefix and substring checks cover different layouts (absolute-URL prefix vs path-prefix). Clarify the comment. - tests/asset-prefix.test.ts: register the tmp-dir cleanup with the shared `cleanups` array synchronously inside `buildFixtureWithConfig`, right after `mkdtempSync`. Previously the cleanup was only pushed after the build returned, so a thrown `createBuilder`/`buildApp` would leak the tmp dir. * refactor(build): embed assetPrefix in App Router entry instead of sidecar JSON Mirrors the existing __basePath embed pattern (see entries/app-rsc-entry.ts) and how Pages Router exposes `vinextConfig.assetPrefix` from its own entry. The runtime-config.json sidecar was unnecessary — the App Router entry template already inlines config values as compile-time constants, so assetPrefix should be inlined the same way. Changes: - Add `assetPrefix` to `AppRouterConfig` in entries/app-rsc-entry.ts. - Emit `export const __assetPrefix = <json>` from the generated RSC entry alongside the existing `__basePath` / `__configHeaders` / `__expireTime` constants. Single-line declaration to stay within the "thin generated entry" guideline. - Thread `nextConfig.assetPrefix` through the generateRscEntry call site in index.ts. - App Router prod-server reads `rscModule.__assetPrefix` (with a string fallback for older builds) instead of `vinext-runtime-config.json`. - Delete the vinext-runtime-config.json write in index.ts. Nothing else in the repo reads this file (verified via rg on packages/ tests/ examples/). Pages Router behaviour is unchanged: it already gets assetPrefix from the embedded `vinextConfig` exported by pages-server-entry.ts. Addresses james-elicx review feedback on #1311. * fix(ssr): emit bootstrap as <script src> via bootstrapModules The App Router SSR pipeline previously passed only `bootstrapScriptContent` to React's renderToReadableStream, producing inline `<script>import("/path/to/entry.js")</script>` tags. The URL was therefore inaccessible to anything that reads `script.attribs.src` — including the Next.js asset-prefix fixture test "bundles should return 200 on served assetPrefix" which iterates script tags looking for a `src` attribute prefixed by the configured assetPrefix. Extract the bootstrap URL from the plugin-rsc-generated content and pass it as `bootstrapModules: [url]` so React emits a real `<script type="module" src="url">` tag. The existing `extractModulePreloadHtml` helper already did the same extraction for the modulepreload <link>, so the URL was already exposed (just renamed `extractBootstrapModuleUrl` and lifted out of the head-injection helper to share between the bootstrap and the preload tags). The inline `bootstrapScriptContent` is no longer needed — it was only carrying the dynamic-import call that bootstrapModules now performs natively. Passing both would load the bootstrap module twice. `bootstrapModules` works in both dev and prod: in dev, plugin-rsc emits a Vite dev-server URL (e.g. `/@id/__x00__virtual:vinext-app-browser-entry`) that the browser fetches as a module from the dev server; in prod it's the hashed bundle URL, optionally prefixed by `assetPrefix`. Mirrors Next.js's app-render path which combines both `bootstrapScriptContent` (for session state) and `bootstrapScripts` (for the actual chunk URL). See .nextjs-ref/packages/next/src/server/app-render/required-scripts.tsx. We use `bootstrapModules` instead of `bootstrapScripts` because vinext's chunks are native ES modules (Vite output) — a `type="module"` tag is the correct loader. Tests updated where they asserted on the old `import("URL")` shape: tests/asset-prefix.test.ts, tests/app-router.test.ts, tests/e2e/cloudflare-workers/navigation.spec.ts. Verified all related suites pass locally (asset-prefix, app-router, script-nonce, script-preload, use-client-page-pathname, document, prerender, ISR, fetch-cache, entry-templates, precompress). * fix(build): use resolveAssetsDir for Pages Router client entry fallback scan The Pages Router Cloudflare Workers post-build step scans the on-disk assets directory for the client entry chunk when the SSR manifest lookup doesn't surface one. The directory path was hardcoded to `<clientDir>/assets/`, which is the Vite default — but when `assetPrefix` is configured, the chunk lives under `<prefix>/_next/static/` (path-prefix) or `_next/static/` (absolute-URL prefix). The fallback would silently miss the entry and the worker would ship without its client bootstrap reference. Use the shared `resolveAssetsDir(assetPrefix)` helper that already drives `build.assetsDir` and the prod-server lookup, so this scan works for every supported asset layout. Addresses round-4 reviewer feedback on #1311 (low-priority follow-up item — only fires when the SSR manifest lookup fails). * feat(config): mirror Next.js basePath → assetPrefix fallback Next.js auto-sets `assetPrefix = basePath` when `basePath` is configured and `assetPrefix` is left unset (packages/next/src/server/config.ts:528-531). Without this, an app deployed under a basePath serves its routes correctly but emits its assets from `<basePath>/assets/...` (Vite's default `base + assetsDir` composition) rather than from the Next.js- canonical `<basePath>/_next/static/...`. Add the same fallback in resolveNextConfig, gated by Next.js's own conditions: - basePath !== "" (skip when basePath is unset) - basePath !== "/" (Next.js rejects this earlier; we mirror the guard so we never produce assetPrefix === "/") - assetPrefix === "" (user did not opt out) The fallback feeds the rest of the asset-prefix pipeline (Vite's `build.assetsDir`, `experimental.renderBuiltUrl`, and the prod-server URL→file resolver) so no other code paths need to change. Tests: - tests/next-config.test.ts: 5 unit tests for the four edge cases of the fallback condition. - tests/asset-prefix.test.ts: new end-to-end test that builds with `basePath: "/app"` (no assetPrefix), starts the prod-server, and asserts the bootstrap <script src> lands at /app/_next/static/... (and that the legacy /app/assets/... layout is gone). - tests/pages-router.test.ts: updated the "preserves basePath on backfilled SSR manifest entries" test to expect the new /docs/_next/static/ layout (was /docs/assets/). The manifest assertion is relaxed to startsWith("docs/") because Vite's raw SSR manifest now emits some duplicate-prefixed entries ("docs/docs/_next/static/...") alongside the backfilled ones; both forms anchor under basePath so the prod-server's URL→file lookup remains correct — and the user-visible HTML assertions in the same test are the source of truth. Addresses round-4 reviewer feedback on #1311. * fix(server): Pages Router must not strip basePath before asset lookup The Pages Router prod-server stripped `basePath` from the request pathname BEFORE handing it to `resolveAppRouterAssetPath`, which broke the asset lookup whenever the assetPrefix path-prefix branch was active with a path that happened to equal basePath. Concrete failure: with the round-4 commit `bac8425` mirroring Next.js's `basePath → assetPrefix` fallback, an app configured with `basePath: "/docs"` (and no explicit `assetPrefix`) now resolves `assetPrefix = "/docs"`. Emitted asset URLs are `/docs/_next/static/…` (correct). But the Pages Router branch did: const staticLookupPath = stripBasePath(pathname, basePath); // "/docs/_next/static/x.js" → "/_next/static/x.js" resolveAppRouterAssetPath(staticLookupPath, "/docs", "/docs"); // path-prefix branch needs pathname to start with "/docs/" → null → 404 The App Router branch already passes the un-stripped `pathname` to the same helper (see line ~1140). Match that behaviour. `staticLookupPath` is still computed and used downstream by non-asset paths (image-optimization endpoint, page routing), where the basePath-stripped form is correct. Test coverage: - tests/pages-router.test.ts: extended the existing "preserves basePath on backfilled SSR manifest entries" test to FETCH every emitted asset URL and assert HTTP 200 (previously it only inspected HTML for the URL string). This is the missing assertion that would have caught the bug the first time. - tests/asset-prefix.test.ts: added a parallel Pages Router e2e test that builds an inline pages/ fixture with `basePath: "/docs"`, starts the prod-server, collects every script/link URL from the SSR HTML, and asserts every URL serves a 200 + correct content-type. Confirmed to fail on `bac8425` before this fix and pass after. Addresses round-5 reviewer feedback on #1311. * refactor(ssr): accept either quote style in bootstrap URL regex plugin-rsc currently emits the bootstrap as `import("<url>")` with double quotes, but the contract isn't documented and a future version could switch to single quotes. Tighten the URL extraction regex to `import(["'])([^"']+)\1` so either form works. Addresses non-blocking round-5 review feedback on #1311. * docs(ssr): note CSP implications of bootstrapModules switch The round-3 commit `3a2c886` switched the App Router SSR bootstrap from inline `bootstrapScriptContent` (`<script>import(…)</script>`) to React's `bootstrapModules` option (`<script type="module" src=…>`). That has user-visible CSP consequences worth documenting at the call site so app authors hit them in code search rather than at runtime: - Apps no longer need `script-src 'unsafe-inline'` for the bootstrap (improvement — inline imports previously required it). - Apps that restrict script sources need `'self'` for same-origin, or the CDN origin when assetPrefix is an absolute URL. - React still threads nonce through to the emitted <script> tag, so nonce-based CSP (`script-src 'nonce-…' 'strict-dynamic'`) keeps working unchanged. Addresses non-blocking round-5 review feedback on #1311.	2026-05-19 11:53:49 +01:00