mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
v0.0.41
482 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Stephen Zhou
|
14ff303cbd |
feat: csp support (#795)
* fix: support csp * tweaks * tweaks * tweaks * tweaks * tweaks * tweaks * clean * tweaks * clean * fix test * tweaks * fix lock * fix test * trigger ci * tweaks * tweaksv0.0.41 |
||
|
James Anderson
|
1e6b7e681d | rebuild lockfile (#804) | ||
|
Divanshu Chauhan (divkix)
|
8d29f76512 |
fix(plugins): track font call ranges during transform (#785)
* fix(plugins): track font call ranges during transform The injectSelfHostedCss function was calling s.overwrite() to transform font calls but not adding the call range to overwrittenRanges. This could potentially cause issues when multiple font calls exist in the same file, as subsequent transforms might not properly detect that a previous call was already processed. Add the call range to overwrittenRanges after each font call transformation to ensure proper tracking. Related to #751 - defensive consistency fix * test(fonts): remove duplicate test and add E2E build test for #751 Removes the near-duplicate unit test 'transforms multiple fonts with identical options patterns' from font-google.test.ts. This test was a duplicate of 'handles multiple font imports in one file' and did not actually reproduce issue #751. Adds a proper integration test in font-google-build.test.ts that uses createBuilder + buildApp to exercise the actual Vite build pipeline where issue #751 occurred. Includes fixture at app-basic/app/font-google-multiple/ with Geist + Geist_Mono fonts matching the original issue reproduction. The actual #751 bug (Unexpected token error) was already fixed in previous commits #719 (double-comma) and #723 (nested-brace). The defensive overwrittenRanges tracking remains in fonts.ts as correct defensive code. All 78 font-related tests pass, plus 291 app-router tests. * style(fonts): fix lint and formatting issues in #751 test - Remove unused beforeAll import - Remove unnecessary Plugin type import and cast - Fix formatting from vp check --fix * style: fix formatting in font-google-multiple layout * refactor(tests): move font-google-multiple to standalone fixture and improve assertions Moves the font-google-multiple fixture from a nested location inside app-basic to a standalone fixture at tests/fixtures/font-google-multiple/. This prevents side effects for other tests that build the app-basic fixture. Updates the build test to remove misleading comments about reproducing #751. The overwrittenRanges.push() fix in fonts.ts is a defensive consistency improvement, not a fix for issue #751. - Removes the nested fixture from app-basic/app/font-google-multiple/ - Creates minimal standalone fixture with package.json and vite.config.ts - Updates test comments to clarify this tests general build validation - Removes incorrect claim about reproducing #751 * fix(tests): address PR review feedback on font transform build test - Fix vite import to vite-plus per repo conventions - Strengthen build test assertions to verify font transform output - Add missing package.json to font-google-multiple fixture - Remove unused vite.config.ts from fixture - Remove #751 references (this is a defensive fix, not a fix for that issue) * chore: update pnpm-lock.yaml for font-google-multiple fixture * fix(tests): handle pre-existing node_modules and correct build output extension in font-google-build test Two issues causing CI failure: 1. EEXIST on symlink: fixture's node_modules could be a real directory (from pnpm install), not just a stale symlink. Use fs.rm with force:true before creating the symlink to handle both cases. 2. Wrong output extension: build produces index.js not index.mjs (likely a Vite 8/Rolldown default extension change). |
||
|
Shorebirdmgmt
|
e854f77bd4 |
Update @clerk/nextjs compatibility: unsupported → partial (#803)
Tested against @clerk/nextjs v7.0.12 with vinext 0.0.40. Runtime-verified that clerkMiddleware, auth.protect, frontendApiProxy, ClerkProvider, and client hooks all work through vinext's middleware runtime. What works: - clerkMiddleware() — runs correctly, authenticates requests - auth.protect() — redirects unauthenticated page requests to sign-in - frontendApiProxy — proxies Clerk Frontend API through the app domain - ClerkProvider — renders with Clerk JS SDK and publishable key - x-clerk-auth-* headers — propagated correctly on responses - Clerk handshake flow — 307 redirect with correct handshake URL What's still WIP: - auth() in Server Components (requires next/headers shim) Closes #800 Co-authored-by: Sam Hill <samhill7738@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Steve Faulkner
|
31b1f9b61c |
fix: validate RequestCookies names before mutating Cookie header (#802)
The next/server RequestCookies class mutates the raw Cookie request header when set() or delete() are called. Unlike the next/headers cookie shim and ResponseCookies, it did not validate cookie names before rewriting the header. Add validateCookieName() checks to RequestCookies.set() and delete(), including array-form delete(). Also adds focused direct tests against the RequestCookies API in server.js so this path stays covered. |
||
|
Steve Faulkner
|
5151201206 |
fix: normalize URL scheme detection for control characters (#801)
Match browser and Next.js handling of control characters in URL schemes so obfuscated values like java\nscript:, java\rscript:, java\tscript:, and leading-NUL forms are still recognized. This keeps vinext's broader data:/vbscript: blocking while adopting Next.js's character-by-character scheme matching for javascript:. Also updates tests to cover embedded control characters and the parser behavior for spaces and zero-width characters before the colon. |
||
|
Steve Faulkner
|
f910e74045 |
fix(app-router): reject cyclic Flight payloads in server actions (#798)
* fix(app-router): reject cyclic Flight payloads in server actions * test(app-router): update generated entry snapshots |
||
|
Nathan Nguyen
|
fc8b0d8a4d |
fix(app-route): merge middleware headers with override semantics (#789)
* fix(app-route): merge middleware headers with override semantics * refactor(app-route): share middleware header merge helper * fix(app-page): apply shared middleware header merge to HTML |
||
|
Nathan Nguyen
|
794fd95c8a |
fix(app-router): apply slot override params to parallel slot segment map (#790)
* fix(app-router): apply slot override params to slot segment map * refactor(app-router): dedupe effective slot params lookup |
||
|
Nathan Nguyen
|
5b44b9cb3b | fix(app-router): use override semantics when merging middleware headers into HTML responses (#788) | ||
|
Stephen Zhou
|
ec0fb3bb23 |
fix: silent IMPORT_IS_UNDEFINED for proxy.ts (#787)
* fix: silent IMPORT_IS_UNDEFINED for proxy.ts * tweaks |
||
|
Nathan Nguyen
|
28980b01f1 |
fix: prevent URL/content mismatch on rapid Pages Router navigation (#782)
* fix: prevent URL/content mismatch on rapid Pages Router navigation Replace the boolean `_navInProgress` guard in navigateClient() with a monotonically increasing navigation ID + AbortController. The old guard silently dropped the second navigation when two push() calls overlapped, leaving the URL bar showing B while page content showed A. Now each navigateClient() call: - Aborts the previous in-flight fetch via AbortController - Checks after every await whether a newer navigation has superseded it - Throws NavigationCancelledError (with .cancelled = true) when stale Callers (push/replace in both hook and singleton, plus popstate) catch the error and emit routeChangeError for cancelled navigations, matching Next.js behavior. routeChangeComplete only fires for navigations that actually render. Also fixes: failed navigations (non-OK response, missing data) now emit routeChangeError instead of silently emitting routeChangeComplete before the hard redirect. * chore: trigger CI * refactor: clarify routeChangeError/hard-nav contract and simplify catch blocks - Add comment to !res.ok path explaining the window.location.href + throw ordering contract: the assignment schedules navigation as a task, so synchronous routeChangeError listeners observe the error before unload. Document the contract requirement: listeners MUST be synchronous. - Simplify all 4 push/replace catch blocks: extract routerEvents.emit() before the instanceof check so the emit is unconditional and the if-branch only determines the return value. Add clarifying comment explaining why cancelled navigations return true (superseded, not failed — matches Next.js behaviour). - Simplify popstate .then() rejection handler: remove redundant if/instanceof guard since both paths emitted identical arguments; add comment noting there is no return-value distinction in a void handler. * test: add signal-aware and stale-first navigation cancellation tests Two new tests for the Pages Router concurrent navigation suite: 1. "abort signal fires when navigation is superseded" — uses a signal-aware fetch mock where the first fetch rejects with AbortError when its signal is aborted. Verifies the AbortError → NavigationCancelledError conversion path end-to-end (i.e. the abort signal integration actually works, not just the assertStillCurrent checkpoint). 2. "stale response arriving first" variant — fetchA (stale) resolves before fetchB (winner). Exercises assertStillCurrent() checkpoints when the stale navigation gets further through the pipeline (past the fetch, into HTML parsing) before being cancelled. Verifies stale data never reaches the DOM. * test: remove Router.events listeners in finally blocks Store each Router.events handler in a named variable before the try block and call Router.events.off() in the finally block. Prevents handlers from accumulating on the module-level routerEvents singleton across the 6 concurrent navigation tests. * fix: address review feedback — defer __NEXT_DATA__ write, extract helper, add hard-nav fallback - Defer window.__NEXT_DATA__ assignment until just before root.render() so a stale navigation can never briefly pollute the global between assertStillCurrent() checkpoints. - Extract runNavigateClient() helper to deduplicate the identical 12-line try/catch block across useRouter().push/replace and Router.push/replace. - Add hard-navigation fallback (window.location.href) for non-cancel errors in both the helper and the popstate handler, fixing a regression where unexpected errors (JSON parse, import failure) left URL and content out of sync. - Add test asserting __NEXT_DATA__ is not stale at the moment routeChangeError fires for a cancelled navigation. * fix: avoid duplicate hard navigation fallback * fix: align popstate hard-nav handling * chore: clarify navigation fallback invariants * chore: tighten navigation regression proofv0.0.40 |
||
|
Nathan Nguyen
|
ccb299ab79 |
fix: move NotFoundBoundary inside Template in per-segment wiring (#784)
* test: assert not-found fallback renders inside template Add a structural element-tree test that verifies NotFoundBoundary is nested inside the Template component (not outside it) in the per-segment wrapping chain. Walks the React element tree to compare component depths, asserting Template appears at a shallower depth than NotFoundBoundary. * fix: move NotFoundBoundary inside Template in per-segment wiring The per-segment wrapping loop was applying NotFoundBoundary after the Template wrap (building bottom-up), which placed it outside the Template in the final tree: Layout > NotFoundBoundary > Template > Error > Page (wrong) When notFound() fires, NotFoundBoundary catches the error and renders its fallback — but because it was outside Template, the Template was no longer in the tree. Next.js's correct order is: Layout > Template > Error > NotFoundBoundary > Page (correct) Fix: move the layoutNotFoundComponent wrapping block to the start of the loop body (before the ErrorBoundary wrap), so it becomes the innermost wrapper. Bottom-up build order is now: 1st wrap: NotFoundBoundary (innermost) 2nd wrap: ErrorBoundary 3rd wrap: Template 4th wrap: Layout (outermost) This matches Next.js semantics: templateValue wraps ErrorBoundary > NotFoundBoundary > Page, and the Template component receives that subtree as children. |
||
|
Nathan Nguyen
|
93ef5ae65e |
fix: interleave templates with layouts at each segment level (#780)
* fix: interleave templates with layouts at each segment level Templates were being grouped and wrapped as a batch separately from layouts, producing: Layout[0] > Layout[1] > Template[0] > Template[1] > Page Next.js nests per segment as: Layout > Template > Error > children, so the correct tree is: Layout[0] > Template[0] > Layout[1] > Template[1] > Page Move template wrapping into the layout loop so each template sits inside its corresponding layout at the same directory level. * fix: align templates array with layouts to prevent index mismatch discoverTemplates() only pushed entries for levels with a template.tsx, producing a sparse array not aligned with the layouts array. When a directory had a layout but no template, templates[i] paired with the wrong layout. Replaced with discoverLayoutAlignedTemplates() (modeled on discoverLayoutAlignedErrors) that walks the same directory levels as layouts and inserts null for missing templates. Updated the generated entry to handle null template entries. Fixes the review feedback on index alignment. |
||
|
Brendan Dash
|
a9d3455487 |
test: regression tests for inherited intercepting route slot deduplication (#721)
* fix: allow inherited intercepting routes to share target patterns * fix: validate intercepting route target consistency per page * refactor: simplify intercept target pattern collection by using Set for deduplication * fixup: use main's inline Set dedup, remove helper export, restore .gitignore * fix(test): pass explicit out dirs and appDir to buildApp to fix UNRESOLVED_IMPORT in RSC build * chore: restore .gitignore to match main --------- Co-authored-by: James <james@eli.cx> |
||
|
Nathan Nguyen
|
3102d17d5c |
fix: extract actual URL params for intercepting route source routes (#783)
* fix: deduplicate inherited intercept target patterns in validation
Child routes inherit parent parallel slots (including their intercepting
routes), so the same target pattern can appear on both parent and child
route. The validateRoutePatterns call for intercept targets was seeing
these duplicates and throwing a false "two routes resolve to same path"
error. Deduplicate via Set before validation.
* fix: extract actual URL params for intercepting route source routes
matchSourceRouteParams was feeding the source route's pattern string
(e.g. "/team/:teamId/members") into matchRoute(), which does a trie
lookup. The trie's dynamic child matched the literal ":teamId" as a
param value, producing {teamId: ":teamId"} instead of extracting the
actual value from the request URL.
Replace with a prefix-match param extractor that walks the source
pattern and the request URL in parallel, extracting dynamic segment
values from corresponding URL positions and stopping when static
segments diverge. This correctly handles (.) same-level, (..) one-
level-up, and (...) root interception conventions.
* fix: handle optional catch-all in source route param extraction
Move the catch-all/rest check before the bounds check so optional
catch-all segments (`:path*`) correctly produce an empty array when
the URL has no remaining segments at that position.
* test: update entry template snapshots for catch-all reorder
|
||
|
Nathan Nguyen
|
a035918c67 |
fix: populate parallel route segments in LayoutSegmentProvider (#781)
useSelectedLayoutSegment(key) always returned null for named parallel route keys (@modal, @sidebar, etc.) because the segment map only contained the "children" key. This adds per-slot routeSegments to the ParallelSlot type and populates them at scan time, then includes them in the LayoutSegmentProvider's segmentMap at each layout level. Ported test cases from Next.js: test/e2e/app-dir/parallel-routes-use-selected-layout-segment |
||
|
Nathan Nguyen
|
d0633207d6 |
fix: emit no-store Cache-Control for revalidate = 0 pages (#777)
Pages exporting revalidate = 0 should be treated as fully dynamic (never cached), matching Next.js behavior. The previous code used truthiness checks on revalidateSeconds, causing 0 to fall through with no Cache-Control header — allowing CDNs to cache dynamic content. Add explicit === 0 guards in both resolveAppPageRscResponsePolicy and resolveAppPageHtmlResponsePolicy, placed before any branch that uses !revalidateSeconds or revalidateSeconds > 0. Also tighten the isForceStatic/isDynamicError condition in the HTML helper from (=== null || === 0) to === null, since 0 is now handled earlier. |
||
|
Nathan Nguyen
|
336289b0ca |
fix: default redirect() to "push" in Server Action context (#779)
redirect() called inside a Server Action always used "replace" navigation, breaking the Back button after form submissions. Next.js defaults to "push" in action context via actionAsyncStorage. Instead of adding a new AsyncLocalStorage, use an empty string sentinel in the digest when no explicit type is provided. The catch site resolves the default based on context: "push" for Server Actions, "replace" for SSR renders. This matches Next.js behavior. Also adds an optional `type` parameter to permanentRedirect() matching the Next.js signature (defaults to "replace"). |
||
|
Nathan Nguyen
|
e4831109c1 |
fix: await buildPageElement in server action re-render path (#778)
The action re-render path was calling buildPageElement() without await, assigning a Promise to element instead of a resolved React element. This caused redirect()/notFound() thrown inside generateMetadata() during action re-renders to become RSC stream errors instead of propagating to the outer catch handler, and caused getAndClearPendingCookies() to run before page tree construction resolved, missing cookies set during page building. Adds a focused test asserting the generated code contains the await so this regression can't silently reappear. Updates snapshots accordingly. |
||
|
James Anderson
|
1a339c7ca4 | chore: bump vulnerable deps (#773) | ||
|
James Anderson
|
7625d4a215 | remove unused rsc-html-stream dep (#771) | ||
|
Nathan Nguyen
|
6f8d37ea09 |
refactor: extract route wiring from generated entry into typed runtime module (#748)
* Extract app page route wiring helpers * Fix app page error boundary serialization * Fix client error boundary pathname reset * Document Next.js error boundary verification * Address bonk review: edge case tests and readability nit - Fix optional catch-all length check readability (> len-1 → >= len) - Add dedicated tests for behavioral changes: strict undefined check on optional catch-all params, nullish coalescing preserving empty strings instead of falling back to raw segment syntax - Add edge case tests: route groups, empty array, undefined dynamic params - Add Next.js source reference comments on pathname-reset error boundary tests (no direct Next.js test equivalent exists) * Address second-round review: type honesty and test cleanup - Widen resolveAppPageChildSegments params to include undefined, matching runtime behavior of Record indexing without noUncheckedIndexedAccess - Fix ErrorBoundaryInnerConstructor test type to match actual Partial<ErrorBoundaryState> return from getDerivedStateFromError - Remove null param test case (impossible at runtime given AppPageParams type, testing unreachable behavior) - Remove vestigial type cast on empty-string test * Add comment explaining template vs layout param asymmetry Templates receive raw params (Next.js doesn't pass params to templates at all), while layouts receive thenable params matching the Next.js 15+ async params contract. |
||
|
James Anderson
|
12c747f686 |
fix: restore baseUrl in hackernews tsconfig to fix build (#770)
Removing baseUrl in the security audit PR (#741) silently broke the hackernews example build: loading.tsx, page.tsx and other app files import components and lib via bare specifiers ('components/skeletons', 'lib/get-comments', etc.) that Vite resolves using the tsconfig baseUrl. Without it Rolldown cannot find those modules and the build fails. Restore 'baseUrl: "."' so Vite continues to resolve those imports correctly. Other examples (app-router-playground, fumadocs-docs-template) already keep baseUrl for the same reason. |
||
|
Stephen Zhou
|
1dc23037b0 |
fix: serve public files in prod (#766)
* fix: serve public files in prod * fix test * resolve comments * update |
||
|
James Anderson
|
49656ed5cf |
fix: security audit findings from issue #741 (#757) (#769)
* fix: security audit findings from issue #741 CI shell injection (high): - publish.yml: use env var for inputs.bump in case statement - nextjs-tracker.yml: use env vars for since_hours and dry_run inputs - nextjs-tracker.yml: use env var in Skip step echo Test file false positive (critical flag): - tests/vite-hmr-websocket.test.ts: add gitleaks:allow for RFC 6455 example WebSocket nonce (dGhlIHNhbXBsZSBub25jZQ==) innerHTML / XSS (high): - packages/vinext/src/shims/script.tsx: add security comment documenting that dangerouslySetInnerHTML is developer-supplied inline script only - packages/vinext/src/shims/head.ts: document tag is bounded to RAW_CONTENT_TAGS (script/style), not user input dangerouslySetInnerHTML in example (medium): - examples/hackernews/components/comment.jsx: sanitize HN API HTML with DOMPurify before rendering - examples/hackernews/package.json: add dompurify dependency - pnpm-workspace.yaml: add dompurify and @types/dompurify to catalog Non-literal RegExp (medium): documented as internal config values only, no user input reaches any of the 6 flagged patterns. See safeRegExp() in config-matchers.ts which already enforces ReDoS protection. Deprecation cleanup: - examples/hackernews/tsconfig.json: remove deprecated baseUrl option (moduleResolution: bundler handles resolution; all imports are relative) - examples/hackernews/tsconfig.json: add noEmit: true to prevent TypeScript from attempting to write over existing .js files in lib/ - examples/hackernews/package.json: add missing @cloudflare/workers-types devDependency (was referenced in tsconfig types but not installed) * fix: address bonk review comments - Switch dompurify → isomorphic-dompurify so sanitize() works during SSR (DOMPurify is a no-op without window/document; isomorphic-dompurify bundles jsdom for server-side use) - Revert package.json whitespace to 2-space indentation (was inadvertently changed to 4-space, creating noise in the diff) - Replace dompurify + @types/dompurify catalog entries with isomorphic-dompurify (ships its own types, no separate @types needed) - Add missing trailing newline to tsconfig.json * fix: address bonk review comments on PR #769 - Switch from isomorphic-dompurify to sanitize-html, and move sanitization to the server-side data boundary (lib/get-comments.js). isomorphic-dompurify depends on jsdom which is Node.js-specific and does not work in the Cloudflare Workers runtime. sanitize-html is pure-JS (htmlparser2-based) with no DOM dependency and works in both Workers and Node.js. Sanitizing once at the data boundary means both the SSR'd initial HTML response and client re-renders receive safe text. - Remove the DOMPurify import from comment.jsx; the client component no longer needs to sanitize since text is already clean when passed as a prop. - Replace isomorphic-dompurify with sanitize-html in the workspace catalog. - Remove inert eslint-disable-next-line comment from script.tsx; the project uses oxlint which ignores ESLint-specific inline suppression directives. --------- Co-authored-by: Luke Percy <lpercyagile@gmail.com> |
||
|
Nathan Nguyen
|
adf8ab5045 |
fix: ResponseCookies deduplicate Set-Cookie headers and add missing API surface (#761)
* fix: ResponseCookies deduplicate Set-Cookie headers and add missing API surface
ResponseCookies.set() unconditionally appended Set-Cookie headers, causing
duplicate headers when the same cookie name was set twice and get() returning
stale values. Rewritten to use an internal Map<name, {serialized, entry}>
as the single source of truth, matching @edge-runtime/cookies.
Also adds missing Next.js API surface:
- set() object form: set({ name, value, httpOnly, ... })
- get() object form: get({ name })
- getAll(name?) / getAll({ name }) filtering
- delete({ name, path, domain }) object form with path/domain propagation
- Constructor hydrates from existing Set-Cookie headers
- delete() now uses Expires=epoch (matching edge-runtime) instead of Max-Age=0
* fix: only forward path/domain in ResponseCookies.delete()
Explicitly extract path and domain instead of spreading the full opts
object, preventing callers from accidentally overriding the tombstone
expires/value fields. Matches @edge-runtime/cookies behavior.
* fix: forward httpOnly/secure/sameSite in ResponseCookies.delete()
Address review feedback:
- delete() now forwards httpOnly, secure, and sameSite attributes
to match @edge-runtime/cookies behavior (browsers need matching
Secure flag to actually delete a cookie)
- Simplify parseCookieSetArgs: always return opts (empty object is
harmless since all CookieOptions properties are optional)
|
||
|
Nathan Nguyen
|
1e2e68f35f |
fix: apply middleware headers to intercept route and server action responses (#762)
* fix: apply middleware headers to intercept route and server action responses Middleware response headers (_mwCtx.headers) were silently dropped on three code paths: intercepting route responses, server action re-render responses, and server action redirect responses. All three constructed Response objects directly without merging middleware headers, unlike the normal page path which goes through buildAppPageRscResponse(). Extract mergeMiddlewareResponseHeaders() from the inline logic in buildAppPageRscResponse() and call it in all three paths. This ensures auth cookies, CORS headers, and security headers set in middleware are applied consistently to all response types. * fix: merge middleware headers before x-action-redirect control headers Codex review correctly identified that mergeMiddlewareResponseHeaders() uses set() for most keys, meaning middleware headers could overwrite the x-action-redirect* control headers if a middleware happened to set them. Fix by merging middleware headers first, then writing the x-action-redirect* headers afterward so they are always authoritative. The framework's own redirect control signals must survive regardless of what middleware sets. * fix: update RSC entry snapshots for middleware headers |
||
|
Nathan Nguyen
|
301b8cb8c8 |
fix: pass thenable params and searchParams to probePage() (#763)
probePage() passed raw null-prototype params instead of thenable params, causing TypeError for any page using the Next.js 15+ async params pattern (await params). The probe also omitted searchParams entirely. Both issues caused the probe to silently fail, defeating its purpose of early notFound() and redirect() detection. The layout probe already used the correct thenable params — the page probe was the only inconsistent path. |
||
|
Nathan Nguyen
|
ea0a134bbc |
fix: clean up request context on stream error in deferUntilStreamConsumed (#760)
deferUntilStreamConsumed's pull() handler had no error path — when the upstream RSC/SSR stream errored mid-consumption, the onFlush callback (clearRequestContext) was never invoked, leaking per-request state. The cancel hook handled client disconnect and the flush hook handled normal completion, but stream errors were uncovered. Adds an error handler that calls once() before propagating the error. |
||
|
Nathan Nguyen
|
9afb1d9842 |
fix: validate redirect status codes in NextResponse.redirect() (#759)
NextResponse.redirect() accepted any numeric status code without validation. Passing an invalid code like 200 silently created a non-redirect response (browser ignores Location header on 200). Now throws RangeError for non-redirect status codes, matching Next.js behavior which throws on invalid codes. Valid codes: 301, 302, 303, 307, 308. |
||
|
Nathan Nguyen
|
1ee0686230 |
fix: preserve multi-valued Set-Cookie headers in route handler ISR cache (#758)
buildAppRouteCacheValue() used headers.forEach() which overwrites same-key entries, causing all but the last Set-Cookie header to be silently dropped from cached route handler responses. Uses getSetCookie() to preserve all Set-Cookie headers as a string array, matching the pattern used elsewhere in the codebase (worker-utils.ts, prod-server.ts). |
||
|
Stephen Zhou
|
bb6dda840c |
fix: standalone dependency resolution (#765)
* fix: standalone dependency resolution * tweaks |
||
|
Divanshu Chauhan (divkix)
|
063bf00be5 |
fix(build): eliminate Vite 8 treeshake.preset warning (#746)
* fix(build): eliminate Vite 8 treeshake.preset warning Add version-gated getClientTreeshakeConfigForVite() function that returns Rollup-compatible config (with preset) for Vite 7 and Rolldown-compatible config (without preset) for Vite 8+. The treeshake.preset option is Rollup-specific and causes warnings in Vite 8 which uses Rolldown. The moduleSideEffects: 'no-external' option is valid in both bundlers and is preserved in all configurations. Changes: - Add getClientTreeshakeConfigForVite(viteMajorVersion) function - Update 4 usage sites to use version-gated function - Mark clientTreeshakeConfig as @deprecated for backward compatibility - Export getClientTreeshakeConfigForVite for testing - Add comprehensive tests for Vite 7/8/9+ compatibility - Update existing tests to expect Vite 8 format Fixes #540 * refactor(build): address PR review feedback for treeshake config Remove dead workaround plugin from playground - Deleted strip-rolldown-incompatible-rollup-options plugin from examples/app-router-playground/vite.config.ts - This workaround is no longer needed since getClientTreeshakeConfigForVite() never emits treeshake.preset for Vite 8+ Add documentation comment about Rolldown defaults - Enhanced JSDoc for getClientTreeshakeConfigForVite() to explain behavior gap - Documents how Rolldown defaults compare to Rollup's recommended preset - Clarifies that moduleSideEffects: no-external is the key optimization Fix test import style - Changed from dynamic await import() to static imports - Matches existing test patterns in build-optimization.test.ts - Removes async from test cases that don't need it Addresses review comments from PR #746 * docs(build): clarify Rolldown treeshake divergence in comments Address PR #746 review feedback from @ask-bonk: - JSDoc: Explicitly call out unknownGlobalSideEffects as a known acceptable divergence (Rolldown defaults to true vs Rollup recommended preset's false). Makes it clear this is intentional, not an oversight. - Inline comment: Note that Rolldown's built-in defaults already cover what Rollup's 'recommended' preset provides (annotations, correctContext, tryCatchDeoptimization). Provides immediate context at the code site. Both changes are documentation-only; no functional impact. * chore: address PR review feedback on treeshake config - Update comment to reference getClientTreeshakeConfigForVite instead of deprecated clientTreeshakeConfig - Remove redundant not.toHaveProperty('preset') assertions from tests (toEqual already does exact matching) Addresses review feedback from @ask-bonk[bot] on PR #745 |
||
|
Nathan Nguyen
|
ba71e640d4 |
feat: add client primitives for layout persistence (Slot, Children, mergeElementsPromise) (#747)
* Add slot client primitives
* fix: address review feedback on slot primitives
- Export Elements type for PR 2c consumers
- Document intentional module-level sharing of EMPTY_ELEMENTS_PROMISE
- Document intentional cache-forever-including-rejections in mergeElementsPromise
- Move console.error mock before renderPromise to close leak window
- Strengthen merge test to verify next wins shallow merge
* fix: prototype pollution in Slot, freeze shared default, improve tests
- Use Object.hasOwn() instead of `in` to prevent prototype chain lookup
- Freeze EMPTY_ELEMENTS_PROMISE resolved value to enforce immutability
- Document mergeCache SSR safety (client-only optimisation, GC-eligible)
- Remove dead vi.mock("next/navigation") (slot.tsx uses relative import)
- Simplify Suspense test: direct deferred.resolve() instead of setTimeout
- Add comments explaining Promise.race suspension check
|
||
|
Divanshu Chauhan (divkix)
|
ed2618a1ae |
refactor: post-PR #690 cleanup - shared test helper and ??= syntax
Addresses review comments from PR #690: 1. Extract waitForHydration helper into shared location (tests/e2e/helpers.ts) - Adds waitForAppRouterHydration() using expect().toPass() pattern - Updates 19 App Router test files to use shared helper - Removes ~100 lines of duplicated code across test suite - Standardizes on Pattern A (expect().toPass) for better error messages 2. Apply ??= operator syntax nit in navigation.ts - Replaces if (!x) { x = {...} } with x ??= {...} - Cleaner, idiomatic TypeScript for lazy initialization Type check and E2E verification passed.v0.0.39 |
||
|
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>
|
||
|
Nathan Nguyen
|
e19ec196c8 |
fix(tpr): use correct KV cache key format matching runtime (#736)
* fix(tpr): use correct KV cache key format matching runtime TPR's uploadToKV wrote keys as `cache:<pathname>` but the runtime KVCacheHandler reads `cache:app:<buildId>:<pathname>:html`. Every TPR-seeded entry was a dead key — guaranteed cache miss on first request. Extract buildTprKVPairs() as a testable pure function that delegates key construction to the shared isrCacheKey(), and read buildId from the prerender manifest at upload time. * fix(tpr): read buildId from BUILD_ID file instead of prerender manifest TPR doesn't use the prerender pipeline, so vinext-prerender.json never exists in TPR context. Write a standalone BUILD_ID file during every build (matching Next.js convention) and read from that instead. * address bonk review comments - Add console.warn when BUILD_ID can't be read so operators can diagnose key mismatches - Match runtime KVCacheHandler TTL strategy: use flat MAX_KV_TTL_SECONDS (30d) instead of 10x revalidate, preventing premature eviction of low-traffic TPR-seeded pages - Export ENTRY_PREFIX from kv-cache-handler.ts and import it in tpr.ts instead of duplicating with a comment - Restrict vinext:build-id writeBundle to rsc environment only (was firing for both rsc+ssr, writing duplicate files) - Guard nextConfig resolution so buildId is generated exactly once per plugin instance across all Vite environments - Update tpr-kv-keys tests to reflect new flat-TTL behaviour * address second bonk review round - Document intentional 24h fallback TTL divergence from runtime for non-revalidating entries - Clarify index.ts guard comment: resolveNextConfig()->resolveBuildId() chain; note fileMatcher etc stay outside guard intentionally - Make missing BUILD_ID a hard failure: skip KV upload entirely instead of proceeding with dead keys - Update no-buildId test to document it exercises buildTprKVPairs() directly; runTPR() now fails fast before reaching that path * fix(build-id): write BUILD_ID once per build via closeBundle, not per-environment The previous writeBundle + envName === 'rsc' guard only wrote BUILD_ID for App Router builds. Pages Router builds (ssr environment) and any future router types were silently skipped. BUILD_ID has nothing to do with which environment is active — it is a per-build property. Replace with a closeBundle hook wrapped in an IIFE that carries a write-once guard (buildIdWritten flag). The output path is always root/dist/server/BUILD_ID, derived from the plugin-level root variable, so it is router-agnostic and matches exactly where tpr.ts reads it from. --------- Co-authored-by: James <james@eli.cx> |
||
|
Nathan Nguyen
|
7d9f4a09b7 |
feat: implement parallelRoutesKey support in useSelectedLayoutSegment(s) (#737)
* feat: migrate LayoutSegmentContext to segmentMap for parallelRoutesKey
Context type changes from string[] to SegmentMap — a record with a
required "children" key plus optional parallel route keys. useChildSegments()
now accepts a parallelRoutesKey parameter to index into the map.
Consumers not yet updated — entry template and boundary rendering still
use the old childSegments prop. Next commit updates them.
* refactor: update all LayoutSegmentProvider consumers to segmentMap
Mechanical change: every createElement(LayoutSegmentProvider, { childSegments })
becomes createElement(LayoutSegmentProvider, { segmentMap: { children: ... } }).
Behavioral output is identical — useSelectedLayoutSegments() returns the
same values. The segmentMap structure is ready for parallel slot entries.
* test: add parallelRoutesKey integration tests
Verifies useSelectedLayoutSegments(parallelRoutesKey) returns correct
per-slot segments through the full SSR pipeline. Flat slots correctly
return [] until PR 2 populates per-slot entries in the segmentMap.
* refactor: remove as cast from useSelectedLayoutSegments
SegmentMap arrays changed from readonly string[] to string[] to match
Next.js public API return type without requiring a forbidden as cast.
The map itself stays Readonly — arrays are created fresh at each layout
boundary so mutable typing is safe.
* Update packages/vinext/src/shims/layout-segment-context.tsx
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
* Update packages/vinext/src/shims/navigation.ts
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
* Update packages/vinext/src/shims/layout-segment-context.tsx
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
---------
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
Co-authored-by: James Anderson <james@eli.cx>
|
||
|
Divanshu Chauhan (divkix)
|
ef4192db49 |
fix: cross-route client navigation hangs in Firefox (#652) (#690)
* fix: cross-route client navigation hangs in Firefox (#652) Replace flushSync-based RSC tree rendering with a two-phase navigation commit that uses startTransition for same-route navigations and synchronous updates for cross-route navigations. This fixes the Firefox hang where startTransition never commits when the entire component tree is replaced, and also resolves the Suspense double-flash from #639. Key changes: - navigation.ts: ClientNavigationState on Symbol.for global for module instance safety, render snapshot context for hook consistency during transitions, RSC response snapshot/restore for visited cache, unified navigateClientSide() entry point, history suppression helpers - app-browser-entry.ts: BrowserRoot component with useState-managed tree, NavigationCommitSignal that defers URL commit to useLayoutEffect, visited response cache with LRU eviction, navigation ID counter for stale navigation bailout, manual scroll restoration - link.tsx: remove duplicated helpers, delegate to navigateClientSide - form.tsx: delegate App Router GET navigation to navigateClientSide Closes #652 * fix: detect React Flight Thenables in updateBrowserTree and apply params eagerly createFromFetch() returns a Thenable (not instanceof Promise), so the async branch was never taken. The raw unresolved Thenable was set as React children, causing suspension without a Suspense boundary and empty content after cross-route navigations. - Duck-type the .then() check to handle both Promises and Thenables - Replace stageClientParams with applyClientParams so useSyncExternalStore subscribers see correct params immediately - Remove dead stageClientParams function and replaceClientParamsWithoutNotify import Fixes useParams E2E failures in CI (hooks.spec.ts:161, hooks.spec.ts:180). * fix: await createFromFetch before passing RSC payload to tree The Flight Thenable from createFromFetch was passed unresolved to renderNavigationPayload/updateBrowserTree. The .then() callback fired unreliably across consecutive navigations, causing empty content. Await createFromFetch in both navigation paths (cached and fresh) so renderNavigationPayload always receives a resolved ReactNode. Add stale navigation checks after the new await points. The HMR handler and server action handler already awaited correctly. * fix: preserve app router params across cached navigation Preserve the X-Vinext-Params header when snapshotting prefetched and visited RSC responses so cached navigations keep dynamic params intact. Stage client params until the URL commit lands so useParams, history state, and redirect handling stay aligned during App Router transitions. * fix: address review feedback on navigation caching and history management - Remove double history push for RSC navigations: let the commit effect in navigateRsc own push/replace exclusively. Also fixes a hidden bug where the early push broke isSameRoute detection (always true), negating the Firefox cross-route hang fix. - Enforce prefetch cache size limits in prefetchRscResponse() by extracting shared eviction logic from storePrefetchResponse(). - Add TTL check in consumePrefetchResponse() so expired snapshots are discarded instead of served as stale UI. - Prevent navigationSnapshotActiveCount leak when async payloads reject in updateBrowserTree by adding error handlers that balance the activate/commit pairing. - Move BrowserRoot setBrowserTreeState assignment from render to useLayoutEffect to avoid side effects under React Strict Mode. - Make visited response cache true LRU (delete+set on hit) and add a 30-minute upper bound for traversal cache hits to prevent arbitrarily stale back/forward content. * fix(navigation): address PR review feedback for Firefox navigation fix Fixes the following review issues from ask-bonk: Major fixes: - Fix prefetch cache eviction order to add entry first, then evict (prevents temporary overflow past MAX_PREFETCH_CACHE_SIZE) - Add error handling in renderNavigationPayload to ensure navigationSnapshotActiveCount is decremented on synchronous errors, preventing counter leaks Minor fixes: - Add cross-reference comments between Pages Router and App Router scroll restoration handlers (navigation.ts and app-browser-entry.ts) - Add JSDoc to getPathnameSnapshot/getSearchParamsSnapshot explaining cached behavior vs live window.location reads - Add comment documenting server action navigation flow (doesn't use activateNavigationSnapshot because it stays on same URL) - Add comment explaining Map iteration safety in resolveCommittedNavigations - Use stripBasePath in isSameRoute comparison for robust basePath handling - Explicitly pass undefined for historyUpdateMode in redirect chains with explanatory comment (prevents double URL push) All 1094 tests pass. * fix: prevent stale navigation bailout from leaking staged params Addresses review feedback on PR #690: - Move stageClientParams() to after final navId check in both cached and fresh paths - Build navigationSnapshot from local params instead of latestClientParams - Move storeVisitedResponseSnapshot after final stale check - Add doc comment about history method patching behavior This fixes a race where cancelled navigations could leak their staged params into subsequent unrelated navigations when commitClientNavigationState() was called later without clearing the pending state. Test: Add E2E test for cancelled slow navigation param leak scenario * fix: remove duplicate onRouterTransitionStart call and fix formatting The refactor to delegate Link navigation to navigateClientSide() left a duplicate notifyAppRouterTransitionStart() call in link.tsx, causing the callback to fire twice per push navigation. * fix(navigation): address code review feedback on Firefox cross-route navigation hang fix Fixes four edge cases identified in code review: 1. NavigationCommitSignal cleanup now resolves pending promises on unmount to prevent hanging callers if React unmounts the component (e.g., error boundary triggers full-page nav) 2. Added dev-mode warning for navigationSnapshotActiveCount going to 0 before decrement to catch activate/commit pairing bugs early 3. Changed prefetch eviction from if to while loop to match visited cache pattern and prevent temporary cache overflow during rapid prefetch calls 4. Moved storeVisitedResponseSnapshot below final navId check to avoid storing stale cache entries from superseded navigations Also added cross-reference comments between the two popstate handlers in app-browser-entry.ts and navigation.ts for future maintainers. * fix(navigation): resolve infinite recursion in getServerSearchParamsSnapshot The function getServerSearchParamsSnapshot had infinite recursion when called on the server - it would call itself infinitely when getClientNavigationState() returned null. Fixed by implementing proper server context handling: - Use _getServerContext() to access server navigation context - Cache ReadonlyURLSearchParams wrapper using symbol-based caching - Return cached empty instance when no context available - Follows established patterns from getServerParamsSnapshot and getSearchParamsSnapshot Fixes CI failures in PR #690 where 18 tests were failing with "Maximum call stack size exceeded" errors during SSR. * style(navigation): fix lint errors from stricter oxlint rules Convert interface declarations to type aliases per consistent-type-definitions rule: - CachedRscResponse, PrefetchCacheEntry, ClientNavigationRenderSnapshot - VisitedResponseCacheEntry in app-browser-entry.ts Add oxlint-disable block comments for react-hooks/rules-of-hooks violations: - useChildSegments, useClientNavigationRenderSnapshot, usePathname - useSearchParams, useParams The original conditional hook pattern (if isServer return early) is intentional and required for SSR/client duality. Server components must not call client-only React hooks. Block-level disable comments suppress the false positives while maintaining the correct runtime behavior. * fix(navigation): address review feedback on Firefox cross-route navigation fix - Fix isHashOnlyChange to strip basePath for consistent comparison with isSameRoute - Add snapshotPending guard to prevent double-decrement in catch block - Document storePrefetchResponse API contract (callers manage prefetchedUrls) - Remove duplicate prefetched.add from _appRouter.prefetch (ownership now in prefetchRscResponse) - Add invariant comment to drainPrePaintEffects explaining activate/commit pairing - Improve test error handling in cancelled navigation test with debug logging * fix: address PR review — prefetch dedup, comments, and test robustness - Add dedup guard to router.prefetch() matching link.tsx pattern (check/add prefetchedUrls before calling prefetchRscResponse) - Fix incorrect comments claiming prefetchRscResponse manages prefetchedUrls - Add doc comment to storePrefetchResponse noting caller owns URL tracking - Document module-level state assumption in app-browser-entry.ts - Add inline comments in drainPrePaintEffects for winning/superseded pairing - Add element visibility assertion in E2E cancelled-navigation test * chore: review changes implemented * Address ask-bonk review feedback: remove dead code and add defensive comments - Remove vestigial response field from PrefetchCacheEntry (never read by consumePrefetchResponse) - Add defensive comment for _snapshotPending guard brittleness - Document isSameRoute race condition during rapid navigations - Update test helper to use snapshot instead of response All changes are non-behavioral documentation/clarity improvements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: prevent double-decrement of navigationSnapshotActiveCount on renderNavigationPayload throw When updateBrowserTree throws synchronously inside renderNavigationPayload, the inner catch already calls commitClientNavigationState() and re-throws. Previously the outer navigateRsc catch saw _snapshotPending === true and decremented again, leaving the counter off by 1 for concurrent navigations. Fix by wrapping each renderNavigationPayload call in try-finally that always clears _snapshotPending, so the outer catch guard correctly skips the already-committed case. * docs: document latestClientParams staging invariant in stageClientParams Per bonk review: stageClientParams diverges latestClientParams from ClientNavigationState.clientParams between staging and commit. Server action snapshots read latestClientParams, so add a comment clarifying this narrow race is acceptable since the commit effect fires synchronously in the same React commit phase. * fix: address two blocking concerns from bonk review 1. Guard outer catch against hard-navigating to stale URL when superseded: - Hoist navId above the try block so it is accessible in the catch - Add 'if (navId !== activeNavigationId) return' before window.location.href - Move navId check before createFromFetch in cached route path to avoid wasted parse work and the same stale-URL risk on throw 2. Guard handleAsyncError in updateBrowserTree against counter corruption: - Add snapshotActivated parameter (default false) to updateBrowserTree - Only call commitClientNavigationState() when snapshotActivated is true - renderNavigationPayload passes true; server actions and HMR pass false (they never call activateNavigationSnapshot so must not decrement) - Update comments on server action and HMR call sites to document why Also add a note on restoreRscResponse that reconstructed Response.url is always empty string — the Response constructor does not accept a url option. * fix: tighten cached-route stale-nav guards and fix stale comment - Add navId check between cachedNavigationSnapshot creation and createFromFetch in the cached route path, so superseded navigations bail out before the async parse rather than only after - Fix stale line reference in stageClientParams comment (533 -> 544/558) - Document slow-route fixture delay / click timeout coupling in E2E test so future changes to the fixture delay don't silently break the test * fix: stage params after _snapshotPending=true in both nav paths, fix stale comment Move stageClientParams() to after _snapshotPending = true in both the cached-route path and the non-cached path. This makes the intent clearer: params are staged as part of an in-flight snapshot, not as a standalone side-effect before the snapshot flag is set. If renderNavigationPayload throws synchronously, its inner catch calls commitClientNavigationState() which flushes pendingClientParams — committing params for a route that never rendered. The hard-nav fallback in the outer catch resets everything, so this was self-correcting, but reordering makes the relationship explicit and guards the intent. Also replace the stale "lines 544/558" reference in the stageClientParams comment with the function name (registerServerActionCallback) so it doesn't rot as the file evolves. * fix: expand module-singleton comment, remove redundant stale-nav check, document BASE URL - Expand the module-level state comment to call out Map fragmentation as the most severe consequence of multiple instances: pendingNavigationCommits and pendingNavigationPrePaintEffects would split across instances, causing drainPrePaintEffects to miss effects queued by the other instance and permanently leaking navigationSnapshotActiveCount. - Remove the redundant navId !== activeNavigationId check between createClientNavigationRenderSnapshot (synchronous) and createFromFetch (async). The pre-check before createClientNavigationRenderSnapshot is sufficient; the meaningful post-check is after the await of createFromFetch. Added a comment noting the function is synchronous so no recheck is needed. - Add a comment to BASE in navigation-regressions.spec.ts explaining it follows the established repo convention (same pattern in navigation.spec.ts, server-client-only.spec.ts, after.spec.ts, etc.). * docs: annotate stageClientParams throw path in both nav paths If stageClientParams throws (e.g. a param object with a throwing getter), _snapshotPending is true but renderNavigationPayload was never called. The outer catch then calls commitClientNavigationState() which decrements a counter never incremented by activateNavigationSnapshot() — safe due to the > 0 guard — but pendingClientParams and hasPendingNavigationUpdate could persist until the hard-nav fallback resets JS state. Add a brief NB comment on both stageClientParams call sites noting that the outer catch hard-navigates, which resets all JS state, making this benign in practice. * fix: store visited response snapshot only after successful render Move storeVisitedResponseSnapshot to after the renderNavigationPayload try-finally block so we only cache RSC responses from navigations that actually rendered successfully. Previously, the cache write happened before renderNavigationPayload — if it threw, a future back/forward navigation could replay a snapshot from a navigation that never rendered. The hard-nav fallback in the outer catch made this self-correcting in practice, but caching only on success is the correct invariant. Also expand the cached-path navId comment to acknowledge the style asymmetry vs the non-cached path, explaining why no extra check is needed between createClientNavigationRenderSnapshot (synchronous) and createFromFetch. --------- Co-authored-by: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
f99ff7afa8 | chore: try different opencode version for bonk (#738) | ||
|
James Anderson
|
bd2e41f848 | chore: turn on pnpm supply chain rules (#729) | ||
|
James Anderson
|
ef1943ce5d | chore: add more gitignore entries (#725) | ||
|
James Anderson
|
d60c8107bc |
fix(fonts): handle nested-brace options objects in self-hosting transform (#723)
* fix(fonts): handle nested-brace options objects in self-hosting transform
namedCallRe and memberCallRe used \{[^}]*\} which stops at the first
closing brace, so calls like Inter({ axes: { wght: 400 } }) were silently
skipped and fell back to CDN delivery.
Replace both regexes with a lookahead-only pattern plus a
findBalancedObject() helper that tracks brace depth and skips string
literals, correctly extracting the full options object regardless of
nesting depth.
Adds two regression tests: one for the named-import path and one for the
default-import proxy (namespace member) path.
* fix(fonts): address bonk review — template literal interpolations, dedup, hoisting
- Fix _findBalancedObject to correctly skip ${...} interpolations inside
template literals. The previous backtick skipper treated any '}' as the
end of the string, which would prematurely exit for values like
`val ${nested}`. Now tracks an exprDepth counter inside interpolations.
- Extract _findCallEnd(code, objEnd) helper that skips whitespace and
returns the index after ')'. Removes ~20 lines of duplicated logic
between the named-call and member-call branches.
- Hoist both helpers to module level (top-level exported functions) so
they are not recreated on every transform() call.
- Add tests: integration test for string values containing brace chars
(Inter({ label: "font {bold}" })), plus unit describe blocks for
_findBalancedObject and _findCallEnd covering string/template/escape/
interpolation/whitespace/null edge cases.
|
||
|
Sankalp Mukim
|
f6aedd3615 |
Add standalone self-host output and align init production scripts (#178)
* feat: add standalone self-host output for vinext build Generate dist/standalone from output: 'standalone' with a runnable server entry and runtime deps, and align init scripts with vinext build/start for production self-host workflows. * fmt * Address bonk review: fix duplicate PHASE constant, complete lazy-chunks extraction, remove dead imports, add process.exit to standalone path * Address bonk nits: document regex patterns, move PHASE re-exports to bottom, add assets field to BuildManifestChunk * refactor(standalone): use Vite bundle graph for server externals instead of regex scan Replace the fragile post-build regex scan of emitted JS files and the package.json#dependencies over-seeding with a new vinext:server-externals-manifest Vite plugin. The plugin collects chunk.imports + chunk.dynamicImports from the SSR/RSC writeBundle hook and writes dist/server/vinext-externals.json — the authoritative, compiler-derived list of packages left external in the server bundle. emitStandaloneOutput now reads this manifest as its sole seed for the BFS node_modules copy, so standalone output only includes packages the server actually imports at runtime. Removed collectServerExternalPackages, walkFiles, the four regex constants, and the package.json deps seeding. * fix(standalone): copy vinext runtime deps, add node_modules filter to cpSync, remove unused pendingWrites * fix(standalone): address round-6 bonk nits — explicit root, manifest comment, pre-flight check, bare-specifier comment, export-from re-export * fix(standalone): address round 7 bonk review comments - harden outDir resolution in server-externals-manifest: replace fragile walk-up heuristic with direct basename check (ssr -> dirname, else dir), avoiding misfires when a user's project path contains a 'server' segment - remove redundant 'chunk' alias in server-externals-manifest writeBundle; use 'item' directly after the type guard narrows it to OutputChunk - add node_modules filter to vinext dist/ cpSync in standalone.ts, matching the same defensive filter used for app package copies - rewrite standalone server.js as pure ESM using import.meta.dirname (Node >= 21.2, vinext requires >= 22); drop CJS require/__dirname and switch standalone package.json to 'type':'module' - add explanatory comment for DEFAULT_PHASE constant in next-config.ts - add comment in cli.ts noting pre-flight path and resolveVinextPackageRoot must stay in sync * fix(standalone): address round 8 bonk review comments - fix test import: use 'vite-plus/test' instead of 'vitest' in standalone-build.test.ts, matching the convention used by every other test file in the repo (documented in AGENTS.md) - extract resolveVinextPackageRoot into utils/vinext-root.ts so cli.ts and standalone.ts share a single source of truth; removes the coupling comment added in round 7 and the duplicated path traversal logic - add node_modules filter to dist/client and dist/server cpSync calls for defensive consistency with the vinext dist/ and package copies * interface -> type * fix(standalone): address round 9 bonk review comments - add node_modules filter to public/ cpSync for defensive consistency with all other cpSync calls in emitStandaloneOutput - warn on malformed vinext-externals.json instead of silently returning an empty list; includes the error string to aid debugging - add doc comment on copyPackageAndRuntimeDeps clarifying that the return value is the full accumulated set (including pre-existing entries), not only packages copied by the current call - document HOST (not HOSTNAME) env var in README standalone section, with a note explaining the difference from Next.js standalone behaviour * fix(standalone): address round 10 bonk review comments - add comment in writeStandaloneServerEntry explaining why the import of vinext/server/prod-server is intentional as a static import: it is a documented exports-map entry, always present in the copied dist tree, and a static import gives a clearer ERR_MODULE_NOT_FOUND at startup - generalise outDir sub-directory detection in server-externals-manifest: replace basename === 'ssr' with basename !== 'server' so any future sub-directory environment (e.g. 'edge') is handled without code changes - add JSDoc on runtimeDeps clarifying that it returns both dependencies and optionalDependencies * fix(next-config): drop PHASE_DEVELOPMENT_SERVER from re-export No external consumer imports it from next-config.ts — only PHASE_PRODUCTION_BUILD is used externally (by cli.ts). Removing it minimises the public surface area of the module. --------- Co-authored-by: GPU VM <copilot-gpu-vm@example.com> Co-authored-by: James <james@eli.cx> |
||
|
Nathan Nguyen
|
33dfe26148 |
perf: optimize resolveParentParams lookups (#718)
* test: add direct unit tests for resolveParentParams * perf: optimize resolveParentParams lookups - Use Map<pattern, route> index for O(1) route lookups (was O(N) linear scan) - Use pre-computed patternParts instead of re-splitting pattern string - Pre-compute last dynamic segment index (was per-iteration slice+some) * test: add edge case tests for resolveParentParams Cover fully static routes, single-segment dynamic routes, and catch-all child segments. * refactor: simplify resolveParentParams internals - Remove dead ParentSegment.params field (allocated but never read) - Replace slice+join with incremental prefix string building - Export StaticParamsMap type so tests derive from source - Improve comment to explain WHY last segment is excluded * refactor: address bonk review nits in resolveParentParams - Move StaticParamsMap type above JSDoc block so the function comment sits directly above resolveParentParams (was erroneously separating them) - Declare 'part' before building prefixPattern so it's used for both the prefix string concatenation and the startsWith check, avoiding redundant array indexing --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
a7dd2d56af | fix(fonts): double-comma syntax error when font options have a trailing comma (#719) | ||
|
James Anderson
|
a83be594f9 |
chore: turn on lint rule to prefer type over interface (#716)
* chore: turn on lint rule to prefer `type` over `interface` * fix vite-hmr.d.ts |
||
|
James Anderson
|
2245429c38 |
chore: turn on no-explicit-any lint rule (#715)
* chore: turn on no-explicit-any lint rule * Apply suggestions from code review Co-authored-by: James Anderson <james@eli.cx> |
||
|
James Anderson
|
1ff166eb92 |
chore: turn on more lint rules (#714)
* chore: turn on more lint rules * fmt * add todos |