3 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
Nathan Nguyen	6da45a680a	perf: build-time precompression + startup metadata cache for static serving (#641) ... * perf: add build-time precompression for hashed static assets Generate .br (brotli q11), .gz (gzip l9), and .zst (zstandard l19) files alongside compressible assets in dist/client/assets/ during vinext build. These are served directly by the production server, eliminating per-request compression overhead for immutable build output. Only targets assets/ (hashed, immutable). Public directory files still use on-the-fly compression since they may change between deploys. * perf: add startup metadata cache for zero-syscall static serving StaticFileCache walks dist/client/ once at server boot and caches: - File metadata (path, size, content-type, cache-control, etag) - Pre-computed response headers per variant (original, br, gz, zst) - In-memory buffers for small files (< 64KB) for res.end(buffer) - Precompressed variant paths and sizes Per-request serving is Map.get() + res.end(buffer) with zero filesystem calls, zero object allocation, and zero header construction. Modeled after sirv's production mode but with in-memory buffering for small files which eliminates createReadStream fd overhead. * perf: refactor tryServeStatic to async + cache + precompressed serving Refactor tryServeStatic to use StaticFileCache for the hot path: - Pre-computed response headers (zero object allocation per request) - In-memory buffer serving for small precompressed files - 304 Not Modified via ETag + If-None-Match - HEAD request optimization (headers only, no body) - Zstandard serving (zstd > br > gzip > original fallback chain) - Async filesystem fallback for non-cached files (replaces blocking existsSync + statSync) - Skip decodeURIComponent for clean URLs (no % in path) Wire StaticFileCache.create() into both startAppRouterServer and startPagesRouterServer at startup. Integrate precompressAssets() into the vinext build pipeline. CONTENT_TYPES is now a single source of truth exported from static-file-cache.ts (was duplicated in prod-server.ts). * docs: fix stale comments in precompress (mention .zst) * fix: deduplicate buffer reads for HTML alias entries, fix stale JSDoc * docs: fix stale comment in cli.ts (mention zstd) * fix: address code review findings - Add Vary: Accept-Encoding to original variant when compressed siblings exist - Use pipeline() instead of .pipe() for stream error safety - Remove legacy accessor fields from StaticFileEntry (all new code) - Exclude Content-Type from 304 responses per RFC 9110 - Fix misleading Accept-Encoding comment in prod-server - Batch stat() calls in walkFilesWithStats (chunks of 64) - Bound precompression concurrency to availableParallelism (max 16) - Gracefully skip zstd when zlib.zstdCompress unavailable (Node <22.15) - Add --no-precompress CLI flag to skip build-time compression - Drop brotli to q5 (fast, nearly same ratio as q11 at build time) - Max out zstd at level 22 (build time, no reason to hold back) * fix: address remaining review findings in prod-server - Gate negotiateEncoding zstd on runtime availability (HAS_ZSTD) - Add ZSTD_e_flush for streaming mode (progressive SSR decodability) - Serve HEAD without body on slow path (compressed + uncompressed) - Handle pipeline errors instead of silently swallowing them - Lowercase Accept-Encoding in fast path per RFC 9110 - Fix stale comments: add zstd to module header and variant detection - Add tests for slow path, HEAD, and malformed percent-encoded pathnames * perf: move precompression to Vite plugin with edge auto-detection Move build-time precompression from CLI post-step into a writeBundle plugin hook (vinext:precompress). The plugin already knows the deployment target — auto-skips for Cloudflare Workers and Nitro where the CDN handles compression at the edge. - Add precompress option to VinextOptions: 'auto' | boolean - CLI --no-precompress forwards via VINEXT_NO_PRECOMPRESS env var - Works for any build invocation (CLI, API, CI), not just vinext build * docs: fix inaccurate comments in precompress and static-file-cache - precompress: remove false idempotency claim (overwrites, not skips) - static-file-cache: "frozen objects" → "reusable objects" (no freeze) - static-file-cache: fix unrealistic compression ratio example * fix: address code review findings - Remove dead code: redundant .br/.gz/.zst extension checks already caught by COMPRESSIBLE_EXTENSIONS filter - Fix unreachable pipeline error handlers: res.writeHead() was called before pipeline(), so res.headersSent was always true in the callback. Use res.destroy(err) to cleanly tear down the socket on stream errors. - Read file content lazily inside compression chunks instead of loading all files upfront, keeping peak memory at O(CONCURRENCY * max_file_size) * perf: tune compression levels and restore auto mode - Drop zstd from level 22 to 8 (250x faster build, 7% larger output) - Drop gzip from level 9 to 8 (marginal size difference, faster) - Restore precompress: 'auto' | boolean config with auto as default (auto-enables for Node.js targets, skips for Cloudflare/Nitro) - Add progress bar for precompression step (matches prerender UX) - Clarify compression ratio output as brotli-specific * fix: use filename hash for ETag on hashed assets Hashed assets (assets/app-DqZc3R4n.js) now derive their ETag from the content hash in the filename instead of mtime. More robust across Docker builds, CI artifacts, and cp -r where mtime changes between deploys even when content is identical. - Use weak validator (W/) since the tag is shared across content-encoded variants (.br, .gz, .zst) which are byte-different (RFC 9110 §8.8.1) - Fall back to mtime-based ETag when filename has no recognizable hash (custom assetFileNames, plugin-emitted assets, public/ files) * fix: make precompress opt-in per maintainer feedback Real-world benchmark shows 13.5% build-time regression on fumadocs with auto mode (2.4s → 2.7s). Switch to explicit opt-in via `precompress: true` or `--precompress` CLI flag until the feature stabilises. * fix: address code review findings - ETag regex: use lastIndexOf('-') to extract only the final hash segment, avoiding capture of filename fragments in multi-hyphenated names (e.g. `before-pop-state-D3R3Ien6` now extracts `D3R3Ien6`) - Progress bar: track processed count (including skipped files) so the bar always reaches 100% - Tests: update ETag test for filename-hash behavior, add coverage for mtime fallback on non-hashed and unhashed-in-assets files * chore: use type instead of interface per lint rule * fix final precompression review comments * perf: optimize precompress build path - Overlap precompression with SSR build phase (async writeBundle + closeBundle await) - Lower concurrency cap from 16 to 8 (reduces zlib queue contention) - Fix progress counter race in chunked processing * fix: address final review feedback - Fix progress counter race in precompress (single onProgress call site after readFile, monotonic by construction) - Add console.error before res.destroy in all pipeline error handlers for operational visibility - Chunk buffer reads at 64 concurrent to bound FD usage on large projects - Defer createCompressor until after HEAD check on slow path - Use Math.floor for mtime ETag (matches send/Express convention) - Document Rolldown hash length assumption * fix: address final bonk review nits - Move progress reporting to after chunk completion to avoid non-deterministic ordering within Promise.all - Use Math.trunc instead of Math.floor for mtime-based ETags (semantically correct for truncating to whole seconds) * address bonk review comments: counters post-write, Math.floor, Content-Length comment, warn log level - precompress.ts: move filesCompressed++ and totalOriginalBytes+= to after writes complete so partial failures (e.g. ENOSPC) don't inflate totals - precompress.ts: add comment clarifying that progress tracks all files including skipped ones below MIN_SIZE, unlike filesCompressed - static-file-cache.ts: Math.trunc -> Math.floor for mtime-based ETags (matches send/Express convention; correct for pre-epoch mtimes) - prod-server.ts: add comment explaining Content-Length is intentionally omitted on slow-path compressed responses (chunked transfer encoding) - prod-server.ts: downgrade all three pipeline error logs from console.error to console.warn and add comments explaining the destroy-only pattern * address bonk: slow-path ETag+304 parity, visible precompress error log - prod-server.ts slow path: add mtime-based ETag (W/"size-mtime") and 304 Not Modified support, matching fast-path cache behaviour; also propagate mtimeMs through resolveStaticFile/statIfFile - index.ts: add console.error in precompress .catch() so build errors are visible even if the SSR closeBundle hook never fires * address bonk: slow-path 304 Vary header, HAS_ZSTD comment - prod-server.ts slow path: include Vary: Accept-Encoding in 304 responses when the content type is compressible, matching fast-path notModifiedHeaders behaviour and preventing shared caches from collapsing compressed/uncompressed representations - prod-server.ts fast path: add comment explaining HAS_ZSTD is not checked during precompressed variant selection — we're serving a pre-existing .zst file from disk, not invoking zstdCompress() at runtime; HAS_ZSTD only guards the slow-path on-the-fly compressor * address bonk: clarify Vary/compress=false intent in slow-path 304 comment Expand the 304 comment to explain that Vary: Accept-Encoding is intentionally omitted when compress=false (matching fast-path behaviour where compress=false skips all compressed variants), and note that spreading undefined is a deliberate no-op per ES2018+. * address bonk: readFile-before-stat comment, document VINEXT_PRECOMPRESS env var - precompress.ts: add comment on MIN_SIZE early-return explaining the read-first vs stat-first tradeoff (one wasted read vs extra syscall per tiny file; read-first cheaper for typical hashed asset sets) - index.ts: document VINEXT_PRECOMPRESS=1 env var in VinextOptions.precompress JSDoc alongside the existing --precompress CLI flag mention * address bonk: filename-hash ETag on slow path, entry-level dedup in buffer pass - static-file-cache.ts: export etagFromFilenameHash so prod-server can reuse the same filename-hash extraction logic - prod-server.ts: slow path now uses etagFromFilenameHash for /assets/* files, falling back to mtime-based ETag for non-hashed files; this matches fast-path cache behaviour and makes ETags stable across deploys - static-file-cache.ts buffer pass: deduplicate at the StaticFileEntry level (using a seenEntries Set) rather than at the variant path level; HTML aliases share the same entry by reference so the old approach iterated variants multiple times for sites with many HTML pages * address bonk: fix etagFromFilenameHash arg, explicit re-run safety comment - prod-server.ts: use resolved.path (not pathname) for etagFromFilenameHash so that ext and the hash extraction always come from the same file; pathname and resolved.path diverge after HTML fallback (e.g. a request for /assets/widget-abc123 resolving to widget-abc123.html would have extracted a hash from the wrong name) - precompress.ts: add inline comment on the COMPRESSIBLE_EXTENSIONS filter making explicit that .br/.gz/.zst are intentionally absent, so precompressed variants from a previous run are never re-compressed * address bonk: add slow-path 304 test coverage --------- Co-authored-by: James <james@eli.cx>	2026-04-01 23:33:12 +01:00