mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
codex/cacheability-platform-io
1925 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Divanshu Chauhan (divkix)
|
db57d45070 |
Shim next/root-params (#941)
* fix(app-router): shim next root params * fix: address review feedback * address review feedback * test: update snapshots for root params cleanup change |
||
|
Divanshu Chauhan (divkix)
|
f972d45034 |
fix: resolve symlink paths in standalone BFS package copy (#724) (#942)
* fix: resolve symlink paths in standalone BFS package copy (#724) When resolvePackageJsonPath finds a package via the lookup-paths fallback, the returned path may be a symlink (e.g., under a hoisted node_modules/ directory). Calling createRequire() on a symlink path causes Node module resolution to miss transitive dependencies because it walks up from the symlink ancestor directories rather than the real (e.g., .pnpm/) path. Fix: call fs.realpathSync() on the resolved package.json path before using it for createRequire() and readPackageJson(). Also adds E2E test coverage for the standalone output mode. * fix: address review feedback Add standalone-output to CI E2E matrix, rename server header test to reflect what it actually asserts, fix unused req parameter. |
||
|
Nathan Nguyen
|
16cb9ee5ef |
fix(app-router): return 405 for non-action mutations to static pages (#940)
* fix(app-router): return 405 for non-action mutations to static pages Static and SSG App Router page routes currently render HTML or RSC payloads for POST and PUT requests that are not server actions. That diverges from Next.js and makes unsupported mutation methods look successful. The App Router page render path only handled server actions and route handlers before falling through to page rendering. Add a small page-method policy helper and call it from the generated RSC entry before page rendering so static and SSG candidates return 405 with Allow: GET, HEAD while possible server action POSTs still pass through. Tests cover the helper policy and the live App Router fixture for static and ISR pages. * fix(app-router): preserve 405 allow header Set the App Router page 405 Allow header after merging middleware response headers so middleware cannot clobber the methods advertised by the generated Method Not Allowed response. Add a focused regression test that keeps middleware header propagation while asserting Allow remains GET, HEAD. |
||
|
Nathan Nguyen
|
e9139542cb |
fix(app-router): reject middleware control responses in route handlers (#939)
* fix(app-router): reject middleware control responses in route handlers App Route handlers currently treat NextResponse.next() and NextResponse.rewrite() as ordinary 200 responses. That diverges from Next.js, where those helpers are middleware control-flow signals and are rejected after a route handler returns. The route-handler execution path now validates returned responses before cache policy or response finalization runs. Focused unit and integration tests cover the invalid next and rewrite helper responses. * Update tests/app-route-handler-execution.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update packages/vinext/src/server/app-route-handler-response.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(app-router): validate route handler ISR regeneration responses Stale App Route handler regeneration called runAppRouteHandler directly and could serialize a middleware control response into the ISR cache. That bypassed the validation used by the live route-handler execution path. Run the same response assertion before background regeneration writes a new APP_ROUTE cache entry, restore exact Next.js error wording for parity, and cover the x-middleware-next boundary semantics. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Nathan Nguyen
|
ebeb044e01 |
fix(app-router): flush server-inserted HTML during SSR streaming (#926)
App Router SSR rendered useServerInsertedHTML callbacks only once before returning the HTML stream. CSS-in-JS registries registered at shell render were cleared before later Suspense chunks, so styles collected after the shell never reached streamed HTML. Keep callback registrations alive for the stream lifetime and let the tick-buffered HTML transform request insertion HTML on each flush after the initial head injection. This matches Next.js per-chunk server inserted HTML transform and preserves cleanup on stream completion or cancellation. Adds regression coverage for persistent callback rendering and a Next.js-ported Suspense CSS-in-JS streaming order case. |
||
|
Nathan Nguyen
|
bee7e6b654 |
fix(router): reject invalid app route discovery conflicts (#928)
* fix(router): reject invalid app route discovery conflicts App Router route discovery currently lets upward interception markers climb past the normalized root and relies on generic duplicate-pattern validation when a page and route handler share the same app route. The scanner now validates visible interception climb depth before resolving the target, and reports an explicit page/route conflict before running generic route pattern validation. Regression coverage captures the Next.js parity cases for root interception markers, route-group-transparent roots, two-level upward markers, and page plus route handler conflicts. * fix(router): address app route validation review |
||
|
Nathan Nguyen
|
0b6eee2221 |
fix(app-router): restore hash anchors on history traversal (#929)
* fix(app-router): restore hash anchors on history traversal App Router hash-only entries can be pushed without saved vinext scroll state while vinext has browser scroll restoration set to manual. On forward traversal, the URL moves back to the fragment but the browser no longer performs native anchor scrolling, so the page can remain at the top. Restore saved numeric scroll state first, then fall back to Next-compatible hash target scrolling when no vinext scroll state exists. The regression test drives a hash-only Link entry through back and forward traversal. * test(app-router): cover hash scroll edge casesv0.0.45 |
||
|
Nathan Nguyen
|
d395994d2e | fix(cache): serve stale unstable_cache entries (#932) | ||
|
Nathan Nguyen
|
e9c4fb75a3 |
test(app-router): stabilize RSC fetch error redirects (#925)
The RSC fetch error E2E used direct private navigation calls and Playwright route mocking for a redirect chain. Those assumptions are timing-sensitive and can skip the intended RSC request under parallel Playwright runs. Drive the cases through fixture links and serve the redirect-to-500 chain from middleware so the test observes real browser fetch behavior. This keeps coverage for hard navigation to the post-redirect URL while removing the route redispatch dependency. |
||
|
Nathan Nguyen
|
c80c756c11 |
fix(app-router): keep ownerless URL commits from releasing snapshots (#930)
Ownerless URL sync commits from hash navigation and patched history methods could decrement an active client navigation snapshot. That let hooks stop reading the pending render snapshot before the transition committed, briefly exposing stale committed URL state. The snapshot release invariant was tied only to commit count underflow, not to ownership of the activated snapshot. Only navigation-owned commits release snapshots now, while App Router superseded-transition cleanup opts into snapshot release without taking pending pathname ownership. A regression test covers external history.pushState during an active pending render snapshot. |
||
|
Nathan Nguyen
|
ae45ff2cf2 |
fix(router): ignore pre-root parallel slots for routed layouts (#927)
Routes whose first layout lives below app/ currently inherit slots discovered before that layout and clamp them to layout index 0. That incorrectly passes top-level parallel slots into a separate root layout. Keep the pre-layout sentinel while scanning ancestors, skip those slots once the route has a real layout, and preserve the existing layout-less scanner metadata path for separate validation work. Add routing tests for the pre-root leak and the valid in-subtree slot case. |
||
|
Nathan Nguyen
|
67e963bfa6 |
fix(cache): scope use cache entries by build id (#924)
* fix(cache): scope use cache entries by build id Shared "use cache" entries currently key only on the cached function id and serialized arguments. That lets a persistent cache handler reuse serialized results after a deploy when the generated function id and arguments remain stable. The violated invariant is that a cache entry belongs to the build that produced its function body, matching Next.js buildId key material. Include __VINEXT_BUILD_ID in the shared cache key when it is available, preserving the existing key shape for environments without a build id. Add a regression test that proves two builds with the same function id do not share the stale value. * fix(cache): capture use cache build id at registration Shared "use cache" wrappers should derive their build scope from the bundle that registered the cached function, not from a fresh environment lookup on every invocation. The prior implementation still produced build-scoped keys, but it kept the build id read inside the hot wrapper path and allowed a mutated environment to change the key for an already-registered function. Resolve the build id once in registerCachedFunction and close over it when constructing cache keys. Keep the direct process.env.__VINEXT_BUILD_ID access so Vite can inline the build id for processless Worker bundles. |
||
|
Nathan Nguyen
|
dce7b70329 | fix: normalize NextRequest.url through nextUrl (#931) | ||
|
James Anderson
|
098240d474 | chore(ci): change bonk opencode version (#937) | ||
|
James Anderson
|
a701d72bab | chore(ci): change bonk perms back to normal (#936) | ||
|
James Anderson
|
103aef2b2d | chore(ci): change bigbonk permissions prereq (#935) | ||
|
James Anderson
|
8747b3d1a0 |
chore(ci): change bonk back to opus 4.6 (#934)
* chore(ci): change bonk back to opus 4.6 * add nathan to bonk workflow * boink |
||
|
Nathan Nguyen
|
18962c51d6 |
fix(app-router): action redirects (#910)
* Fix app form action redirects * Trigger CI rerun * Refactor progressive action handling * Address app action review feedback * Trigger CI rerun |
||
|
Nathan Nguyen
|
598782e7b5 |
Make revalidatePath expire route-scoped fetch cache reads (#917)
Fetch cache entries without explicit next.tags could survive revalidatePath(). The page or route cache entry was invalidated, but the regenerated render could still read the old fetch response and reproduce stale data. The missing boundary was Next.js soft tag semantics: path-derived implicit tags are read-time context for fetch cache lookups, not durable tags stored on the fetch entry. Thread route-derived soft tags through App Router renders, pass them to fetch cache reads, and teach the memory and KV handlers to treat revalidated soft tags as cache misses without deleting shared fetch entries. Regression coverage verifies revalidatePath invalidates untagged fetch cache reads and that KV soft-tag misses do not delete the shared entry. |
||
|
Iago Lima Toledo
|
dcb417dfa3 |
fix(fonts): google fonts building perfectly (it was just a regex 😭) (#920)
* fix(fonts): google fonts building perfectly (it was just a regex 😭) * fix(fonts): allow newlines inside bracket clauses Previous fix's [^;\n]+? blocked the semicolon-less line bug but also stopped matching multi-line bracket imports/exports — Prettier wraps specifiers across lines once the clause crosses printWidth. Clause is now a sequence of brace blocks (newlines allowed inside, but [^}] keeps the lazy match from spanning past the matching close brace) or single non-;\n chars. Multi-line shapes match again; the original swallow-prevention guard still holds. Reverts the export regex back to [^}]+ — } already delimits, so the extra \n exclusion was over-defensive. * fix(fonts): apply formatter to test file --------- Co-authored-by: MrIago <28714061+MrIago@users.noreply.github.com> |
||
|
James Anderson
|
fc5f7a6492 |
fix(dev): hoist socket-error backstop to module top-level so lifecycle events can't remove it (#916)
* debug(dev): hoist socket-error backstop to module top-level + opt-in trace Two changes intended to pin down why the process-level handler from PR #913 still doesn't catch the ECONNRESET trace some users report: 1. Move installation from inside configureServer to module top-level, guarded by Symbol.for to prevent double-install. Earlier versions tied teardown to httpServer 'close', which Vite emits on dep re-optimization, full reloads, and other lifecycle events — leaving a window where the listener is absent when a stale stream errors. Module-level install + Symbol guard removes that window entirely. 2. Add an opt-in console.warn marker behind VINEXT_DEBUG_SOCKET_ERRORS=1 that fires whenever the listener absorbs a peer-disconnect error. The visible `node:events:487 throw er; // Unhandled 'error' event` output otherwise leaves it ambiguous whether the listener never ran (install / lifecycle issue) or ran but was somehow bypassed (different async surface / dispatcher). The marker disambiguates. No behavior change for users not setting the env flag beyond the hoisting. Filter codes and re-throw shape are unchanged from #913. Refs #905. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address bonk review on dev socket-error backstop Three issues from PR #916 review: 1. Module-top-level install was wrong — it ran during 'vinext build', 'vp pack', and Vitest worker imports too, so the JSDoc 'dev-only' claim was inaccurate and a real ECONNRESET from a pre-render fetch in CI would have been silently swallowed. Move the install back into configureServer (which only fires in dev) but **without** binding teardown to httpServer 'close' — that was the original lifecycle bug PR #913 had. Symbol.for guard makes re-invocation a no-op so the listener still survives server restarts within a session. 2. Restore the middleware-mode skip dropped in the previous commit. Embedders running vinext in middleware mode (Express/Connect) keep ownership of their own process-level handlers. Gated on server.httpServer, matching the prior PR #913 behavior. 3. Document the listener-ordering implication of installing during configureServer (registers late in the queue, so Sentry / structured logging / test-runner hooks still observe non-peer-disconnect errors before vinext's sync re-throw aborts iteration), and the Symbol.for first-loaded-wins caveat for the multi-version case. Refactor: collapse the duplicated `(err as Error & { code }).code` casts into a single `peerDisconnectCode` helper that returns the matched code or undefined, so the debug log path reuses it without recasting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dev): re-hoist socket-error backstop to module load (configureServer too late) The previous attempt to address bonk's review by moving install back into configureServer was wrong — it didn't restore working behavior: $ VINEXT_DEBUG_SOCKET_ERRORS=1 vp dlx vinext@455a61b dev ... GET /stage/oliver 200 in 1.1s node:events:487 throw er; // Unhandled 'error' event Error: read ECONNRESET ... No `[vinext] dev socket-error backstop installed` startup line. The function never ran. Cause: in vite-plus's plugin lifecycle, `server.httpServer` is null at the moment configureServer fires, so the `if (server.httpServer)` middleware-mode guard skipped install. Requests still get served (httpServer is created later) but the listener was never attached. Hoist back to module load. To address bonk's "loaded in build/test contexts too" concern, gate install on: - `process.env.VITEST === "true"` → Vitest worker imports - `process.argv` contains "build" → `vinext build` / `vp build` `process.argv` is read because the CLI entry imports index.ts before it has a chance to set an env var, and `configResolved` runs too late for module-load gating. Updated JSDoc to be honest about the install context, the listener- ordering implication, the middleware-mode embedder situation, and the Symbol.for first-loaded-wins caveat. Refs #905. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(dev): positive-gate socket backstop on argv[2]==='dev' Replace the negative VITEST/build skip list with a single positive check: install only when argv[2] === "dev". Matches `vinext dev`, `vp dev`, `vite dev`, and any CLI following the "<bin> dev" convention. Everything else — build, Vitest workers, library embedders with a custom runner — skips install, so genuine peer-disconnect errors surface normally in those contexts. Cleaner default: no listener unless we're confident this is a dev server invocation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(dev): gate socket backstop on Vite's command==='serve' Replace argv-sniffing with the canonical signal: Vite's `config()` hook receives `{ command: "serve" | "build" }` directly. Install the backstop only when `command === "serve"`. Works identically for `vinext dev`, `vp dev`, `vite dev`, and library embedders that call `createServer` themselves — anywhere Vite considers itself a dev server. `config()` runs before `configureServer` (so before httpServer matters) and before the dep-optimization or full-reload events that broke the earlier `httpServer.close`-tied teardown. Symbol.for guard keeps the install idempotent across server restarts within a session. Removes: - Module-load-time install + argv[2] === "dev" check - VITEST / build env-var negative skip list Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dev): narrow gate to !isPreview so vite preview skips install `command === "serve"` covers both `vite dev` and `vite preview` (the post-build static server). Preview doesn't stream RSC and doesn't need this guard, so narrow to `command === "serve" && !env.isPreview`. `vinext start` runs prod-server.ts directly without Vite, so it was already correctly excluded — `config()` never fires there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: extend socket-error backstop to vinext start (Next.js parity) Next.js installs uncaughtException + unhandledRejection handlers in their router-server unconditionally — no dev/prod gate. Both the Node.js dev server and the Node.js prod server share the same process-level error guards. See vercel/next.js packages/next/src/server/lib/router-server.ts:809-810. vinext was only guarding the dev path. `vinext start` runs prod-server.ts directly as a Node HTTP server (used for self-hosted deploys; Cloudflare Workers prod doesn't load this module — the runtime owns socket lifecycle there) and had the same theoretical exposure to peer-disconnect crashes through the pipeline() / fetch() paths it streams responses through. Refactor: - Extract the install function from index.ts into a new src/server/socket-error-backstop.ts (drop the "Dev" prefix — no longer dev-only). - Keep the call from the vinext:config plugin's config() hook, gated on command === "serve" && !isPreview (covers vinext dev / vp dev / vite dev / library embedders). - Add a parallel call at the top of startProdServer() in prod-server.ts (covers vinext start). vinext is more conservative than Next.js's log-only handler — we filter strictly on peer-disconnect codes and sync re-throw the rest, so genuine bugs still surface. The parity is in *where* we install, not what we swallow. Vitest workers and `vinext build` never reach either entry point, so peer-disconnect errors in those contexts surface normally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dev): auto-install socket-error backstop at module load The Vite config() hook gate (`command === "serve" && !isPreview`) proved unreliable in vite-plus's lifecycle — the hook didn't fire (or didn't pass command correctly), and install was silently skipped. Field reproducer: published |
||
|
Nathan Nguyen
|
696948aca4 |
fix(next/font/google): use real axis range, validate options at build time (#885) (#893)
* shim: rewrite buildGoogleFontsUrl on top of the new build-url helper
Replace the inline URL builder in shims/font-google-base.ts with a thin
wrapper around the pure buildGoogleFontsUrl helper from PR 1
(packages/vinext/src/build/google-fonts/build-url.ts). The shim still
takes a FontOptions object and still returns a CSS URL, so external
callers and the existing 84 unit tests are unaffected.
Two visible behaviour changes:
1. Empty options no longer emit `:wght@100..900`. They emit no axis
segment at all, which makes Google return the default static face
(HTTP 200) for every font, including narrow-axis families like Sen
that previously got HTTP 400. This is the dev fallback only; the
build plugin always pre-resolves the real axis range from metadata
in production.
2. Italic-only requests no longer drop the ital axis. Pre-fix the outer
guard `weights.length > 0 || styles.length > 0` entered the block
but the inner branch only handled the wght path, so a call like
`Inter({ style: ['italic'] })` produced `family=Inter&display=swap`
with no italic anywhere in the URL. The shim now derives an ital
array from style and falls back to weight '400' when ital is set
without an explicit weight, so Google has somewhere to attach the
ital axis in the URL.
The shim deliberately does not import font-data.json or validate.ts: a
388 KB metadata file in the Worker bundle would dwarf the rest of the
shim, and the production path already has the metadata-aware variant.
Refs #885.
* plugin: validate options against metadata, surface HTTP errors as build errors
Replace the inline URL builder in plugins/fonts.ts with the pipeline added
in PR 1: validateGoogleFontOptions parses the call's options against the
bundled font metadata, getFontAxes resolves the actual `wght` range for
variable fonts, and buildGoogleFontsUrl emits the URL.
Three behaviour changes:
1. Variable fonts called with no `weight` now request the font's real
axis range (Sen 400..800, Inter 100..900) instead of a hardcoded
100..900. This is the user-visible fix for issue #885: Sen returns
HTTP 200 with all five face files instead of HTTP 400.
2. Validation errors surface as build errors with the source file path
attached. Unknown family, missing weight on a static font (Anton,
Archivo Black, etc.), unknown weight, mixing 'variable' with explicit
weights, axes on non-variable fonts, invalid display values, and
missing/unknown subsets are now all caught at transform time. Pre-fix
these silently produced URLs that Google rejected at request time.
3. HTTP non-2xx responses from Google are now distinguished from network
errors. A 4xx/5xx is wrapped in GoogleFontsHttpError and surfaced
with the failing URL plus the response body, because pre-fix the
silent `catch { return; }` skipped self-hosting and let the runtime
CDN path retry the same broken URL in the browser. Network errors
(offline dev) still fall through silently to keep `vp dev` usable
without a connection.
The fontCache key is the URL, so existing on-disk caches under
`.vinext/fonts/<hash>/` will be stale for affected fonts after this
change. One refetch per font on the next build.
Refs #885.
* tests: regression coverage for the #885 narrow-axis bug and validator throws
Five new tests in tests/font-google.test.ts plus subset additions to
existing fixtures whose Inter() / Roboto() calls were missing them.
Shim regressions:
- buildGoogleFontsUrl with empty options must not emit `:wght@100..900`
- buildGoogleFontsUrl with style-only options must keep the ital axis
visible in the URL
Plugin regressions:
- Sen() with no weight must fetch `:wght@400..800`, not `:wght@100..900`
- unknown families throw a build error with the file path attached
- non-variable fonts called without an explicit weight throw at build
time instead of silently emitting a broken URL
- HTTP 4xx/5xx from Google surfaces as a build error containing the URL
and the response body
- network errors (DNS, ECONNREFUSED, AbortError) still fall through
silently so offline dev keeps working
The existing plugin transform tests passed `Inter({ weight: '400' })`
without subsets; the validator now treats that as a missing-subsets
error (matches Next.js behaviour), so those fixtures gain
`subsets: ['latin']`. The two nested-brace regression tests still
exercise the parser's balanced-brace logic but use a benign
`_placeholder` field instead of `axes: { wght: 400 }`, which is
semantically invalid for Inter under the new validator.
Refs #885.
* address google font review feedback
|
||
|
James Anderson
|
d9b25e559d |
fix(dev): backstop uncaughtException for socket errors pipe() misses (#913)
* fix(dev): backstop uncaughtException for socket disconnects pipe() misses PR #911 attached an error listener to every connection socket, which fixes the direct res.socket case. But Node's pipe() machinery has a separate re-emission path: when a Readable source errors and the destination has no error listener, pipe() emits the error on the destination — and that destination is not always the connection socket. Two real call sites trigger this: - fromWeb(fetch().body).pipe(res) in proxyExternalRewriteNode, where the source is the upstream fetch body, not the inbound socket - streaming surfaces inside @vitejs/plugin-rsc that own their own pipe topology Outbound sockets created by fetch() also never fire 'connection' on server.httpServer, so they sit completely outside the existing guard. Add a process-level uncaughtException handler scoped to the dev server that drops only peer-disconnect codes (ECONNRESET, EPIPE, ECONNABORTED) and re-throws everything else on nextTick to preserve Node's default crash behavior for real errors. Removed on httpServer 'close' so it doesn't leak across server restarts. Refs #905. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address bonk review on uncaughtException backstop Three issues from PR #913 review: 1. (Blocker) The process.nextTick(() => throw err) re-throw created an infinite loop, not a crash: the listener is still installed when the nextTick callback fires, so it catches the re-thrown error and schedules another, forever — silently swallowing genuine bugs. Replace with a synchronous throw inside the handler, which Node treats as fatal and produces the same observable as no listener (stack to stderr, non-zero exit). 2. Skip installation entirely in middleware mode (httpServer is null). The embedding host owns process-level handlers and there's no reliable teardown hook to remove ours, so installation would leak. 3. Mirror the same filter onto unhandledRejection — peer-disconnect errors from outbound fetch() and pipe re-emissions can surface through either channel depending on the async path. Verified locally: sync throw inside the handler crashes Node with the original stack and a non-zero exit code; the previous nextTick pattern loops indefinitely on the same input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
James Anderson
|
54c91f16f3 |
fix(app-router): surface Error.cause in dev-server error output (#912)
* fix(app-router): flatten Error.cause chain into dev-server error output
Vite's dev-server "Internal server error:" formatter
(buildErrorMessage in vite-plus-core) builds output from err.message
and err.stack only, silently dropping err.cause. A wrapper like
`new Error("Failed query", { cause: pgError })` would surface in the
dev console as just "Failed query" — the actual postgres ECONNREFUSED,
role-missing, or workerd socket error in .cause was invisible.
This commit adds flattenErrorCauses(err), called at the App Router RSC
handler boundary before any error escapes to Vite. It walks .cause
(depth-capped, cycle-safe) and embeds each cause's message into
err.message and stack frames into err.stack as `at` lines (so they
survive Vite's cleanStack /^\s*at/ filter). Marked idempotent via a
non-enumerable symbol so Node's util.inspect output is unaffected and
prod logs that already format causes correctly are not duplicated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address bonk review feedback
1. Gate flatten on NODE_ENV !== "production" at the call site. Without
this, prod-server.ts:1106's `console.error("[vinext] Server error:", e)`
would render the cause twice — once in the synthesized message line,
once in util.inspect's native [cause]: block. Vite build-time-replaces
NODE_ENV, so the prod bundle compiles the catch branch out entirely.
2. Replace the marker-enumerability test with one that actually invokes
util.inspect and asserts the marker symbol is invisible to it.
3. Wrap the three mutating operations (defineProperty, .message =,
.stack =) in try/catch so frozen / non-extensible errors are left
untouched rather than masking the user's real failure with a
TypeError from inside the helper.
4. Switch FLATTENED_MARKER from Symbol.for(...) to module-private
Symbol(...) — there's no cross-module coordination needed.
Add a regression test for the frozen-error case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
James Anderson
|
a212c2d74a |
fix(dev): swallow socket errors so peer disconnects don't crash dev server (#911)
Node throws on unhandled 'error' events on sockets. When the browser drops a connection mid-response (HMR triggers a reload while an RSC stream is still flushing), the next res.write surfaces ECONNRESET on res.socket with no listener attached and takes down `bun dev`. Attach a no-op error listener on every connection — same guard Vite's HMR server and Next.js install for the same reason. Closes #905. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Nathan Nguyen
|
67a929bf25 |
Preserve middleware headers on app boundary responses (#908)
* Preserve middleware headers on app boundary responses * test: stabilize app router redirect e2e |
||
|
Nathan Nguyen
|
f335e2da3f |
fix(app-router): block javascript: URLs in router.push/replace/prefetch (#906)
* fix(app-router): block javascript: URLs in router.push/replace/prefetch
router.push("javascript:alert(1)") currently runs the script. _appRouter.push,
.replace, and .prefetch in shims/navigation.ts forward the href straight to
navigateClientSide, where isExternalUrl matches the javascript: scheme and
falls through to window.location.assign, which executes the URL. router-derived
hrefs (URL params, user content, query state) become an XSS vector.
Next.js blocks dangerous URI schemes at the router boundary and throws
"Next.js has blocked a javascript: URL as a security precaution." See
packages/next/src/client/components/app-router-instance.ts:343,400,440,458 in
the canary tree. Vinext's url-safety.isDangerousScheme already covers
javascript:/data:/vbscript: with the same obfuscation handling Next.js uses,
and Link and Form already gate on it; the App Router programmatic surface was
the remaining gap.
Add assertSafeNavigationHref at the top of push/replace/prefetch, before the
isServer short-circuit so SSR callers fail loudly too. Throw an Error with
the verbatim Next.js message so consumer error tooling can match across both
frameworks.
Test: tests/router-javascript-urls.test.ts ports the assertions from
.nextjs-ref/test/e2e/app-dir/javascript-urls/javascript-urls.test.ts down to
unit scope, covering javascript:/data:/vbscript:, uppercase, embedded tabs,
leading whitespace, and a safe-URL no-throw guard for each method.
* test(router): split ported vs vinext-only dangerous-scheme cases
The header said "Ported from Next.js" but data: and vbscript: are vinext
extensions, not in the upstream e2e suite. Add a Coverage split note that
attributes each scheme: javascript: + obfuscation variants come from
test/e2e/app-dir/javascript-urls/javascript-urls.test.ts; data: and vbscript:
ride on top via shims/url-safety.isDangerousScheme.
* fix app router dangerous redirect gaps
* test address dangerous URL review nits
|
||
|
Nathan Nguyen
|
2f7e5cfcbc |
Return leaf selected layout segment for named slots (#909)
useSelectedLayoutSegment(parallelRoutesKey) returned the first segment for every route key. That matches children routes, but it misreports nested named parallel routes such as a slot path with members/profile by returning members instead of the active leaf profile. The root cause was treating the array returned by useSelectedLayoutSegments() as having the same reduction rule for children and named slots. Match Next.js by keeping first-segment behaviour for children and using the final segment for non-children parallel route keys. Adds a nested @team fixture route that mirrors Next.js parallel-routes-use-selected-layout-segment coverage. |
||
|
Nathan Nguyen
|
428c199ec1 |
port Next.js Google Fonts metadata + URL pipeline (PR 1 of 2 for #885) (#892)
* vendor Google Fonts metadata from Next.js Adds packages/vinext/src/build/google-fonts/font-data.json (1911 entries, 388 KB pretty-printed) plus a typed re-export wrapper. The metadata enumerates each Google Font's available weights, styles, variable axes, and preloadable subsets, and is the source of truth a faithful next/font/google port needs to encode the right URL for variable fonts and reject invalid options at build time. Today vinext hardcodes :wght@100..900 in both the shim runtime URL builder and the build-time plugin, which produces HTTP 400 from Google for fonts whose wght axis is narrower (Sen 400..800, Anton single 400). Without metadata there is no way to emit the correct axis range, validate weights against the font's real options, or auto-disable preload for fonts with no preloadable subsets. The JSON itself is consumed only at build time and during dev. Production Workers run the self-hosted CSS injected by the build plugin and never read this file. The pretty-printed shape matches the upstream layout so future refreshes from canary remain reviewable. This commit only vendors the data and types. URL pipeline and validator land in the next two commits. Refs #885. * port Google Fonts URL pipeline from Next.js Adds three pure helpers under packages/vinext/src/build/google-fonts: sortFontsVariantValues (numeric variant comparator), buildGoogleFontsUrl (URL assembler), and getFontAxes (metadata-driven axis resolver). Each is a direct port of the matching module in vercel/next.js packages/font/src and has no defaults of its own. Defaults live in the validator (next commit). This is the layer that translates a validated options object into the URL shape Google Fonts CDN actually accepts. Sorting matters because Google rejects out-of-order variant lists with HTTP 400. The metadata-driven axis resolver is what lets a Sen({}) call produce :wght@400..800 instead of the broken :wght@100..900 default. No consumer wired up yet. The shim and plugin still call the old hardcoded URL builders; the integration lands in PR 2. Tests: 21 new tests across the three modules. Each helper is exercised in isolation against the vendored metadata, including the regression case from #885 (variable Sen resolves to wght 400..800) and the italic-only branch that vinext currently drops. Refs #885. * port Google Fonts options validator from Next.js Adds validateGoogleFontOptions, the parse-once boundary for next/font/google calls. Mirrors Next's validate-google-font-function-call.ts, dropping the SWC-coupled (functionName, fontFunctionArgument) signature in favour of (fontFamily, options). Returns ValidatedGoogleFontOptions, a type whose existence proves the input was rejected if Google would have rejected it. Behavior matches Next.js: throws on unknown family, unknown weight, mixing 'variable' with explicit weights, axes on non-variable fonts, axes without weight 'variable', invalid display, unknown subsets, preload with no subsets. Defaults missing weight to ['variable'] for variable fonts, and auto-disables preload for fonts with no preloadable subsets. This closes the loop on the URL pipeline added in the previous commit. Together with getFontAxes and buildGoogleFontsUrl, vinext now has a metadata-driven path from caller options to a valid Google Fonts URL. The shim and plugin still build URLs the old way; PR 2 wires this through. Tests: 16 new tests covering every throw branch and every default branch, plus an index.ts barrel that the future plugin import will use. The "auto-disable preload" test pins on Playwrite AR Guides, which has empty subsets in upstream metadata; if a future canary refresh adds a subset to that family the test will need a different fixture. Refs #885. * fold attribution into font-metadata header, drop unused exports Drop NOTICE.md and the index.ts barrel, demote four type aliases from exports to module-internal. Knip flagged the index file as unused (no consumer until PR 2) and the type aliases as unused exports for the same reason; YAGNI says wait for the consumer rather than ship dead public API. Vercel's MIT copyright on font-data.json is preserved in the font-metadata.ts header instead of in a sibling NOTICE.md, matching the in-tree pattern Next.js uses for vendored third-party files. Tests: vp run knip exits 0, 37 google-fonts tests still pass. * address PR 892 review: document subsets parity, harden preload-disable test `validateGoogleFontOptions` rejects `subsets: []` with `preload: true` where Next.js silently accepts it (Next uses a truthy check on `subsets`, vinext defaults to `[]` and checks `length`). The behavior is intentional (an empty subsets array with preload enabled is always a caller mistake), but undocumented divergence from a faithful port invites a future "fix back to upstream" PR. Annotate the divergence at the throw site. Auto-disable-preload test pins on `Playwrite AR Guides` having an empty subsets array in the bundled metadata. If a metadata refresh ever adds a subset to that family, the family-specific test silently flips into a false positive. Add an invariant test asserting the bundled metadata still contains at least one family with no preloadable subsets, so loss of branch coverage fails CI loud. |
||
|
James Anderson
|
ec4cdd4e23 | chore(ci): switch bonk back to opus 4.7 (#904) | ||
|
Nathan Nguyen
|
f8e9d3a57b |
ci(bonk): bump opencode to 1.4.11 to unblock OAI through cf-ai-gateway (#903)
Bots throw "max_tokens not supported, use max_completion_tokens" on 1.4.6 with gpt-5.x reasoning models. opencode 1.4.6 predates the cf-ai-gateway plugin's chat.params hook (PR #22864, landed in 1.4.7), which strips maxOutputTokens for OpenAI reasoning models so the @ai-sdk/openai-compatible default no longer leaks the rejected field. The earlier ProviderInitError on 1.14.x was a separate regression in opencode's npm install path: the major bump introduced loadOptions(dir) via @npmcli/config inside Npm.add, which fails on CI runners and bubbles up as InstallFailedError, then gets re-wrapped at provider.ts:1522 as ProviderInitError with the original cause swallowed by the session error path. Other Cloudflare repos (workers-sdk, workerd, kumo) hit the same regression on anthropic and pinned 1.4.6 / 1.2.27 to dodge it. 1.4.11 is the last 1.4.x release: it has the chat.params max_tokens patch and predates the loadOptions install regression. Same ai-gateway-provider 3.1.2, same @ai-sdk pins as 1.4.6, so the install path stays the known-good arborist.reify call shape that works for workers-sdk + anthropic on 1.4.6. Verified locally with opencode 1.14.25 + cf-ai-gateway/openai/gpt-5.4 + fake creds: empty-cache short-circuit and loadOptions both reproduce the silent ProviderInitError. 1.4.11 lacks both code paths. |
||
|
Nathan Nguyen
|
e7706ea31b |
ci(bonk): pin opencode to 1.4.6 to dodge cf-ai-gateway ProviderInitError (#902)
Every bonk and bigbonk run since opencode 1.14.25 throws UnknownError: ProviderInitError during cf-ai-gateway SDK init, regardless of model. Verified across gpt-5.5 (#898), gpt-5.4 (#900), and the throw fires before any per-model code path runs. workers-sdk hit the same regression and pinned opencode_version to 1.4.6 with an inline comment naming ProviderInitError as the cause. Several other Cloudflare repos pin 1.2.27 / 1.4.6 for the same reason. vinext is the only Cloudflare repo running OAI through cf-ai-gateway, so the OAI-specific path stayed unobserved until #898 re-tried it on 1.14.25. Pinning to 1.4.6 here reuses workers-sdk's known-good version. The gpt-5.4 model from #900 stays; gpt-5.4 predates 1.4.6's bundled snapshot by over a month, so registry resolution is fine. |
||
|
Nathan Nguyen
|
5b5071cd5c |
ci(bonk): downgrade bots from gpt-5.5 to gpt-5.4 (#900)
bonk and bigbonk currently throw ProviderInitError on every invocation after the gpt-5.5 switch in #898. The lookup-side regression that #899 patched was masked by the opencode bump (1.14.22 -> 1.14.25) but a second failure now surfaces during SDK init for cloudflare-ai-gateway. opencode swallows the underlying cause, and the gpt-5.5 path through ai-gateway-provider has no working precedent in this repo, so iterating on it would block PR review indefinitely. Roll the model back to gpt-5.4, which has a known-good path through opencode + ai-gateway-provider + Cloudflare AI Gateway, and remove the opencode.json workaround introduced in #899. Reasoning effort stays at "xhigh" per James's preference; opencode will fall back to its default effort if gpt-5.4 doesn't accept it. Once gpt-5.5 SDK init is fixed upstream (opencode + ai-gateway-provider + models.dev cloudflare-ai-gateway entry), we can roll forward again. |
||
|
Nathan Nguyen
|
ab243f2036 |
ci(opencode): register openai/gpt-5.5 under cloudflare-ai-gateway (#899)
* ci(bonk): switch bonk workflow to GPT-5.5 xhigh variant Updates the bonk workflow to use cloudflare-ai-gateway/openai/gpt-5.5 with reasoning effort variant xhigh, replacing the previous claude-opus-4-7 model. https://claude.ai/code/session_012n6RFuqfQGQY7AaayqLcaG * ci(bonk): extend GPT-5.5 xhigh switch to bigbonk, agent defs, and docs - bigbonk workflow: cloudflare-ai-gateway/openai/gpt-5.5 with variant xhigh (was claude-opus-4-7 + max) - viguy and reviewer OpenCode agent definitions: openai/gpt-5.5 (was anthropic/claude-opus-4-7) - CONTRIBUTING.md: update recommended setup and BigBonk description to GPT-5.5 xhigh The Next.js tracker workflow and agent are intentionally left on Claude Opus 4.7. https://claude.ai/code/session_012n6RFuqfQGQY7AaayqLcaG * ci(opencode): register openai/gpt-5.5 under cloudflare-ai-gateway bonk and bigbonk currently throw ProviderModelNotFoundError when invoked because opencode's bundled models snapshot predates GPT-5.5 (released 2026-04-23) and the cloudflare-ai-gateway entry on models.dev does not yet list it. opencode resolves provider.models[modelID] before any network call, so the lookup miss fires synchronously and the workflow exits non-zero before reaching the gateway. opencode merges user-supplied provider.<id>.models into the resolved registry dict, so registering the entry locally unblocks workflow invocations without waiting on either an upstream registry merge or an opencode release that rebundles the snapshot. Pricing, limits, modalities, and reasoning capability mirror the direct openai/gpt-5.5 entry from models.dev. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Nathan Nguyen
|
76690d4bcf |
Switch bonk and bigbonk workflows to GPT-5.5 xhigh variant (#898)
* ci(bonk): switch bonk workflow to GPT-5.5 xhigh variant Updates the bonk workflow to use cloudflare-ai-gateway/openai/gpt-5.5 with reasoning effort variant xhigh, replacing the previous claude-opus-4-7 model. https://claude.ai/code/session_012n6RFuqfQGQY7AaayqLcaG * ci(bonk): extend GPT-5.5 xhigh switch to bigbonk, agent defs, and docs - bigbonk workflow: cloudflare-ai-gateway/openai/gpt-5.5 with variant xhigh (was claude-opus-4-7 + max) - viguy and reviewer OpenCode agent definitions: openai/gpt-5.5 (was anthropic/claude-opus-4-7) - CONTRIBUTING.md: update recommended setup and BigBonk description to GPT-5.5 xhigh The Next.js tracker workflow and agent are intentionally left on Claude Opus 4.7. https://claude.ai/code/session_012n6RFuqfQGQY7AaayqLcaG --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
James Anderson
|
f37b783e73 | switch bonk back to opus (#897) | ||
|
James Anderson
|
3ddaefa23c | bump opencode (#896) | ||
|
James Anderson
|
2c837da309 | try gpt 5.5 for bonk (#895) | ||
|
James Anderson
|
dbcca8c780 | try gpt 5.5 for bonk (#894) | ||
|
James Anderson
|
9ff25d8be0 |
fix knip issues (#890)
* fix knip issues * run knip in ci * . * feedback |
||
|
dependabot[bot]
|
89a351b44b |
chore(deps): bump postcss from 8.5.3 to 8.5.10 (#889)
Co-authored-by: James <james@eli.cx> |
||
|
Steve Faulkner
|
c58daa61ba |
fix: centralize protocol-relative URL guard to handle percent-encoded delimiters (#888)
Six places in the request pipeline inlined a variation of:
pathname.replaceAll("\\", "/").startsWith("//")
to reject protocol-relative paths before they reach the trailing-slash
redirect emitter (which would otherwise echo them into a Location header).
The check handles literal delimiters but not their percent-encoded forms —
`/%5Cevil.com/` survives the guard, then `normalizePathnameForRouteMatchStrict`
re-encodes the decoded backslash back to `%5C` (preserving it as part of a
single path segment), so the 308 trailing-slash redirect ends up with
`Location: /%5Cevil.com`. Browsers percent-decode Location headers and
WHATWG URL treats backslash as forward slash, so the browser resolves that
to a protocol-relative host.
Factor the leading-segment shape check into a new `isOpenRedirectShaped`
helper in `server/request-pipeline.ts` that recognises literal (`//`, `/\\`)
and percent-encoded (`%5C`, `%2F`) variants, and swap each call site over
to it. Also add a defense-in-depth check in `normalizeTrailingSlash` so a
future caller that skips the upstream guard still can't emit a bad Location.
Updated call sites:
- server/request-pipeline.ts — guardProtocolRelativeUrl + normalizeTrailingSlash
- server/prod-server.ts — App Router and Pages Router Node handlers
- server/app-router-entry.ts — default Cloudflare Worker entry
- server/app-ssr-entry.ts — SSR environment entry
- index.ts — Pages Router Vite dev middleware
- deploy.ts — generated Pages Router Cloudflare Worker (inlined, kept in sync)
Regression tests in tests/request-pipeline.test.ts cover the encoded
variants plus the defense-in-depth path. Updates the pinned string
assertion in tests/deploy.test.ts that was checking for the old inlined
pattern.
v0.0.44
|
||
|
James Anderson
|
ff5e14c41d |
setup knip (#886)
* setup knip * nits |
||
|
Steve Faulkner
|
54516468c3 | [codex] Fix usePathname hydration snapshot (#878) | ||
|
James Anderson
|
504447db68 | bump opencode version (#881) | ||
|
Nathan Nguyen
|
dd379ffc22 |
fix(app-router): hard-navigate to browser URL on non-ok RSC fetch (#875)
* fix(app-router): hard-navigate to browser URL on non-ok RSC fetch
Client-side RSC navigation fetch passes non-ok responses (404, 500, etc.)
directly to createFromFetch. The HTML error body is parsed as an RSC
stream, producing a cryptic "Connection closed" error, and the outer
catch hard-navigates to the same URL that just failed — risking a
reload loop and surfacing an opaque diagnostic.
Non-ok responses are not RSC payloads and must not reach createFromFetch.
Next.js handles this identically in fetch-server-response.ts:211 with a
hard ("MPA") navigation to the browser-facing URL stripped of flight
markers.
Add a !navResponse.ok guard after the stale-navigation check in the
navigation loop: hard-navigate to currentHref (the browser URL without
.rsc suffix). The existing finally block handles
settlePendingBrowserRouterState and clearPendingPathname on this return
path, so no additional bookkeeping is required.
Add a !rscResponse.ok guard in readInitialRscStream so the initial
hydration fallback path reloads instead of attempting RSC parsing on an
HTML error body. Return a never-resolving ReadableStream so the caller
does not proceed into createFromReadableStream before the reload takes
effect.
E2E tests verify: 404 RSC nav hard-navs to the non-.rsc URL; 500
(simulated via page.route interception) hard-navs without logging an
RSC parse error; the final URL never contains .rsc.
* fix(app-router): break reload loop when initial RSC fetch is persistently non-ok
readInitialRscStream reloads on a non-ok RSC response so the server can
render the correct error page as HTML. That assumes the next HTTP
response will differ, which is not guaranteed: if the RSC endpoint is
persistently broken while the HTML document is served successfully
(mismatched server handler, upstream gating, intercepted requests in
tests), the reload fetches the same HTML, the post-hydration RSC fetch
gets the same non-ok, and the page reloads forever.
Guard the reload with a sessionStorage key scoped to the current path.
The first non-ok response sets the key and reloads. The second non-ok
response for the same path clears the key and throws, letting the outer
bootstrap catch surface the error instead of triggering another reload.
A successful RSC response clears the key so a later unrelated failure
on the same path still gets one reload attempt.
Tighten the Playwright test that covers the 500 hard-nav path: after
the hard navigation settles, wait and assert both URL stability and a
bounded intercept hit count. A runaway reload loop would fail the hit
count assertion.
* fix(app-router): harden initial RSC recovery and cover non-RSC content-types
Five review follow-ups to the non-ok RSC fetch guard.
sessionStorage.getItem / setItem / removeItem can throw SecurityError in
strict-mode iframes, storage-disabled browsers, and some Safari private-
browsing configurations. The unguarded calls in readInitialRscStream
crashed hydration in exactly those environments — worse than the
original bug. Wrap every access in readReloadFlag / writeReloadFlag /
clearReloadFlag helpers that swallow the exception and fall through to
the "no prior attempt" branch.
Replace the throw after a looped reload with console.error plus a
never-resolving ReadableStream. main() is invoked as `void main()` with
no outer catch, so a throw here surfaced as an unhandled promise
rejection and fired window.onerror reporting hooks instead of leaving
the SSR'd DOM visible as the comment promised. Returning a stream that
never produces data halts hydration cleanly; the server-rendered HTML
stays on screen, client components simply never hydrate.
Clear the reload flag when readInitialRscStream enters the embedded-RSC
branch. The embed branch runs when the server successfully rendered the
page, so any prior flag is stale — without this clear, the flag could
persist across a hard-reload of the same path and skip the one legitimate
recovery attempt the user should get.
Cover the non-RSC content-type case on both the initial-hydration fetch
and the client-side navigation fetch. Next.js's equivalent guard
(fetch-server-response.ts:211) is `!isFlightResponse || !res.ok || !res.body`;
the previous commit only covered `!res.ok`. A proxy or CDN that returns
200 with a rewritten Content-Type (`text/html` instead of
`text/x-component`) would still reach createFromFetch and throw the
exact stream-parse error these guards exist to prevent. Check for a
Content-Type starting with `text/x-component` before parsing.
Tighten the Playwright hit-count assertion from <= 3 to <= 2. With the
guard in place the expected sequence is exactly two hits (the triggering
client RSC nav fetch plus one post-reload fetch that aborts); any extra
hit signals an unnecessary reload worth catching.
* test(app-router): narrow RSC error filter to stream-parse diagnostics
The test's console-error filter matched any message containing "RSC
navigation error", which also captures an unrelated pre-existing
hydration race: when page.evaluate calls __VINEXT_RSC_NAVIGATE__
before the AppRouter useLayoutEffect has committed, getBrowserRouterState
throws "Browser router state is not initialized". That race has nothing
to do with the stream-parse bug this PR fixes — the outer catch still
hard-navigates to the target URL correctly — but the broad filter
treated it as a failure, producing a flaky test that only passed on
retry (seen on commit
|
||
|
Nathan Nguyen
|
7bbb9b5f95 |
update bonk to opus 4.7 (#873)
* chore: upgrade workflows from claude-opus-4-6 to claude-opus-4-7 Update model references in bonk, bigbonk, and nextjs-tracker workflows to use the latest Opus 4.7 model via the Cloudflare AI Gateway. https://claude.ai/code/session_018pEfYehfR4YDA1Ekmu1vn3 * chore: upgrade opencode agents from claude-opus-4-6 to claude-opus-4-7 Update model references in reviewer, viguy, and nextjs-tracker agent configs to use the latest Opus 4.7 model. https://claude.ai/code/session_018pEfYehfR4YDA1Ekmu1vn3 --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Nathan Nguyen
|
faf1d9b4d6 |
Render intercept route layouts inside App Router slots (#872)
* Render intercept route layouts inside App Router slots Intercepted App Router pages currently render without any layout.tsx files that live inside the intercepting route tree. This is wrong because Next.js applies those layouts before the slot layout, and nested intercept routes lose required wrappers when a modal intercept matches. The routing pipeline assumed intercepts only needed a page module. That drops the intercept layout chain during discovery and never gives the slot override renderer enough information to rebuild the wrapper tree. Collect intercept layout paths during route discovery, serialize them through the generated RSC entry, and apply the loaded layout modules around the intercepted page before the slot layout. Add regression coverage for discovery, code generation, and slot render order. * Avoid repeated intercept layout scans and slot param wrapping Intercept layout discovery rescanned the same directories for every intercepted page, and slot rendering rebuilt thenable params multiple times inside one render path. That adds avoidable work and makes the intercept wrapper path harder to read. The implementation assumed these repeated lookups and allocations were harmless. That leaves the recursive router walk doing duplicate filesystem checks and gives slot wrappers different params wrapper identities in the same tree. Thread the discovered intercept layout chain through the recursion, compute slot thenable params once per slot render, and keep the generated entry snapshots in sync with the added intercept layout plumbing. * Clarify intercept layout plumbing and cover nested layout chains The intercept layout plumbing still used mixed names in generated code, and the reverse layout wrapper loop took extra decoding to read. That makes the fix harder to review and maintain even though the behavior is correct. The generated entry treated intercept-local layouts as a generic `layouts` field while the rest of the pipeline called them `interceptLayouts`, and the slot wrapper loop used a less direct reverse-index pattern. The tests also only covered a single intercept-local layout level. Rename the generated intercept field to `interceptLayouts`, switch the reverse wrapper loop to the clearer classic form, and add regression coverage for nested intercept layout chains. Update the generated-entry snapshots to match. |
||
|
Nathan Nguyen
|
ad1d1971af |
fix(app-router): keep isPending true across RSC-level redirects (#870)
* test(app-router): add E2E coverage for isPending across RSC redirect router.push() to a page that calls redirect() in its server component produced isPending flashing false mid-redirect. This test exposes that by installing a MutationObserver before click, recording every text value of #pending-state, and asserting no idle entry appears between the first pending transition and the destination commit. Ported from Next.js: test/e2e/app-dir/navigation/navigation.test.ts#L482 New fixtures: - /nextjs-compat/router-push-pending-redirect -- server component that calls redirect() - /nextjs-compat/router-push-pending-destination -- final destination with #redirect-destination - Push-redirect button added to existing PendingClient * fix(app-router): keep isPending true across RSC-level redirects When router.push() targeted a page whose server component called redirect(), isPending flashed false mid-redirect. The fetch returned a 307, navigateRsc detected the response URL differing from the requested URL, settled the deferred pendingRouterState to prevent promise orphaning, then recursed with programmaticTransition=false. The settle fired before the recursive call so React resolved the transition and isPending dropped to false during the hop. Root cause: the recursive re-entry model required settling the deferred promise before handing off, because the recursive call had no way to inherit the existing promise. This was explicitly documented as a tradeoff in the bandage comment at the settle site. Convert the recursive redirect follow into an inline while loop. The loop variables (currentHref, currentHistoryMode, currentPrevNextUrl, redirectCount) are updated on each redirect hop and the loop continues without re-entering navigateRsc. A single pendingRouterState is created before the loop and lives until the finally block, so isPending stays true across all redirect hops and drops exactly once when the final destination commits. The scattered settlePendingBrowserRouterState() calls inside the try body (early-return stale-id guards) are removed. The finally owns the single settlement site, and the function is already inside React.startTransition from the programmatic push, so the correct transition priority is maintained throughout. Closes #869. |
||
|
Nathan Nguyen
|
a775527b8b |
fix(app-router): keep isPending true across programmatic router.push (#868)
* fix(app-router): track pending router state for programmatic navigations `router.push()` / `router.replace()` / `router.refresh()` dispatched the new tree only after the RSC payload arrived, so `useTransition().isPending` dropped to false the moment the user callback returned and Suspense boundaries visibly committed their fallbacks during the fetch window. Wrapping the programmatic call in `useTransition` did not help because the state update React needed to latch onto was not scheduled until the navigation was already committed. Issue #639 describes the downstream symptom: double-flashing Suspense fallbacks during client navigation. Root cause: the browser router state was a `useReducer` that was only dispatched at commit time. React had no in-flight promise to wait on, so transition priority could not keep the previous UI on screen. Switch the browser router state to `useState<State | Promise<State>>` and, when a programmatic navigation starts, publish a deferred promise into the state slot synchronously. `BrowserRoot` unwraps the slot via `use()`, so any render during the RSC fetch suspends with transition priority and the previous UI stays visible. The deferred promise is resolved with the reducer output when the payload dispatches, or settled with the current committed state on supersession / error / internal RSC redirect, so `useTransition().isPending` latches true for the full navigation window and drops exactly once at commit. `router.push` / `replace` / `refresh` now also wrap their synchronous entry in `React.startTransition`, matching Next.js's `app-router-instance.ts`. Covered by `tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts`: same-route search-param push inside `useTransition` must keep the pending flag true until the server component commits its new filter. Known limitation (tracked as a follow-up): the internal RSC-redirect path still recurses through `__VINEXT_RSC_NAVIGATE__` with `programmaticTransition=false`, so the outer pending must be settled before the recursion. `useTransition().isPending` flashes false mid-redirect until the recursive call finishes. Fixing this requires converting the recursive redirect into an inline loop inside `navigateRsc` so a single deferred promise can span all hops, the way Next.js's action queue does. * address review feedback on programmatic router pending state Replace duck-type "then" in value check with instanceof Promise in isRouterStatePromise — the promises are always created locally so instanceof is correct and avoids misclassification if AppRouterState ever gains a then property. Add a comment in dispatchBrowserTree explaining why the pendingRouterState early return safely ignores the useTransitionMode branch — the outer React.startTransition in router.push/replace/refresh already provides transition priority, so no additional wrap is needed. Remove dead typeof React.startTransition === "function" guards in push/replace/refresh — vinext requires React 19+, where startTransition is always present. Inline the transition call directly. Bump SlowFilterResult delay from 200ms to 1_000ms so the isPending assertion has a wider window and is less susceptible to CI timing races.v0.0.43 |