mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
vinext@0.2.0
1503 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nathan Nguyen
|
2737a38d46 |
fix(pages-router): support stream proxying in API routes (#1902)
* fix(pages-router): support stream proxying in API routes
* fix(pages-router): address review feedback on stream proxying PR
- Narrow auto-end condition to only skip ending when handler returns res
(the destination of req.pipe(...).pipe(res)), preventing hangs from
non-stream return values.
- Replace full-body buffering with a PassThrough-backed PagesResponseStream
that resolves on first write, enabling real streaming responses.
- Add error handling to reject responsePromise when the response stream
errors, preventing silent request hangs.
- Add regression tests: non-stream return auto-end, multi-chunk write,
and response stream error handling.
* fix(pages-router): avoid hanging responses and PassThrough unhandled errors
- Narrow auto-end condition to to prevent non-stream
return values from hanging requests.
- Replace PassThrough + Readable.toWeb() with a direct ReadableStream controller
in PagesResponseStream to eliminate Controller is already closed unhandled
errors during stream proxying.
- Add destroy() override to error/close the stream body when the Node Writable
is destroyed after streaming starts.
- Add rejectResponse callback to PagesResponseStream constructor for proper
error propagation.
- Add regression tests for auto-end, multi-chunk streaming, and destroy error
handling.
* fix(pages-router): destroy Node side when Fetch body is cancelled
- Reject writes in _write() when streamEnded or destroyed to prevent
buffering chunks after cancellation.
- In the ReadableStream cancel handler, clear bufferedChunks and destroy
the Node-compatible PagesResponseStream so upstream writers receive
an error instead of silently accumulating data.
- Add regression test proving writes after body cancellation fail with
an error and do not accumulate in the bridge.
- Rename multi-chunk test to be more accurate about what it verifies.
* fix(pages-router): settle response when res.destroy() is called without error
- In destroy(), if the response has not been resolved yet, resolve the
response promise (with a closed stream) when no error is provided, or
reject it when an error is provided. This prevents the request from
hanging forever when a handler calls res.destroy() before any body
write and returns the res object.
- Add regression test proving res.destroy() without error resolves to an
empty 200 response instead of hanging.
* chore: trigger CI run for flaky E2E test validation
* fix(pages-router): track pipe event and fix request body cancellation
- Add `res.once("pipe", ...)` listener to detect when a handler pipes into the response without returning the writable. Skipping auto-end by return-value only missed the real-world pattern:
(req, res) => { req.pipe(upstream).pipe(res); } // returns undefined
Next.js uses the pipe event signal, not the handler return value.
- Fix request body cancellation: the async generator finally block now calls `reader.cancel()` before `releaseLock()`, restoring the pre-PR behavior where early return from `for await (chunk of req)` cancelled the underlying stream instead of just releasing the lock.
- Add regression tests for both pipe-without-return and pipe-with-return.
* fix(pages-router): drop handlerResult === res auto-end signal, rely on pipe event only
The `handlerResult === res` check was too broad — chainable helpers like
`return res.status(202)` return `this` without implying streaming, and would
cause a hang by skipping auto-end. Now that `res.once("pipe", ...)` provides
the real stream signal, the return-value check is redundant and harmful.
Removes:
- handlerResult variable and === res comparison
- comment about returning the writable
Adds:
- regression test for `return res.status(202)` not hanging
* fix(pages-router): address stream response review
* fix(pages-router): close IncomingMessage parity gaps in API route streams
- Yield Buffer chunks (zero-copy views) from the request readable and opt
out of objectMode so req matches IncomingMessage byte-stream semantics:
per-chunk chunk.toString("utf8") decodes instead of comma-joining byte
values, and setEncoding()/read(n) behave byte-wise.
- Honour config.api.externalResolver: Next.js never auto-ends a response,
and externalResolver: true is the documented contract for handlers that
respond after the handler promise settles (e.g. proxy middleware that
attaches its pipe asynchronously). Skip the auto-end safety net for such
routes so they are not truncated to an empty response.
---------
Co-authored-by: James <james@eli.cx>
|
||
|
James Anderson
|
56393aedad |
test(e2e): close page before prod server in inline-css fixture teardown (#1927)
The inlineCssApp fixture closed its temp production server while the page was still open. Link prefetches are scheduled via requestIdleCallback and could fire after the test body finished, hitting the closed port and logging ERR_CONNECTION_REFUSED, which the consoleErrors fixture then turned into a flaky failure at teardown. Close the page first so no late request can target the dead port. |
||
|
Nathan Nguyen
|
dea4472c2f |
fix(app-router): include parallel slot params in useParams (#1904)
* fix(app-router): include parallel slot params in useParams App Router navigation state previously used only the primary route match params when setting request navigation context, RSC param headers, render observations, and optimistic navigation payloads. That lost catch-all params from active parallel slot branches, so useParams() diverged from the slot page's params prop after SSR and client navigations. Next.js derives params from the selected router tree across parallel route branches. Resolve active slot params from route metadata, merge them into the navigation params boundary, and feed that selected param set through server render and optimistic navigation payloads. Port the upstream parallel-route-navigations fixture and cover the route graph, selected params helper, and optimistic payload boundary. * fix(app-router): preserve canonical pathname in navigation context after middleware rewrite * fix(app-router): resolve parallel slot params for static, revalidation, and action paths * chore: retrigger CI * fix(app-router): address ask-bonk review comments on parallel slot params - Remove unused parameter from - Add clarifying comment on routeParams fallback being deliberate no-op - Use (full matched params) instead of narrowed as base for slot param merge, preserving interception-specific extras on ISR regeneration path * fix(app-router): use full navigationParams in server-action re-render path The server-action re-render path was passing narrowed actionRerenderTarget.params to resolveAppPageNavigationParams, dropping interception-specific extras from source-route intercepts. Use actionRerenderTarget.navigationParams (full matched params) instead, matching the pattern used in the dispatch ISR path at app-page-dispatch.ts:602. Adds unit tests for the intercepted-slot and mixed intercepted + non-intercepted slot branches of resolveAppPageNavigationParams. * docs(router): clarify decode invariant and type cast rationale in parallel-slot params - app-optimistic-routing.ts: add comment explaining the pre-existing double-decode asymmetry between route params and slot params, noting TODO to converge decode passes - app-server-action-execution.ts: expand the comment on the interceptOpts cast to document why it's safe and what a follow-up would look like * fix(app-router): merge slot params for fresh intercepts --------- Co-authored-by: James <james@eli.cx> |
||
|
Nathan Nguyen
|
757610d2c6 |
refactor(router): plan same-page search and early navigation intent (#1900)
* feat(router): classify early navigation intent in planner
App Router same-page search and hash-only navigation decisions live inline in two separate executors: the shim derives hash-only changes from window.location, and the browser request loop derives same-page search changes from the committed snapshot. Each uses a slightly different predicate, so the early-intent classification cannot be tested without window, and the two paths can drift.
Add navigationPlanner.classifyEarlyNavigationIntent, a pure pre-fetch classifier that takes URL-delta facts and returns a typed decision: same-document scroll for hash-only changes, a cache-bypassing flight for same-path search changes, or an ordinary flight otherwise. This mirrors the classifyRscFetchResult surface added for fetch-result classification.
The classifier compares normalized search params so encoding or ordering differences that parse to the same query do not split a hash-only change into a search navigation. Unparseable hrefs fall back to an ordinary flight rather than masking a real failure as a same-page outcome.
Add the NC_SAME_DOC_SCROLL, NC_SAME_PAGE_SEARCH, and NC_CROSS_DOC_FLIGHT trace codes and a focused unit matrix. No runtime behavior changes yet; the executors cut over in a following commit.
* refactor(router): execute planned early navigation intent
The App Router shim and browser request loop still own the early navigation intent decisions. navigateClientSide derives hash-only changes inline via isHashOnlyChange, and the request loop derives same-page search changes inline via isSamePageSearchNavigation. That keeps the executors coupled to classification logic that now belongs in the planner, and the two predicates compared search params differently (raw vs normalized).
Replace both inline branches with navigationPlanner.classifyEarlyNavigationIntent. The shim acts on the sameDocumentScroll decision (history update plus hash scroll, no RSC fetch); the request loop consumes only the flightNavigation cache-bypass bit. Each executor still classifies against its own authoritative current reference (the shim against the live document, the loop against the committed snapshot), so redirect-hop and committed-state semantics are preserved while the decision logic lives in one tested place.
Delete isHashOnlyChange and isSamePageSearchNavigation along with their now-unused imports. The removed isHashOnlyChange startsWith("#") fast path was dead for the resolved fullHref argument, which toBrowserNavigationHref always prefixes with a path, so behavior is unchanged. The Pages Router keeps its own isHashOnlyChange in router.ts.
* fix(router): require same origin for same-document navigation intent
classifyEarlyNavigationIntent compared only the base-stripped pathname, the serialised search params, and the presence of a hash. It did not compare origin, so a cross-origin target whose path and search happened to match the current document was classified as a same-document scroll (for a hash target) or a cache-bypassing same-page search. navigateClientSide papers over this by filtering external URLs before the planner runs, but the planner is meant to be the semantic authority and should not carry an unstated same-origin precondition, especially as the navigate runtime is an ambient surface.
Gate both same-document outcomes on current.origin === next.origin so a different host falls through to an ordinary flight navigation. Add cross-origin same-path-plus-hash and same-path-plus-search cases to the unit matrix.
Also correct the search-comparison comment: URLSearchParams.toString() normalises encoding but not key order, and we intentionally do not sort, since query order can be observable.
* test(router): pin encoding-equivalent search params to same-page outcome
* refactor(router): consolidate duplicate snapshot-to-path helpers
* test(router): document early intent search normalization
* refactor(router): mark planner inputs as inert
---------
Co-authored-by: James <james@eli.cx>
|
||
|
Nathan Nguyen
|
f5bee1a26f |
fix(app-router): preserve unstable_catchError boundaries (#1906)
* fix(app-router): preserve unstable_catchError boundaries
* test(e2e): add Next-parity test for shouldRerenderAppPageWithGlobalError (#1906)
Add a test proving that for uncaught post-shell RSC errors (with no
client boundary), the server preserves the original HTML stream and the
client renders global-error. This is the Next.js behavior that justifies
keeping shouldRerenderAppPageWithGlobalError unconditional at false.
The server cannot detect userland client boundaries like
unstable_catchError from the route manifest, so the only safe way to
get a global-error rerender is to make a targeted error-boundary test.
The test uses a loading.tsx boundary so the error is truly post-shell,
not caught by the pre-shell error-recovery catch block.
* fix(error): restore sentinel wrapper for falsy thrown values in unstable_catchError
Restores the _CatchErrorState sentinel wrapper ({ thrownValue: unknown }) so
falsy thrown values (null, undefined, 0, "") are routed to the fallback
instead of being treated as no error. Also restores ErrorInfo.error as unknown.
Adds back unit tests for falsy values, class-component lifecycle, and
unstable_retry behavior parity with the component-rendered fallback shape.
Refs: #1906
* fix(error): align ambient ErrorInfo.error type with runtime implementation
* refactor(app-router): remove shouldRerenderAppPageWithGlobalError dead code
The function always returned false and its _options parameter and
ShouldRerenderAppPageWithGlobalErrorOptions type were vestigial.
The call site at app-page-render.ts:892-904 was therefore never taken.
Remove the function, type, import, call site, and the now-redundant
unit test, leaving the shell-level recovery path in
renderAppPageHtmlStreamWithRecovery as the sole error recovery seam.
Addresses ask-bonk review comment on #1906.
* fix(test): restore typed globalAny view and document Fallback as any intent
Addresses two non-blocking review findings from ask-bonk on PR #1906:
- Restored the typed `globalThis as unknown as { window?: unknown }`
pattern for window stubs in the unstable_catchError test section,
replacing the `(globalThis as any).window` pattern that was a type-safety
regression from the prior rewrite.
- Added an inline comment explaining why `Fallback as any` is necessary
in the class-component lifecycle test: the test probes the inner
_CatchError class directly, where the fallback function is used as a
React component type (receiving a single `{ props, errorInfo }` object)
rather than the public API signature `(props, errorInfo)`.
* chore: trigger CI
* fix(test): rename forwardedProps to props in unstable-catchError test
Addresses ask-bonk review finding on PR #1906: the test was seeding
the inner _CatchError class with the stale key name forwardedProps,
but the production wrapper now passes props (and isPagesRouter).
Since this test only exercises the App Router unstable_retry path
(which reads neither key), behavior was unaffected, but renaming
aligns the test with the current boundary shape for future readers.
* refactor(server): remove dead routeHasLocalBoundary field
Removing shouldRerenderAppPageWithGlobalError (commit
|
||
|
Jerry Zhao
|
6d9d16da1e | test(next-config): make CJS config fixture deterministic on Windows (#1922) | ||
|
Jerry Zhao
|
0394b98633 | test(og): pass POSIX-style ids to the og-font-patch transform fixture (#1925) | ||
|
Jerry Zhao
|
9013bf8ca3 | refactor(dev-lockfile): normalize stale lock file path in error message (#1926) | ||
|
Jim Geurts
|
23dd15f763 |
fix(prod-server): stop double-evaluating the server bundle when chunks import the entry back (#1924)
* fix(prod-server): stop double-evaluating the server bundle when chunks import the entry back The production server imported the built server entry with a ?t=<mtime> query string. Node keys its ESM cache on the full URL including the query, so any code-split chunk that resolves the same entry file by its plain URL evaluated the entire server bundle a second time. Default Vite builds on both supported majors (Rollup on Vite 7, Rolldown on Vite 8) hoist modules shared between the entry's static graph and lazy route chunks into the entry chunk, and the chunks import them back as ../../index.js; realpath- canonicalized chunk ids also reach the entry via its real path when the server was started through a symlinked path. Module-level singletons then diverged between the two copies: boot-time initialization ran on the instance the server imported while route handlers read the never- initialized duplicate. Import the entry by its bare canonical (realpathed) file:// URL on first use so all importers converge on one module instance, and only append the ?t= cache-buster when the same path is imported again with a different mtime — the same-process rebuild scenario the query existed for. * docs(prod-server): clarify rebuild-branch tradeoff and probe constraints (#1924) - note in resolveServerEntryImportUrl's JSDoc that the ?t= rebuild branch trades back the single-instance guarantee (bare chunk back-imports keep resolving to the first build); freshness + single instance only hold together on the first import of a path - clarify the realpath try/catch only covers realpathSync.native failures, not missing entry paths (those still throw at statSync, as before) - document that the prod-singleton-state fixture probe is only valid on a freshly built output path |
||
|
Divanshu Chauhan
|
9f9c9249b9 |
perf(server): hoist per-request regex compilation out of SSR streaming and shim hot paths (#1915)
* perf(server): hoist close-tag regexes out of SSR stream boundary scanner Replace per-iteration new RegExp(`</\s*>`, "i") in findTrailingOpenHtmlRewriteExcludedRegionStart with a module-level frozen lookup CLOSE_TAG_RES keyed by the four tag names constrained by HTML_REWRITE_EXCLUDED_REGION_START_RE. Call frequency: runs on every streaming App Router SSR response flush, iterating once per <script>/<style>/<textarea>/<title> open tag in the chunk. A page with N Suspense boundaries compiles O(N) regexes per chunk without this fix. Safety: regexes carry i-flag only (no g/y), so shared instances have no lastIndex state and are safe across concurrent requests. Closes #1910 (task 1/3) * perf(server): hoist path-delimiter regexes in escapePathDelimiters Replace per-call new RegExp(...) in escapePathDelimiters with two module-level constants PATH_DELIMITERS_RE and PATH_DELIMITERS_ENCODED_RE. Call frequency: decodePathParams calls escapePathDelimiters once per path segment of every App→Pages bridge request, so an N-segment path previously compiled N regexes per request. Safety: g-flagged regexes are only used via String.prototype.replace, which always resets lastIndex — no exec/test loop touches these instances. Aligns with the hoisted PATH_DELIMITER_REGEX form already used in routing/utils.ts:90 (the other port of the same Next.js utility). Closes #1910 (task 2/3) * perf(shims): replace per-render regex construction with constants and memoization head.ts — escapeInlineContent: replace new RegExp(`<\/(tag)`, "gi") with a two-entry module-level map INLINE_CLOSE_TAG_RES keyed by tag name. All call sites guard with RAW_CONTENT_TAGS.has(tag) so tag is always "script" or "style". Runs once per inline <script>/<style> element per SSR render. image-config.ts — globToRegex: add module-level globRegexCache Map so each distinct (separator, pattern) pair is compiled only once. matchRemotePattern calls globToRegex twice per remotePatterns entry, and hasRemoteMatch iterates all entries for every remote <Image>, so a page with M patterns and K images previously compiled up to 2·M·K regexes per render from a fixed config. Mirrors Next.js's own Map<string, RegExp> memoization in packages/next/src/server/image-optimizer.ts. Closes #1910 (task 3/3) |
||
|
Nathan Nguyen
|
b802f7e0b0 |
test(e2e): use Playwright fixture pattern for inline-css server lifecycle (#1914)
* fix(e2e): poll for server readiness before navigating in inline-css test The inline-css browser test builds and serves a temporary fixture outside Playwright's webServer config, so server readiness is not guaranteed before page.goto() navigates. Under CI load, the browser can connect before the server is accepting connections, causing ERR_CONNECTION_REFUSED flakiness. Added a waitForServer utility in tests/e2e/helpers.ts that polls the server URL until it responds, then uses it in the inline-css test before navigation. Uses 30s timeout with 300ms polling and 2s per-request timeout, matching the pattern from packages/vinext/src/cloudflare/tpr.ts. Fixes https://github.com/cloudflare/vinext/issues/1913 * fix(e2e): use Playwright fixture pattern for inline-css server lifecycle Replace the waitForServer polling approach with Playwright's fixture pattern (base.extend) to manage the inline-css test's server lifecycle. This matches the pattern used in client-reference-runtime-map.browser.spec.ts. The previous approach added a waitForServer utility to shared helpers.ts, but startProdServer already waits for the server to listen via the server.listen() callback, making the polling redundant. The fixture pattern guarantees the server is ready before the test body runs and handles cleanup in the finally block. |
||
|
MaxtuneLee
|
88a9947e14 |
fix(trailing-slash): canonical url trailing slash support (#1888)
* fix: canonical url trailing slash support * fix(metadata): prevent trailing slash on file-like and .well-known canonical URLs * fix(metadata): remove trailing slash handling for .well-known URLs * test: add tests for createAppPageRouteBodyMetadata function with body placement * fix(metadata): match trailing slash URL scope * fix(metadata): resolve URL object alternates --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
2e83e7f558 |
fix(edge-wasm): handle *.wasm?module imports in non-Cloudflare builds (#1877)
* fix(edge-wasm): handle \`*.wasm?module\` imports in non-Cloudflare builds (#1351) In plain Node.js builds (deploy-suite / standalone `vinext start`), Rolldown has no built-in handler for the `?module` query on `.wasm` imports, so middleware and edge API routes that use: import wasm from './add.wasm?module' would throw at bundle time. Root cause: the `?module` query is a Cloudflare Workers / workerd convention handled by `@cloudflare/vite-plugin`'s `additionalModulesPlugin` (enforce:"pre"). When that plugin is absent (e.g. after `vinext init` which emits a plain `vite.config.ts` with only `vinext()`), no plugin resolves the query. Fix: add a new `vinext:wasm-module-import` Vite plugin (enforce:"pre") that, in the absence of @cloudflare/vite-plugin: 1. Intercepts `*.wasm?module` imports in `resolveId`. 2. Reads the WASM binary at load time and inlines it as base64. 3. Exports `await WebAssembly.compile(buffer)` via a virtual module — valid in Node.js (top-level await in ESM). The `hasCloudflarePlugin` guard is checked inside the hook handler (not as an array-spread condition) so it reflects the value set by the `config` hook after plugin registration. workerd forbids compiling WASM from bytes at runtime, so this path intentionally never runs when @cloudflare/vite-plugin is present. Fixes #1351. * fix(wasm-module-import): suppress no-control-regex lint on virtual module regexes Replace bare null-byte with unicode-escape in the two filter/replace regexes and add oxlint-disable-next-line comments following the same convention used in client-reference-dedup.ts. Fixes the Check job failure. * fix(wasm-module-import): exclude @vercel/og from plugin scope, add resolveId tests - Add importer guard in resolveId to skip imports originating from @vercel/og (resvg.wasm, yoga.wasm): vinext:og-font-patch converts those to dynamic imports with a .catch() disk-read fallback; if wasm-module-import intercepts them, the Node.js fallback never runs, the ~1.3 MB resvg WASM ships twice, and findEmittedWasmAsset dedup in vinext:og-assets breaks. Guard mirrors the pattern in the existing isVinextOgShimImporter() helper. - Remove dead `return null` after `this.error()` (which throws); use `bytes!` non-null assertion to satisfy TypeScript. - Add resolveId test suite: happy path, ?module query stripping, null resolver, og-collision guard (plain + virtual-prefix importer), and non-og pass-through. Fixes ask-bonk findings on PR #1877. * refactor(wasm-module-import): drop non-null assertion, document TLA and filter intent * test(wasm-module-import): add build-level coverage, document runtime-gating assumption * refactor(wasm-module-import): extract to plugins/, guard client environment, watch wasm file in dev * fix(wasm-module-import): delegate target-owned modules * refactor(wasm-module-import): share query stripping |
||
|
Nathan Nguyen
|
d2ffc83670 |
fix(config): define cache components flag as boolean (#1903)
* fix(config): define cache components flag as boolean Disabled cache components currently compile to a truthy "false" string. That diverges from Next.js and makes bundled user code that checks !!process.env.__NEXT_CACHE_COMPONENTS select the cache-components path even when next.config rewrites select the legacy destination. Mirror Next.js by defining process.env.__NEXT_CACHE_COMPONENTS as a boolean expression, and keep the local slot shim tolerant of both boolean defines and string env values. Add a faithful revalidatePath-with-rewrites production regression that proves rewritten static and dynamic routes invalidate after revalidating the resolved destination path. * test(prod-server): cover cacheComponents enabled revalidatePath with rewrites * test(prod-server): skip dist/ copy in cacheComponents fs.cpSync filter Addresses review feedback: fs.cpSync(APP_FIXTURE_DIR, fixtureRoot) copies the earlier-built dist/ directory into the temp fixture before it is immediately deleted. Adding a filter skips the unnecessary copy. |
||
|
James Anderson
|
f5355964aa |
fix(pages-router): set x-nextjs-deployment-id on SSG _next/data responses (#1863)
* fix(pages-router): set x-nextjs-deployment-id on SSG _next/data responses Pages Router `/_next/data/<buildId>/<page>.json` responses were missing the `x-nextjs-deployment-id` response header that Next.js sets on these routes when a deployment ID is configured. The client router reads this header to detect when a new deployment has been rolled out and triggers a hard navigation for deployment-skew protection. Root cause: the `isDataReq` short-circuit path in `pages-page-handler.ts` that builds the JSON envelope (`buildNextDataJsonResponse`) never injected the header. Next.js's `pages-handler.ts` sets `NEXT_NAV_DEPLOYMENT_ID_HEADER` on every `isNextDataRequest` response (success, redirect, and notFound cases). Fix: read `process.env.__VINEXT_DEPLOYMENT_ID || process.env.NEXT_DEPLOYMENT_ID` (the runtime-available deployment ID, inlined at build time by Vite's define transform) and set `x-nextjs-deployment-id` on the `_next/data` response headers when a deployment ID is configured. Maps to test: `pages ssg data deployment skew - hard navigate when a new deployment occurs > header with deployment id > header is set on data routes`. Fixes #1829 * fix(pages-router): complete x-nextjs-deployment-id parity for _next/data (bonk #1829) Address all three ask-bonk findings on PR #1863: 1. Incomplete parity — add x-nextjs-deployment-id to redirect and notFound _next/data exits in pages-page-data.ts (buildPagesRedirectResponse, buildPagesDataNotFoundResponse). Thread deploymentId through ResolvePagesPageDataOptions and pass it from pages-page-handler.ts, matching the success-path pattern already in that file. 2. Dev/prod parity — patch dev-server.ts to set x-nextjs-deployment-id on all three _next/data exits: the writeGsspRedirect isDataReq path, the getStaticPaths/ getServerSideProps/getStaticProps notFound isDataReq paths, and the success JSON envelope short-circuit (isDataReq block). Mirrors the prod handler. 3. Add regression tests asserting x-nextjs-deployment-id is present on _next/data success / redirect / notFound responses in both pages-page-data.test.ts (unit, via deploymentId option) and pages-page-handler.test.ts (integration, via process.env.__VINEXT_DEPLOYMENT_ID), and absent when no env var is set. * fix(pages-router): add x-nextjs-deployment-id to wrong-buildId/route-miss _next/data 404s (#1829) Fixes the deployment-skew gap identified in review: `buildNextDataNotFoundResponse` now includes the `x-nextjs-deployment-id` header so all three wrong-buildId/route-miss exits (normalizePagesDataRequest, prod-server Node path, pages-page-handler route-miss) surface the deployment ID to the client router. Adds unit tests for the new header on all relevant exits. * fix(pages-router): set deployment-id header on remaining two dev-mode _next/data 404 exits (#1829) Patches the stale-buildId 404 in the vite-dev middleware (index.ts) and the route-miss data 404 in createSSRHandler (dev-server.ts) to set x-nextjs-deployment-id, matching prod parity via buildNextDataNotFoundResponse(). * test(pages-router): cover dev-mode _next/data deployment-id header exits Drive the two dev-only exits flagged in review through the real fixture dev server: the wrong-buildId data 404 in the plugin middleware (index.ts) and createSSRHandler's route-miss data 404 (dev-server.ts), plus the dev success path and the no-deployment-id case. * fix(pages-router): skip deployment-id header on /_error and /500 data success responses Next.js pages-handler.ts guards the success-path x-nextjs-deployment-id injection with `!isErrorPage && !is500Page`. Mirror that exclusion in both the prod success short-circuit (pages-page-handler.ts) and the dev one (dev-server.ts), and lock it in with a prod unit test (/_error and /500) plus a dev integration test (/500). |
||
|
Nathan Nguyen
|
71e2e4762a |
refactor(router): plan RSC fetch result classification (#1889)
* fix(router): preserve streamed RSC redirect targets Streamed RSC redirect headers currently duplicate redirect lifecycle handling in the browser loop. That path preserves the target hash, but it does not share the same helper boundary as response URL redirects and cannot be tested independently. The browser loop needs a pure resolver that applies the same external-origin, depth, and same-target laws while treating the streamed header as the visible redirect target. Add resolveStreamedRscRedirectLifecycleHop and focused helper coverage for external redirects, depth exhaustion, same-target detection, target hashes, hash-only changes, and visible query preservation. * feat(router): add RSC fetch result trace codes Fetch-result classification needs trace causes that can distinguish invalid RSC payloads, compatibility hard navigations, redirect follow or terminal outcomes, streamed redirect loops, and normal commit continuation. The existing navigation trace schema only names decoded commit decisions, so pre-decode HTTP/RSC decisions would otherwise be opaque or forced through unrelated reason codes. Add the reason codes and fetch-result fields without changing runtime behavior. * feat(router): classify RSC fetch results in planner RSC navigation currently classifies invalid payloads, compatibility mismatches, response redirects, and streamed redirects inline in the browser request loop. Those decisions are semantic planner decisions, but they are not independently testable while embedded in the executor. Add a pre-decode RSC fetch-result classifier with typed hard-navigation causes, redirect-follow output, body-discard intent, and trace entries. The classifier preserves the existing precedence ladder and routes streamed redirects through the shared lifecycle law. Add a focused unit matrix for invalid payloads, compatibility mismatch, response-URL redirects, streamed-header redirects, same-target loop guarding, target-hash preservation, cached precedence, and normal commit continuation. * refactor(router): execute planned RSC fetch decisions The browser navigation request loop currently owns semantic fetch-result decisions for invalid RSC responses, compatibility mismatches, response URL redirects, and streamed redirect headers. That keeps the hot executor coupled to decision logic that now belongs in the planner. Replace those inline branches with navigationPlanner.classifyRscFetchResult calls for cached and live responses. The executor now performs only effects: diagnostics, body discard, hard navigation, redirect loop variable updates, and commit fallthrough. This preserves existing history-mode fallback and response-URL behavior while applying the streamed same-target loop guard introduced by the planner classifier. * chore(router): keep RSC fetch planner exports tight The fetch-result planner cutover added helper-only types and left the redirect depth constant exported after the browser executor stopped importing it. Those symbols are internal implementation details, and keeping them exported makes the new planner surface noisier than the runtime contract requires. Make the helper-only aliases and redirect depth constant internal. Re-run focused tests, knip, full check, app-router E2E coverage, and the vinext build after the cleanup. * chore: retrigger PR checks * ci: make release checkout credentials explicit The zizmor check flags the release workflow checkout because it relies on the default persisted checkout token. That token is intentionally needed by changesets/action to push the version branch, tags, and releases. Set persist-credentials: true explicitly so the accepted release requirement is encoded in the workflow instead of depending on the checkout default. * chore: retrigger preview publish * refactor(router): reuse streamed redirect target * refactor(router): share RSC redirect lifecycle ladder * Apply suggestion from @james-elicx --------- Co-authored-by: James Anderson <james@eli.cx> |
||
|
James Anderson
|
f6b07eebcc |
fix(pages-router): buffer SSR response for crawler/bot User-Agents (#1876)
* fix(pages-router): buffer SSR response for crawler/bot User-Agents Pages Router edge-runtime SSR was streaming HTML to all clients, including crawlers like Googlebot and Google-PageRenderer that cannot parse incrementally flushed HTML. This caused the Next.js compat test `streaming-ssr-edge/should not stream to crawlers or google pagerender bot` to fail: bots received multi-chunk responses without an ETag. Mirror Next.js's behaviour from `send-payload.ts`: detect crawler UAs via `isPagesStreamingBot()` (matching Next.js's html-bots.ts + is-bot.ts regex set), await the full compositeStream, generate a FNV-1a 52-bit ETag, and return a single-chunk Response. Normal browser UAs continue to stream. * fix(pages-router): add If-None-Match→304, ISR-HIT ETag, and UA-gating divergence comment - Export `generatePagesETag` and add `etagMatches` helper (weak/strong ETag comparison mirroring Next.js `sendEtagResponse` / RFC 7232 §3.2). - `renderPagesPageResponse`: accept `ifNoneMatch` option; return 304 when the buffered bot response's ETag matches, with an empty body. - `resolvePagesPageData`: accept `userAgent` + `ifNoneMatch`; attach an ETag to ISR cache-HIT responses for bot UAs (consistent with fresh-MISS path); return 304 on HIT when If-None-Match matches. - `pages-page-handler`: plumb `if-none-match` request header into both `resolvePagesPageData` and `renderPagesPageResponse`. - Add a clear divergence comment in `renderPagesPageResponse` documenting that vinext gates buffering/ETag on the crawler UA rather than Next.js's `isDynamic`-based flag. - Tests: 8 new cases covering 304/200 on match/mismatch, non-bot 304 bypass, weak-ETag comparison, ISR-HIT ETag attachment, and ISR-HIT 304. * fix(pages-router): address ask-bonk review — STALE ETag/304, comment fixes, no-cache - Hoist repeated bot ETag/304 logic into `applyBotETagAndCheck` helper so the same code covers both the ISR cache-HIT and cache-STALE branches; previously the STALE branch silently omitted ETag headers and 304 short-circuits for crawler UAs - Add `requestCacheControl` option to `RenderPagesPageResponseOptions` and `ResolvePagesPageDataOptions`; skip the 304 short-circuit when the request carries `Cache-Control: no-cache` (matches `fresh` package behaviour used by Next.js's `sendEtagResponse`) - Fix comment: ETag section header said "weak ETag by default" — Next.js `generateETag()` defaults to strong ETag (`weak=false`); update to match - Fix comment: `etagMatches` said Next.js uses the `etag` npm package with `weak: true`; it actually uses the `fresh` package for staleness checking; replace with accurate description - Add tests: STALE-branch ETag attach, STALE 304, STALE non-bot skips 304, and `no-cache` bypasses 304 on all three paths (fresh-MISS, HIT, STALE) * docs(pages-router): correct RFC 7232 citation for weak ETag comparison (§3.2 → §2.3.2) * docs(pages-router): note 304-on-dynamic consequence of UA-gated ETag divergence * refactor(pages-router): share the no-cache bypass check between MISS and ISR bot paths * refactor(pages-router): move fnv1a52 ETag hash into utils/hash.ts alongside fnv1a64 |
||
|
James Anderson
|
ce29f9c6aa |
fix(scss): preprocess SCSS CSS-module composes deps via SassAwareFileSystemLoader (#1882)
* fix(scss): preprocess SCSS CSS-module composes deps via SassAwareFileSystemLoader postcss-modules' built-in FileSystemLoader reads files referenced by `composes: className from './other.module.scss'` as raw text — without Sass preprocessing. For .scss files this leaves SCSS variables and bare @import directives in the CSS output, causing LightningCSS to crash with "Invalid empty selector" during production builds. Fix: introduce SassAwareFileSystemLoader that delegates to Vite's `preprocessCSS` API for every `composes:` dependency, so Sass compilation runs before CSS-module scoping. Register it via `css.modules.Loader` in vinext's Vite config hook and thread the resolved config through via a new `setSassLoaderResolvedConfig` call in the `configResolved` hook. Fixes the following deploy-suite tests (both sass and sass-embedded variants): - test/e2e/app-dir/scss/nm-module/nm-module.test.ts - test/e2e/app-dir/scss/nm-module-nested/nm-module-nested.test.ts - test/e2e/app-dir/scss/composes-external/composes-external.test.ts Part of #1825 (non-tilde CSS-module/composes cases; complements #1881) * fix(scss): address review feedback on SassAwareFileSystemLoader - Guard the css.modules.Loader injection: respect a user's explicit css.modules: false (mergeConfigRecursively would otherwise re-enable CSS Modules with an object override) and never clobber a user-provided custom Loader. The override is precomputed as a typed const so the conditional spread doesn't disturb the config literal's contextual typing. - Surface composes preprocessing failures via config.logger.warn instead of a bare catch {}, so missing composed classes are observable; a missing/unreadable dependency file now rejects like postcss-modules' FileSystemLoader did. - Use Vite's exported preprocessCSS + PreprocessCSSResult type via a top-level import instead of the lazy dynamic import and stringly typed casts. - Preserve parity for composes from non-*.module.* files: hand preprocessCSS a virtual *.module.* filename so plain '.css' deps are still scoped and export tokens extracted, matching the built-in FileSystemLoader. - Port the three failing Next.js fixtures (composes-external, nm-module, nm-module-nested) as production-build regression tests in tests/scss-composes.test.ts, plus a plain .module.css / non-module .css parity test. All three SCSS tests reproduce the original "[lightningcss minify] Invalid empty selector" crash when the Loader injection is disabled. - Add sass as a root devDependency so the SCSS tests (including the previously always-skipped tests/scss.test.ts) actually run in CI; drop the now-redundant knip ignore entry. Installing sass makes Vite's SassPreprocessorOptions resolve to the concrete sass types (previously any), so the tilde-importer's importers array now carries an explicit SassPreprocessorOptions["importers"] cast. * fix(scss): bind composes-loader config per plugin instance; scope extensionless deps - Replace the module-level resolved-config singleton with a per-vinext- instance binding (createSassAwareFileSystemLoader), so multiple builds in one process never preprocess composes deps with another build's config. - Cover extensionless composes deps in the virtual *.module.* rename (append .module.css) so their tokens are scoped instead of silently dropped; add a parity test case. - Pin url() pass-through behaviour for composed deps in the parity test (verbatim, never mangled into placeholders — matches the built-in FileSystemLoader, verified against a vanilla Vite build). * docs(scss): document the per-subtree recursion boundary of the composes loader Notes the two structural consequences of delegating nested composes recursion to preprocessCSS (per-subtree duplicate inlining collapsed by LightningCSS, and no cross-boundary cycle short-circuit for invalid circular composes), per review. * fix(scss): fail the build on composed-dep preprocessing errors Only a missing Sass implementation is downgraded to a logged warning; any other preprocessCSS error (e.g. a Sass syntax error in the composed dependency) now propagates and fails the build, matching postcss-modules' built-in FileSystemLoader instead of shipping a green build with silently-missing classes. Docstrings updated to accurately describe the silent empty-token path when no resolved config is bound. Adds a regression test for the fail-loudly path. * chore(deps): move sass to the workspace catalog; drop the now-dead dynamic skip in scss.test.ts Review feedback on #1882: sass belongs in the pnpm workspace catalog rather than pinned inline in the root package.json. With sass now a guaranteed root devDependency, the runtime describeIfSass skip in tests/scss.test.ts is dead code — the suite always runs — so make the unskip explicit by removing the conditional machinery. * rebuild lockfile |
||
|
Jerry Zhao
|
55815233c8 |
fix(routing): normalize route root to forward slashes, drop downstream path workarounds (#1896)
* routing: normalize the route root to forward slashes and drop downstream workarounds * fix(routing): compare findOwnerRouteForDir paths in forward-slash space With appDir now normalized to forward slashes, the ancestor walk's current === appDir termination never fired on Windows (current descends through native path.join/path.dirname and stays backslash), so the walk overshot the app root and could attach a sibling intercept to the wrong owner. Top-level markers had the inverse mismatch: a forward-slash dir probed against backslash routesByDir keys. Normalize dir, appDir, routesByDir keys, and route file paths to forward slashes inside the lookup so all comparisons happen in one separator space, and add POSIX-runnable unit tests simulating the Windows path shapes. * refactor(routing): use normalizePathSeparators in findOwnerRouteForDir Swap the inline backslash replaces for the shared util. It is a platform-gated no-op on POSIX, so the Windows-shape unit tests mock it to keep exercising the normalization logic on POSIX CI. --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
835e6b9943 |
fix(app-router): suppress redirect console errors in production onCaughtError (root-layout-redirect) (#1878)
* fix(app-router): suppress redirect console errors in production onCaughtError (#1830) When redirect() is called from a 'use client' root layout (or any client component), React catches the NEXT_REDIRECT sentinel via RedirectErrorBoundary and then calls the hydrateRoot onCaughtError callback. In dev mode vinext already filtered these via devOnCaughtError, but the production path had no onCaughtError at all — React's built-in fallback calls console.error, producing a spurious browser console error that fails the Next.js compat test assertion `expect(foundErrors).toBe(false)`. Fix: add prodOnCaughtError to app-browser-error.ts that silently drops navigation signal errors (NEXT_REDIRECT, NEXT_NOT_FOUND, NEXT_HTTP_ERROR_FALLBACK) and logs all other caught errors, preserving React's default behavior. Add fixture tests/fixtures/root-layout-redirect/ (dedicated 'use client' root layout with redirect()) and Playwright project root-layout-redirect (port 4184) with spec porting the Next.js root-layout-redirect > should work using browser. Part of #1830 * fix: include result/ page in root-layout-redirect fixture * fix: add lockfile entry for root-layout-redirect fixture, use prod server, import isNavigationSignalError Three fixes to unblock catastrophically-failing CI: 1. pnpm-lock.yaml: Add missing importer entry for tests/fixtures/root-layout-redirect. The fixture was added without updating the lockfile, causing ERR_PNPM_OUTDATED_LOCKFILE on every CI job during `vp install`. 2. playwright.config.ts: Switch the root-layout-redirect server from `npx vp dev` (dev mode) to `vinext build + vinext start` (production mode). The fix under test — prodOnCaughtError — is only active in production builds; running in dev mode exercised the pre-existing devOnCaughtError path and never touched the new code. 3. app-browser-error.ts: Replace the inline isNavigationSignalDigest helper with an import of isNavigationSignalError from utils/navigation-signal.ts. The module is dependency-free and already in the production browser bundle; inlining it reintroduced the drift risk that navigation-signal.ts was created to prevent. Addresses ask-bonk findings: blocking (test ran in dev mode) and maintainability (inline classifier duplication). * test: add prodOnCaughtError unit tests, run root-layout-redirect e2e in CI, scope gitignore negation |
||
|
James Anderson
|
9194cb55c6 |
fix(pages-router): hard-navigate to App Router destinations from Pages Router links (#1879)
* fix(pages-router): hard-navigate to App Router destinations from Pages Router links
When a Pages Router <Link> targets an App Router route (detected via the
prefetch manifest and recorded as `components[path] = { __appRouter: true }`),
`performNavigation` now checks for that marker and immediately calls
`window.location.assign/replace` instead of attempting a Pages Router SPA
fetch cycle that cannot succeed.
Mirrors Next.js router.ts:1448-1453. Fixes the deploy-suite test
`pages-to-app-routing > should work using browser`.
- packages/vinext/src/shims/router.ts: add __appRouter hard-nav guard
- tests/fixtures/app-basic/app/about/page.tsx: add id="app-page" to <h1>
- tests/fixtures/app-basic/pages/pages-to-app/[slug].tsx: new GSSP fixture page
- tests/e2e/app-router/pages-to-app-routing.spec.ts: new Playwright regression test
* fix(pages-router): replace no-explicit-any cast with in-operator type narrowing
`(getPagesRouterComponentsMap()[_appPath] as any)?.__appRouter` triggered
the `no-explicit-any` lint rule. Replace with a two-step lookup that uses
the `in` operator so TypeScript narrows the union type
`{ __appRouter: true } | Record<string, unknown>` without an `any` cast.
* fix(pages-router): normalize trailing slash in __appRouter marker key, rename locals, pin fast-path test
- Strip trailing slash on both the write side (markAppRouteDetectedOnPrefetch)
and the read side (performNavigation) so the marker key agrees when
trailingSlash:true adds a slash to the prefetch URL but not to the nav URL
(or vice-versa via router.prefetch with a raw un-normalised path).
Mirrors Next.js: removeTrailingSlash(removeBasePath(pathname)) at router.ts:1442.
- Rename _appPath/_appPathEntry to appPath/appPathNorm/appPathEntry (they are
used, not ignored; the underscore prefix was misleading).
- Add two new unit assertions in pages-router-app-prefetch-detection.test.ts:
one pins that the marker key is always stored without trailing slash, and one
pins that performNavigation calls window.location.assign (the fast-path) when
the __appRouter marker is present, so a regression in the new code path cannot
be silently masked by the pre-existing fallback.
* test(pages-router): address review nits in pages-to-app navigation tests
- e2e: wait for App Router hydration after the hard navigation so the
test asserts the App Router landing explicitly instead of only
matching the new DOM
- unit: drop the unnecessary microtask tick before asserting
window.location.assign — the fast path has no await before it, so
asserting synchronously also pins that the hard navigation cannot
lose a race against the SPA path
|
||
|
Nathan Nguyen
|
08504a7745 |
fix(app-router): preload next/dynamic chunks with CSP nonce (#1594)
* fix(app-router): preload next/dynamic chunks with CSP nonce
Rendered next/dynamic boundaries produced no SSR preload links for their client chunks. That diverged from Next.js and broke nonce-based CSP tests that inspect preload tags before hydration.
The runtime assumed React.lazy alone was enough for dynamic imports. It never carried the dynamic module IDs from the source call into SSR, and client JS chunk URLs did not use Next.js's static/chunks path shape.
Add a focused dynamic metadata transform, resolve boundary files from the client build manifest, emit nonce-bearing preload hints during SSR, and keep client JS assets under _next/static/chunks. Ported regression coverage verifies the nonce and the emitted path shape.
* fix: honor assetPrefix for dynamic preload globals
* test: cover asset-prefixed dynamic preload nonces
App Router production only covered the default dynamic preload URL path. That left assetPrefix regressions at the manifest-to-runtime emission boundary unguarded.
Add a startProdServer regression that builds app-basic with assetPrefix "/cdn" and asserts nonce-bearing next/dynamic chunk preloads render under /cdn/_next/static/chunks/.
* test: isolate asset-prefixed prod server globals
The assetPrefix regression starts a second production server in the same Vitest process. startProdServer and the generated RSC runtime install global manifest and module-loader state, so leaving the temporary build's globals in place breaks later production requests in the shared server.
Snapshot and restore the relevant globals around the temporary /cdn server while keeping the integration assertion at the real runtime boundary.
* refactor: reuse shared record guard in dynamic preload metadata
The dynamic preload metadata transform carried a local object guard even though the codebase already has a shared record predicate. That made the AST helper drift from the existing non-array record invariant used elsewhere.
Reuse isUnknownRecord from utils/record and keep the plugin-specific AST alias local to the transform.
* fix: make dynamic preload metadata binding-aware
The next/dynamic metadata transform matched calls by imported identifier text. Shadowed parameters and block bindings with the same name were therefore mutated even when they no longer referred to the next/dynamic import.
Walk the parsed module with lexical binding state so shadowed names are excluded, and collect object-form imports only from loader/modules rather than every property on the options object.
* fix: model switch and class scopes in dynamic preload transform
The dynamic preload metadata transform still treated imported next/dynamic names as active inside switch case lexical scopes and named class expression bodies. That could inject loadableGenerated metadata into calls that resolve to local bindings rather than the import.
The scope walker now removes switch case-block bindings while traversing case tests and consequents, leaves the switch discriminant in the outer scope, and scopes named class declarations and expressions through their children. Regression tests cover switch case shadowing and named class expressions.
* fix: address PR review feedback for next/dynamic CSP nonce
- Drop `as="style"` from `<link rel="stylesheet">` — `as` is a
preload attribute per the HTML spec, semantically incorrect on
stylesheet links. Browsers ignore it but it produces bad HTML.
- Remove brittle `import(` string gate from the transform — it could
miss `import ("./x")` (whitespace between import and paren). The
Vite filter already gates on "next/dynamic" and the AST parse is the
real validation.
- Add comment explaining MagicString mutation safety in Promise.all.
- Add comment explaining AST end-exclusive assumption on closeParen.
- Add test for whitespace-separated dynamic import syntax.
* fix: update global.d.ts comment to mention asset prefix, fix formatting
* refactor: extract computeClientRuntimeMetadata shared helper
Extract the duplicated client-manifest runtime metadata computation
(lazy chunks, dynamic preloads, client entry) into a single shared
helper in utils/client-runtime-metadata.ts.
Both prod-server.ts (Pages Router Node.js production server) and
index.ts (Cloudflare closeBundle hook) now call the same helper,
eliminating the highest-risk seam in the PR where basePath/
assetPrefix handling produced bugs.
* test: add TSX-generic and object-form metadata regression tests
- Add post-type-strip generic call test verifying multi-line
dynamic() calls work after TS/JSX stripping by esbuild.
- Add object-form existing loadableGenerated preservation test
to guard against duplicate injection in the object overload.
* chore: remove unused ClientRuntimeMetadata type export
* revert: discard unrelated lockfile change from previous commit
* test: replace deploy test simulators with computeClientRuntimeMetadata
simulateCloseBundleAppRouter and simulateCloseBundlePagesRouter
no longer hand-roll lazy chunk and dynamic preload computation.
They call the shared computeClientRuntimeMetadata helper directly,
matching production wiring.
Also:
- Update helper doc comment to mention Node server startup
- Remove unused imports (manifestFileWithAssetPrefix,
manifestFileWithBase, computeDynamicImportPreloads,
dynamicImportPreloadsWithBase) from deploy.test.ts
* test: add absolute assetPrefix CSP nonce tests and clean up redundant globals
* fix(app-router): keep CSP nonce on next/dynamic preloads from Server Component call sites
DynamicPreloadChunks rendered in the environment of the dynamic() call site. For a Server Component call site it ran in the RSC environment, where the script-nonce React context is unavailable, so the emitted dynamic preload <link>s dropped the request nonce — a CSP violation under `script-src 'nonce-…' 'strict-dynamic'`. The existing tests missed this because every fixture called dynamic() from "use client" modules, which render in the SSR pass where the nonce is present.
Extract DynamicPreloadChunks into its own "use client" module (mirroring Next.js's PreloadChunks and vinext's next/script shim) so it always renders in the SSR pass where withScriptNonce() installs the provider, regardless of whether dynamic() is called from a Server or Client Component.
Add fixture regression tests: server-call-site JS and CSS preloads carry the nonce, an ssr:false boundary emits no server preload (Next.js loadable.tsx parity), and preloads are still emitted without a nonce when no CSP is set.
* test(app-router): add browser CSP-nonce e2e for Server Component next/dynamic call site
The existing csp.spec.ts browser test only exercises the client call-site page (/nextjs-compat/dynamic). Add a Playwright test for the Server Component call site (/nextjs-compat/dynamic/rsc-imports-client) asserting the lazily-loaded client widget hydrates under script-src 'nonce-…' 'strict-dynamic' with no CSP violations in the console.
* fix(app-router): correct next/dynamic preload key-spaces and harden the transform
Addresses review findings on the next/dynamic CSP-nonce feature.
- Lazy chunks must stay in the SSR-manifest key-space (basePath only); only dynamic preloads take the assetPrefix. Applying an absolute-URL assetPrefix to lazy chunks broke the Pages Router modulepreload-exclusion membership test and leaked lazy chunks into <link rel=modulepreload>.
- Resolve symlinks on both sides when computing a dynamic boundary's module ID (reusing relativeWithinRoot/tryRealpathSync), so the key matches the client manifest under pnpm/Cloudflare symlinked layouts instead of silently dropping the preload.
- Throw on dynamic() calls with >2 args (Next.js react-loadable parity).
- Insert the generated options arg right after the first arg, preserving comments/whitespace (the previous substring-comma scan ate commas inside comments).
- Recurse into _next/static/chunks/ in the on-disk client-entry fallback (client JS now lands under chunks/).
- Add typeof-window guard to DynamicPreloadChunks and document the deliberate as="style" omission for stylesheet links.
- Extract buildRuntimeGlobalsScript so the Cloudflare closeBundle injection shares one source of truth with the deploy tests (which now mirror includeClientEntry + the mixed app+pages branch).
Tests: lazy/dynamic key-space split incl. absolute-assetPrefix leak regression and a realistic already-prefixed manifest; mixed app+pages client-entry injection; symlinked module-id resolution; nested dynamic() calls; >2-arg throw; comment-safe insertion; ssr:false positive content assertion; honest dev-mode E2E naming (preload-nonce coverage lives in the prod-server Vitest suite).
* fix(app-router): fix parenthesized next/dynamic loader corruption + harden re-review findings
Follow-up to the re-review of
|
||
|
James Anderson
|
9ba0772a2e |
fix(middleware): clear nextUrl.basePath for absolute paths outside basePath (part of #1830) (#1872)
* fix(middleware): clear nextUrl.basePath for absolute paths outside basePath (part of #1830) * fix(middleware): reconcile NextURL basePath tests and App Router regression Address two blocking issues from ask-bonk review on #1872: 1. Update 10 unit tests in tests/shims.test.ts that used basePath-stripped URLs (e.g. http://localhost/dashboard with basePath="/app") — these tests encoded the old incorrect behavior where basePath was always set from config regardless of the URL. Switch them to prefixed input URLs (http://localhost/app/dashboard) to match the new correct semantics. 2. Fix App Router regression: createNextRequest in middleware-runtime.ts was receiving normalizedPathname already stripped of basePath (App Router passes cleanPathname), so the NextRequest URL had no basePath prefix and _stripBasePath incorrectly cleared basePath to "". Fix by applying addBasePathToPathname before constructing the URL, mirroring the un-stripped URL that Next.js's adapter always passes to middleware. Also use addBasePathToPathname helper in prod-server.ts and deploy.ts closures instead of duplicating the root-path edge-case logic. * test(middleware): fix stale test assertions broken by basePath wrapper change Update 9 tests in deploy.test.ts that checked for the old simple runMiddleware passthrough ('runMiddleware: typeof runMiddleware === "function" ? runMiddleware : null') which no longer matches the generated code after the basePath re-add wrapper was introduced in the deploy adapter. Fix 1 test in app-route-handler-runtime.test.ts that incorrectly expected NextURL to re-add the basePath prefix to a URL that was already stripped of it. Per getNextPathnameInfo semantics (the fix this PR implements for #1830), basePath is only set when the pathname actually starts with the configured prefix — a stripped URL stays stripped. * fix(middleware): gate basePath re-add on in-basePath state and restore route handler URL parity - Fix the missed import assertion in tests/deploy.test.ts (the generated entry now imports addBasePathToPathname alongside hasBasePath/stripBasePath). - Gate the basePath re-add in createNextRequest on a new hadBasePath option: the unconditional re-add regressed the Pages out-of-basePath flow by re-prefixing absolute-path requests the adapters deliberately left bare, making middleware see nextUrl.basePath === "/root" instead of "". The flag defaults to URL-derived (correct for prod/deploy Pages adapters) and is asserted true by callers that pass pre-stripped URLs (App Router, dev server). - Re-add basePath in createTrackedAppRouteRequest so App Route handlers see the original prefixed request.url / nextUrl.href and an active nextUrl.basePath, matching Next.js (the routing layer strips basePath before handlers run). - Re-derive the active basePath from the configured value in NextURL._stripBasePath on every parse, so href reassignment toggles basePath like Next.js NextURL.analyze(). - Add unit tests covering the App Router nextUrl.basePath re-add, the Pages in-/out-of-basePath flows, matcher evaluation against stripped paths, and href re-derivation. * refactor(middleware): extract shared wrapMiddlewareWithBasePath helper The runMiddleware basePath re-add closure was duplicated verbatim in prod-server.ts and the generated worker entry in deploy.ts. Extract it to wrapMiddlewareWithBasePath in server/pages-request-pipeline.ts (both adapters already import from that module) to keep the two adapters in sync. * test(middleware): add unit coverage for wrapMiddlewareWithBasePath Covers the helper's gating contract directly: pass-through when hadBasePath is false or basePath is empty, prefix re-add (preserving query, headers, ctx, and opts), and idempotent re-add for an already-prefixed URL. |
||
|
Jerry Zhao
|
e4ac3b6778 | refactor(optimize-imports): drop dead packageName/resolveEntry params from buildBarrelExportMap (#1895) | ||
|
Jerry Zhao
|
80b858acc6 | refactor(optimize-imports): normalize resolvePackageEntry paths via normalizePathSeparators (#1894) | ||
|
Jerry Zhao
|
31fed9ae21 | refactor(optimize-imports): resolve barrel re-export paths with path.posix (#1892) | ||
|
James Anderson
|
206b016303 |
fix(cloudflare): update cache adapter jsdoc and examples (#1898)
* docs: remove cdn adapter from examples * update adapter docs |
||
|
Divanshu Chauhan (divkix)
|
743f252b7a |
refactor(isr): centralise HIT/STALE/MISS + Cache-Control decision in isr-decision.ts (#1883)
* refactor(isr): centralise HIT/STALE/MISS + Cache-Control decision in isr-decision.ts Add server/isr-decision.ts as the single owner of the ISR cache policy decision across all four call sites (app-page-cache, app-route-handler, pages-page-data, dev-server). Every cache disposition, background-regen flag, and Cache-Control string now flows through decideIsr() or its MISS helpers. Also eliminate all hardcoded Cache-Control literals outside cache-control.ts (NEVER_CACHE_CONTROL, NO_STORE_CACHE_CONTROL, and raw s-maxage strings in dev-server.ts, pages-page-response.ts, pages-page-handler.ts). One deliberate behaviour change: dev STALE responses now emit s-maxage=0, stale-while-revalidate (matching prod Pages Router) instead of s-maxage=<revalidate>, stale-while-revalidate. This closes a dev/prod parity gap that was masked by the duplication. Closes #1783 * fix(isr): un-export IsrDisposition and IsrPolicyKind (knip unused-export) * docs(isr): tighten isr-decision docstring and hoist resolveRevalidate * refactor(isr): pass cacheState directly to decideIsr, drop hasUsableValue/isStale Replace the two-field hasUsableValue+isStale decomposition with a single cacheState: "HIT" | "STALE" | "MISS" parameter that callers already have. Also remove the before/after equivalence table from the module docstring, which is only relevant during the migration window. All five call sites updated; tsc --noEmit passes with no errors. * test(isr): update decideIsr tests for cacheState |
||
|
Nathan Nguyen
|
a043347c30 |
refactor(app-router): extract browser history traversal into AppBrowserHistoryController (#1884)
* refactor(pages-router): scope PagesReadinessNextData to its module PagesReadinessNextData was exported but only referenced inside pages-readiness.ts, as the return-type annotation of buildPagesReadinessNextData. No other module imports the type by name, so the export is dead public surface that knip flags as an unused exported type. Drop the export keyword so the alias stays a module-local type. Callers consume buildPagesReadinessNextData's inferred return type, so the public behavior and the readiness shape are unchanged. * refactor(app-router): extract browser history traversal into AppBrowserHistoryController The App Router browser entry owned a coherent cluster of history-traversal responsibilities inline: current/next traversal-index state, index allocation and commit bookkeeping, push/replace/traverse/hash-only history-state writes, hash-only scroll-metadata stripping, previousNextUrl/BFCache id synchronization, restorable-snapshot resolution, and the initial hydrated history writes. That made app-browser-entry.ts more than browser-runtime bootstrap glue and left history-traversal behavior hard to unit test directly. Move that cluster behind a typed AppBrowserHistoryController under server/. The controller owns the traversal-index state machine, history metadata writes, BFCache epoch and snapshot invalidation through RestorableClientStateController, hash-only scroll stripping, restorable-snapshot candidate resolution, and the traversal-index commit that runs after, and only after, an injected approved visible-restore callback succeeds. Browser-history I/O is injected so the controller is exercised without a DOM. Keep the ownership boundary intact. AppBrowserNavigationController remains the only authority for approved visible router-state commits: the entry still owns the restoreHistorySnapshotVisibleState() call, stageClientParams(), and commitClientNavigationState(). The history controller resolves history facts and delegates visible restoration through the injected approved-commit callback. It never imports applyApprovedVisibleCommit(), sets router state directly, or bypasses the ApprovedVisibleCommit boundary. Behavior is unchanged: the moved logic is identical, including the basePath target checks and server-action cache invalidation semantics. Adds focused controller unit tests for index allocation, hash-only scroll stripping, epoch sync, and approved-restore delegation. The #1743 scroll-restoration, hash-popstate, autoscroll, and navigation E2E suites stay green. Closes #1818. |
||
|
Jerry Zhao
|
6107113fa9 | fix(fonts): normalize shims dir so shim-skip guards match on Windows (#1885) | ||
|
Jerry Zhao
|
b3215cea30 | fix(check): normalize scanned file paths to forward slashes on Windows (#1887) | ||
|
Jerry Zhao
|
2884125070 | fix(shims): use path.posix.join in resolveShimModulePath for Windows (#1886) | ||
|
James Anderson
|
7d502f6de5 |
fix(scss): resolve tilde (~) imports from node_modules and project root (#1881)
* fix(scss): resolve tilde (~) imports from node_modules and project root (#1825) Next.js (via sass-loader's webpackImporter) supports two tilde import forms in SCSS: `~pkg/path` (node_modules) and `~/path` (project root). Vite's built-in Sass resolver does not strip the `~` prefix, so any SCSS using tilde imports fails with "Can't find stylesheet to import". This adds `createSassTildeImporter(root)` – a Sass FileImporter (modern API) injected first in `css.preprocessorOptions.scss.importers` for every vinext project. It: - Returns `null` for non-tilde URLs, passing through to Vite's resolver - `~/path` → resolves relative to the Vite project root - `~pkg/path` → fast-path checks `<root>/node_modules/<pkg>` directly; falls back to `createRequire`-based resolution for hoisted pnpm graphs, yarn PnP, and workspace monorepos The importer is always injected (not just when sassOptions is set) so test fixtures that don't configure sassOptions still get tilde support. Fixes #1825 Affected tests: - test/e2e/app-dir/scss/npm-import-tilde (tilde imports) - test/e2e/app-dir/scss/nm-module (CSS module from node_modules) * fix(scss): fix lint errors in sass-options test (unused import, unsafe optional chaining) Remove unused `import fs from "node:fs"` and replace unsafe optional-chaining casts (`opts?.scss?.importers as any[]`) with guarded local variables so oxlint's no-unsafe-optional-chaining rule is satisfied. Also applies the formatter fix already caught by `vp check --fix`. |
||
|
James Anderson
|
0df69782d7 |
fix(pages-router): bridge deprecated Router.on<Event> property callbacks (#1869)
* fix(pages-router): bridge deprecated Router.on<Event> property callbacks
Next.js's singleton Router supports a legacy pattern where userland code
sets a property directly on the Router singleton instead of using
Router.events.on():
Router.onRouteChangeComplete = (url) => { ... }
The reference implementation in packages/next/src/client/router.ts
(lines 105-124) bridges this by listening on Router.events for each
known event and calling the corresponding Router.on<Event> property
if it is set.
Vinext's Router singleton was missing this bridging, so the deprecated
pattern silently did nothing. The e2e test
`withRouter > production mode > allows observation of navigation events
using top level Router deprecated behavior` navigates from /a to /b and
asserts that .active-top-level-router-deprecated-behavior updates — it
never did because Router.onRouteChangeComplete was never invoked.
Fix: after the Router singleton is constructed, register one routerEvents
listener per known event that forwards calls to the corresponding
Router.on<EventName> property when present, matching the reference
implementation's behaviour exactly.
* test(pages-router): add unit tests for deprecated Router.on<Event> bridge; address ask-bonk nits
- Add 4 focused tests in "deprecated Router.on<Event> property bridge" describe block:
onRouteChangeComplete fires the assigned handler with correct args;
onRouteChangeStart fires the assigned handler with correct args;
emitting with no handler assigned does not throw;
clearing the handler stops it from being invoked.
- Rename _deprecatedRouterEvents → deprecatedRouterEvents (underscore implied unused).
- Remove redundant `as string` cast on the template-literal eventField expression.
- Add one-line comment explaining SSR safety of unconditional module-load registration.
|
||
|
James Anderson
|
fd919d4132 |
fix(app-router): extend OTel tracer provider for Cache Component span context (#1868)
* fix(app-router): extend OTel tracer provider after instrumentation register() for Cache Component spans During Cache Component fallback resume, `workUnitAsyncStorage` carries a prerender/cache store. Without the tracer extension, calls to `tracer.startActiveSpan()` / `tracer.startSpan()` inside user RSC code inherited that frozen context, causing span IDs to be reused across requests or not created at all. Mirrors Next.js's `extendInstrumentationAfterRegistration()` in `instrumentation-node-extensions.ts`: wraps the OTel tracer provider's `startSpan` and `startActiveSpan` to exit `workUnitAsyncStorage` when creating spans, then re-enters the store for the `startActiveSpan` callback. Fixes #1495 * fix(otel-tracer-extension): address bonk review findings BLOCKING: replace bare require("@opentelemetry/api") with globalThis.require guard so the call is safe in ESM Worker bundles where bare require is undefined; matches the pattern in client-trace-metadata.ts. SHOULD-FIX #1: restore the isUseCacheFunction warning when a "use cache" function is passed to startActiveSpan — mirrors upstream instrumentation-node-extensions.ts. Adds USE_CACHE_FUNCTION_SYMBOL tag in registerCachedFunction and the inline isUseCacheFn check in the extension (avoids importing the full cache-runtime dep graph from this lightweight file). SHOULD-FIX #2: add focused unit tests in tests/otel-tracer-extension.test.ts covering no-op paths, startSpan ALS exit, startActiveSpan ALS re-entry, use-cache warning, provider/tracer double-wrap guards, and provider-swap re-wrapping. Isolated from the full vinext plugin import graph so they run without optional build-time packages. SHOULD-FIX #3: replace module-global boolean tracerProviderExtended with a WeakSet<object> (extendedProviders) so that a swapped provider gets wrapped rather than being silently skipped forever. NIT: correct the upstream filename in the instrumentation-runtime.ts comment from instrumentation-globals.external.ts to instrumentation-node-extensions.ts (where afterRegistration() is actually defined). * refactor(cache-runtime): drop dead isUseCacheFunction export Nothing imported `isUseCacheFunction` — `otel-tracer-extension.ts` already keeps its own inline copy (`isUseCacheFn`) with a comment explaining why (avoids pulling heavy cache-runtime deps into the lightweight server helper). Remove the unused export to eliminate the dead code flagged in review. |
||
|
James Anderson
|
735d9dad77 |
fix(build): define process.env.NEXT_RUNTIME for server bundles (#1880)
* fix(build): define process.env.NEXT_RUNTIME for server bundles (#1365)
Next.js stamps each route bundle with a compile-time
`process.env.NEXT_RUNTIME` constant ('nodejs' or 'edge' depending on
the route's runtime setting). Vinext was not defining this constant,
so `process.env.NEXT_RUNTIME` resolved to `undefined` at runtime.
User-land code that reads `process.env.NEXT_RUNTIME` (including the
Next.js deploy test fixture `app/path-prefix.js`) would then produce
`'/undefined'` instead of `'/nodejs'`, causing `after()` callbacks
that call `revalidatePath()` to invalidate the wrong cache key. The
ISR page was never revalidated, and the timestamp-change assertion in
`next-after-app-deploy/index.test.ts` always failed.
Fix: inject `process.env.NEXT_RUNTIME = 'nodejs'` into all server
environments (rsc, ssr, custom worker envs) via the existing
`configEnvironment` hook. Client bundles are excluded by the existing
`if (name === "client") return null` guard. Vinext compiles a single
RSC bundle covering all routes, so 'nodejs' is the correct and only
applicable value — it matches both the Workers Node.js compatibility
layer and the `vinext start` Node.js HTTP server.
Fixes #1365
* Fix correct but breaks two tests
Co-authored-by: james-elicx <james-elicx@users.noreply.github.com>
* fix(build): reconcile NEXT_RUNTIME tests and add client '' parity (#1365)
Addresses ask-bonk blocking review findings on #1880:
1. Restore `process.env.NEXT_RUNTIME = 'nodejs'` for server environments
(removed by bot commit
|
||
|
James Anderson
|
de33c3e5a0 |
fix(build): inline ../-relative font assets in OG routes (#1866)
* fix(build): inline ../-relative font assets in OG routes
The vinext:og-inline-fetch-assets plugin only matched paths starting
with "./" (e.g. "./font.ttf"), but Next.js test fixtures for OG custom
fonts use "../"-relative paths like "../../../assets/typewr__.ttf".
Without the inline, at runtime:
- Cloudflare Workers: import.meta.url is "worker" (not a URL), so
new URL("../../../assets/...", import.meta.url) throws TypeError.
- Node.js: fetch() does not support file:// URLs, so the inlined
file:// URL produced by the import-meta-url plugin fails.
Fix: relax the regex from `\.\/[^"']+` to `\.[^"']+` so any
dot-relative path (./x, ../x, ../../x, etc.) is inlined as base64.
Apply the same fix to the readFileSync(fileURLToPath(...)) pattern.
Failing tests addressed:
- test/e2e/og-routes-custom-font: should render og with custom font
for app routes (edge runtime fetch + Node.js fs.promises.readFile)
- test/e2e/app-dir/metadata-font: should handle custom fonts in both
edge and nodejs runtime
* fix(build): inline OG font assets when a formatter adds a trailing comma
The vinext:og-inline-fetch-assets regex only matched the single-line,
comma-less form of `fetch(new URL(...)).then((res) => res.arrayBuffer())`.
When a formatter (Prettier `trailingComma: "all"`, oxfmt) wraps the call
across lines it appends a trailing comma:
.then((res) =>
res.arrayBuffer(),
)
which no longer matched, so the font was left as a runtime fetch. On
Cloudflare Workers `import.meta.url` is "worker", so
`new URL("../...", "worker")` throws "TypeError: Invalid URL" and the OG
route returns 500 — even though the ../-relative path itself was already
supported. Relax both `.then(...)` alternatives with an optional `,?`.
Add real e2e coverage (`/api/og-custom-font`) mirroring Next.js'
og-routes-custom-font fixture: an edge OG route that loads
assets/noto-sans.ttf via
`fetch(new URL("../../../assets/noto-sans.ttf", import.meta.url))`. It
runs in tests/e2e/og-image.spec.ts across the app-router (Node dev),
cloudflare-dev and cloudflare-workers (workerd) projects, with the route
and font added to both the app-basic fixture and the app-router-cloudflare
example. Also adds a unit test for the formatted/trailing-comma shape in
tests/og-inline.test.ts.
* fix(build): inline OG font assets with a semicolon-terminated block body
Follow-up from the /bigbonk review on PR #1866: the block-body alternative
of the og-inline-fetch-assets regex ended `…\.arrayBuffer\(\)\s*\}?\s*,?\s*\)`
with no allowance for a `;` before `}`, so formatter output such as
.then((res) => {
return res.arrayBuffer();
})
.then(function (res) { return res.arrayBuffer(); })
was left as a runtime fetch — which throws "Invalid URL" on Workers
(import.meta.url === "worker"). Add `;?` before the block-body `}` and unit
tests for the arrow- and function-expression block-body forms.
|
||
|
James Anderson
|
d6b9d9a1d1 |
fix(app-router): return HTTP 200 when notFound() is thrown from generateMetadata (#1864)
* fix(app-router): return HTTP 200 when notFound() is thrown from generateMetadata When generateMetadata() calls notFound(), Next.js treats the error as a suspended metadata result — the not-found UI is rendered client-side through the React NotFoundBoundary while the HTTP response stays 200. Previously, vinext was returning the boundary response with the original 404 status from renderAppPageHttpAccessFallback. The fix overrides the rendered fallback response status from 404 to 200 when specialError.fromMetadata is true for http-access-fallback errors, mirroring the existing redirect-from-metadata path which also returns 200. Fixes the failing e2e test: test/e2e/app-dir/metadata-navigation: "should support notFound in generateMetadata" * fix(app-router): gate metadata notFound 200 override on serveStreamingMetadata !== false Bonk finding: the 200 status override for notFound()/forbidden()/unauthorized() thrown from generateMetadata() was applied unconditionally, diverging from Next.js for html-limited bots (serveStreamingMetadata === false). In that path metadata blocks render synchronously and Next.js sets the real HTTP error status (app-render.tsx ~2845). Gate both branches of the http-access-fallback path (with fallback page and without) on `options.serveStreamingMetadata !== false`, symmetric with the existing redirect-from-metadata path which already carries the same gate. Adds unit tests: - fromMetadata:true + serveStreamingMetadata:false + fallback → 404 - fromMetadata:true + serveStreamingMetadata:false + no fallback → 404 |
||
|
James Anderson
|
30be2b330b |
fix(app-router): discover parallel-slot pages inside route groups for catch-all + parallel-routes-group (#1865)
When a parallel slot's page lives inside a route-group subdirectory of the slot root (e.g. `[...catchAll]/@slot/(group)/page.tsx`), `discoverParallelSlots` was looking only for a direct `page.tsx` at the slot root and skipping the slot entirely when none was found. This caused the slot to be absent from the route graph, so the route rendered without the expected slot content. Two related fixes: 1. Add `findSlotRootPage` — a helper that searches for a page file at the slot root URL level, descending through transparent route-group subdirectories (directories whose name matches `(…)`). `discoverParallelSlots` now uses this instead of a bare `findFile` for the slot's page path. 2. In `discoverSlotSubRoutes`, skip the `applySlotSubPages` call when `urlParts.length === 0` (all sub-segments are URL-invisible). A zero-length visible sub-path means the slot page is at the same URL level as the parent route; `discoverParallelSlots` already assigned it with the correct empty `routeSegments`. Calling `applySlotSubPages` in this case would overwrite `routeSegments` with the raw filesystem segments (e.g. `["(group)"]`), making `useSelectedLayoutSegment` surface the route-group name rather than `null`. Fixes the e2e test: `catchall-parallel-routes-group > should work without throwing any errors about invalid pages`. |
||
|
James Anderson
|
ad49b3d8c0 |
fix(use-cache): pass soft tags to cache lookup so revalidatePath invalidates use-cache entries in route handlers (#1867)
When revalidatePath('/node') is called, it records the implicit tag
_N_T_/node as revalidated. Route handlers set path-derived soft tags
(via setCurrentFetchSoftTags in app-rsc-handler.ts) before dispatching
each request, but the "use cache" runtime was calling handler.get without
forwarding those soft tags. As a result, "use cache" data-cache entries
shared across route-handler requests were never checked for soft-tag
invalidation, so revalidatePath had no effect on prerendered route
handlers that use "use cache" internally.
The fix adds getCurrentFetchSoftTags() to fetch-cache.ts and threads the
soft tags into the handler.get call in registerCachedFunction (cache-runtime.ts).
The MemoryCacheHandler already checks _ctx.softTags against tagRevalidatedAt
(the same mechanism used by the patched fetch()), so this is a minimal
one-line change at the call site plus the new getter export.
Fixes: test/e2e/app-dir/use-cache-route-handler-only > should be able to
revalidate prerendered route handlers
|
||
|
github-actions[bot]
|
e7d8a159f4 |
chore: version packages (#1823)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>vinext@0.1.1 |
||
|
Divanshu Chauhan (divkix)
|
bf5b09c46f |
refactor(pages-router): unify triplicated request pipeline into runPagesRequest (#1782) (#1860)
* feat(pages-router): add runPagesRequest pipeline owner with focused tests
Introduces server/pages-request-pipeline.ts which owns the canonical
Next.js 9-step request ordering once. Returns a PagesPipelineResult
discriminated union: type:response for prod/worker callers that supply
render/api callbacks, type:render|api|next intents for dev callers that
omit them. Mirrors createAppRscHandler's thin-closure injection pattern.
Two latent drift bugs reconciled vs the worker copy:
- middlewareStatus now uses result.status ?? result.rewriteStatus
- applyMiddlewareRequestHeaders now passes preserveCredentialHeaders
Adds 28 focused behavior tests covering all pipeline branches.
Adds ./server/pages-request-pipeline export to package.json.
* refactor(pages-router): migrate prod-server.ts to delegate to runPagesRequest
Replaces the ~400-line inlined try block in startPagesRouterServer with a
thin adapter that calls runPagesRequest(webRequest, deps). The adapter
retains only Node-specific concerns: open-redirect guard, decode/400,
prerender endpoint, static assets, image opt, basePath strip, _next/data
normalization, and compression/streaming output.
Extends renderPage callback signature with optional stagedHeaders param
so CSP nonces and other pre-render header injection continue to work.
Copies __vinextStreamedHtmlResponse marker through mergeHeaders to
preserve stream-vs-buffer decision in the adapter.
276 pages-router tests, 112 routing tests, 28 pipeline tests all pass.
* refactor(pages-router): migrate deploy.ts worker template to delegate to runPagesRequest
Collapses the ~280-line inlined fetch handler into ~65 lines. Worker adapter
keeps only: registerConfiguredCacheAdapters, open-redirect guard, static-asset
404, x-nextjs-data header capture, filterInternalHeaders+cloneRequestWithHeaders,
basePath strip with hadBasePath, and image optimization. Delegates the rest to
runPagesRequest via PagesPipelineDeps.
Removes inlined imports: matchRedirect, matchRewrite, preserveRedirectDestinationQuery,
requestContextFromRequest, applyMiddlewareRequestHeaders, isExternalUrl, proxyExternalRequest,
sanitizeDestination, applyConfigHeadersToHeaderRecord, normalizeTrailingSlash, mergeHeaders,
normalizeDefaultLocalePathname, stripI18nLocaleForApiRoute, mergeRewriteQuery.
Updates deploy.test.ts assertions to verify delegation rather than inlined
step logic. 245 deploy tests pass.
* refactor(pages-router): migrate index.ts dev to runPagesRequest, delete parallel helpers
Replaces the inlined dev Pages Router pipeline with runPagesRequest. The
adapter keeps Node-specific concerns: Vite skips, cross-origin guard, image
302 redirect, .html normalization, open-redirect guard, decode/400, basePath
strip with hadBasePath, trailing-slash, _next/data normalization, rawHeaders
snapshot, cloudflare-plugin delegation.
Creates devRunMiddlewareAdapter closure that wraps the dev runMiddleware
(runner, path, i18n, basePath, trailingSlash, isDataRequest args) and
returns a MiddlewareResult-compatible object. Sets VINEXT_MW_CTX_HEADER for
hybrid app+pages mode as a side effect before returning.
Adds writeWebResponseToNodeRes() helper to stream Web Response to Node res
preserving multi-value Set-Cookie. Adds requestHeaders field to render/api
intents so dev adapter can flush post-middleware request headers to req.headers
before calling createSSRHandler/handleApiRoute.
Deletes:
- applyRedirects (lines ~4829-4869)
- proxyExternalRewriteNode (lines ~4867-4928)
- applyRewrites (lines ~4929-4951)
- applyHeaders (lines ~4951-5015)
276 pages-router tests, 112 routing tests, 245 deploy tests, 28 pipeline
tests all pass. vp check clean.
* refactor(pages-router): format deploy.test.ts string literals consistently
* fix(pages-router): restore hasAppDir dev gate; fix external proxy body/headers; update stale after-deploy assertions
Three regressions introduced by the dev pipeline refactor:
1. Hybrid app+pages dev mode: the 'render' intent branch in handlePagesMiddleware
unconditionally called createSSRHandler, missing the original hasAppDir fallthrough
gate. App Router page requests in the hybrid app-basic fixture returned 404 instead
of being deferred to the RSC plugin. Restored by matching the route before calling
the Pages SSR handler and returning next() when there is no pages match and hasAppDir
is true.
2. External proxy body/headers: proxyExternalRewriteNode read the body from the Node
IncomingMessage and the post-middleware headers from req. The refactored pipeline used
a body-less webRequest, so POST bodies were empty and middleware-modified request
headers (x-hello-from-middleware1 etc.) were not forwarded. Fixed by adding an
optional proxyExternal dep to PagesPipelineDeps; the dev adapter supplies it, building
a proper Request from the pipeline's current (post-middleware) headers and the Node
req body stream. Prod and worker adapters fall through to proxyExternalRequest as before.
3. Stale after-deploy assertions: two tests checked old inline call-site strings that
no longer appear in the generated worker entry after the delegation refactor. ctx is
still forwarded (deploy.ts:654,657); updated assertions to match the new call sites.
* fix(pages-router): restore public-file serving + API content-type; gate dev api flushes
Three behavioral regressions from the pipeline extraction, plus the bonk
review findings:
- prod-server (Node): restore post-middleware public-directory static file
serving (original step 5b). The adapter now supplies a serveStaticFile dep
that the pipeline calls after middleware (so middleware can intercept) and
before rewrites. Without it, public/ files (favicon.ico, robots.txt, etc.)
fell through to the renderer and 404'd. Adds a {type:"handled"} result for
callbacks that write their own output.
- prod-server (Node): restore the application/octet-stream content-type default
for API responses. The unified response path collapsed API and page fallbacks
to text/html; the pipeline now tags API results with isApiResponse so the Node
adapter picks octet-stream (arbitrary data) over text/html (content-sniffing
hazard). [bonk finding 1]
- index.ts (dev): gate the staged-header/req-header flushes behind a pages-api
match, mirroring the original 'if (apiMatch)' guard. The unconditional
flushRequestHeaders() wiped req.headers (incl. VINEXT_MW_CTX_HEADER) on the
api-miss -> next() path, dropping middleware context for app/api/* routes in
hybrid app+pages dev mode.
- pipeline: note {type:"next"} is reserved/unused [bonk finding 2]; document
the intentional external-proxy merge asymmetry on the bare proxyExternal
returns [bonk finding 3].
Adds 5 pipeline tests (serveStaticFile/handled ordering, isApiResponse tag).
* fix(pages-router): forward staged headers to worker renderPage; gate data-request defer
Two divergences found in code review of the extraction:
- deploy.ts (worker): the renderPage adapter dropped the pipeline's 4th
stagedHeaders arg, passing undefined for the SSR renderer's middlewareHeaders
param — so middleware-set CSP nonces were never applied to rendered HTML on
the worker, unlike the prod path. Widen the wrapper to forward stagedHeaders.
- pipeline: shouldDeferErrorPageOnMiss only gated on isDataReq, which the worker
never sets (it doesn't normalize /_next/data paths). The pre-refactor worker
gated on the x-nextjs-data header (isDataRequest), so a data-request miss now
wrongly deferred + ran fallback rewrites + re-rendered. Gate on both signals;
no-op for Node/dev where a data request already has isDataReq=true.
Updates the after-deploy assertion to the new call site and adds a pipeline
test for the data-request defer gate.
* fix(pages-router): address code-review findings (content-type, redirect query, dedup)
- prod content-type: passthrough responses (middleware short-circuits, external
proxies) no longer get a text/html default injected. Generalize the response
tag to defaultContentType (render -> text/html, api -> octet-stream, passthrough
-> unset); the Node adapter sends untagged responses verbatim via sendWebResponse,
matching pre-refactor behavior.
- redirect query: config-redirect Location now uses the raw req.url query (via a
new rawSearch dep supplied by the Node prod + dev adapters) instead of
new URL().search, which re-encoded chars and truncated at a literal '#'. The
worker keeps url.search (it only ever had a Web Request). Restores the original
prod/dev behavior.
- efficiency: matchPageRoute no longer runs twice per request on the common
no-afterFiles-rewrite path; reuse the Step 12 match unless afterFiles changed
the pathname.
- dedup: collapse prod-server's mergeWebResponse into the canonical mergeHeaders
(worker-utils) via delegation, removing the duplicate implementation and the
now-orphaned hasHeader/stripHeaders helpers. Existing mergeWebResponse tests
validate the delegation.
Updates pipeline tests for defaultContentType and adds a passthrough-verbatim
assertion. 36 pipeline + 309 features + 276 pages-router + 245 deploy + 9
after-deploy + 112 routing pass; vp check clean.
---------
Co-authored-by: James <james@eli.cx>
|
||
|
Nathan Nguyen
|
3a1e56748f |
fix(app-router): match Pages navigation params in hybrid builds (#1741)
* fix(app-router): emit pages client entry for hybrid builds * fix pages router navigation params readiness * fix(pages-router): track hashed fallback client entry * fix(pages-router): honor navigation readiness in hooks * fix(pages-router): derive server router.isReady from SSR readiness Address review feedback on #1741: - getRouterSnapshot() now reads navigationIsReady from the SSR context on the server instead of unconditionally returning true. For pre-ready Pages routes (auto-export dynamic / query-string / rewrite-capable builds) this keeps useRouter().isReady consistent between SSR and client hydration, avoiding a hydration mismatch for components that read it in JSX. Mirrors Next.js render.tsx's server readiness rule. - Drop the redundant guard branch in getClientParamsSnapshot. - Add a regression test asserting server useRouter().isReady reflects the SSR navigationIsReady context. * fix(pages-router): align param resolution and dev/prod readiness parity Address review feedback (ask-bonk + self-review) on #1741: - Align client Pages navigation params to query-first, matching the server snapshot and Next.js's adaptForPathParams (derives path params from router.query). __NEXT_DATA__.query is kept current on client navigation, so the two snapshots are now provably identical and avoid a latent post- hydration mismatch. - Populate window.__VINEXT_PAGE_PATTERNS__ in the dev server (own script tag) so the next/navigation compat hooks can resolve a dynamic pattern from a resolved path in dev exactly as in production. Closes a dev/prod parity gap (prod set this via the client entry; dev never did). - Drop the redundant pre-readiness setSSRContext in dev; the single post- module-load call now publishes navigationIsReady before rendering, matching the prod handler. - Document that installPagesClientAssetGlobals is intentionally inert for App-only builds, and that getReadonlyPagesSearchParams' string-keyed cache is safe to share across concurrent SSR requests (immutable wrappers). - Add a dev regression test asserting __VINEXT_PAGE_PATTERNS__ exposure. * fix(pages-router): prefer URL params for client snapshots * fix(app-router): gate pages asset globals * test(app-router): add production test for hybrid basePath + assetPrefix * fix(navigation): widen usePathname return type to string | null * refactor(pages-router): dedupe navigation readiness modeling and cache pattern scan Addresses code-review cleanup findings on the Pages Router navigation readiness work: - Extract buildPagesReadinessNextData() so the dev SSR handler and the production Pages page handler derive the gssp/gsp/gip/appGip/autoExport __NEXT_DATA__ readiness flags from one place, instead of two hand-kept copies that could drift and produce a dev/prod hydration mismatch. - Extract resolvePagesNavigationPathname() shared by the client and SSR branches of getPagesNavigationContext, so the pathname null-ness rule (fallback / pre-ready auto-export dynamic) cannot diverge between them. - Memoize resolvePagesRoutePatternForPath()'s pattern scan, which ran an uncached O(routes) loop on every useSyncExternalStore snapshot read. - In getServerSearchParamsSnapshot, serialize the source only when the object-identity fast path misses, avoiding a toString() on every read. * refactor(pages-router): consistent ISR readiness data + robustness cleanups Addresses further code-review findings: - Thread the readiness flags (gssp/gsp/gip/appGip/autoExport) through the ISR background-regeneration path: renderPagesIsrHtml now serializes the same __NEXT_DATA__ readiness state the initial render emits, so a regenerated, re-cached page hydrates with the identical initial router.isReady the server computed. Previously these flags were dropped on that path — benign today (ISR-eligible getStaticProps routes never trigger the flag-gated early-ready branches) but a latent dev/prod hydration-readiness drift. Adds a shared PagesNextDataExtras type so every render path stays in lockstep. - readSsrManifest: degrade a malformed/unparseable SSR manifest to "no SSR manifest" with a warning instead of throwing and aborting server startup, restoring the previous tolerant behavior. - pages-readiness: project PagesReadinessNextData from the canonical VinextNextData and accept PagesPageModule, instead of bespoke types that could drift from the __NEXT_DATA__ shape. - getReadonlyPagesSearchParams: drop the redundant string-keyed cache slot; the per-object WeakMap is authoritative and concurrency-safe on its own. - isPagesRouterReady: drop the redundant server-side window check. * fix(pages-router): restore string-keyed readonly search-params cache Reverts the cache simplification from the previous commit. The string-keyed slot is load-bearing, not redundant: across the Pages Router pre-ready → ready transition the navigation context swaps in a new URLSearchParams instance even when the query string is unchanged, so the per-object WeakMap alone returns a fresh ReadonlyURLSearchParams wrapper and useSearchParams() loses Object.is stability — re-firing a [searchParams] effect. Caught by the rewrite-driven pre-ready static page e2e (search-params-snapshots saw ['',''] not ['']). Keeps the improved comment explaining why both cache levels are needed. --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
9596b72495 |
fix(pages-router): pass revalidateReason "on-demand" to gsp/gssp (#1462) (#1856)
* fix(pages-router): pass revalidateReason "on-demand" to getStaticProps (#1462) Implements res.revalidate() for Pages Router API routes and wires the on-demand revalidation trigger through to getStaticProps/getServerSideProps context as revalidateReason: "on-demand". res.revalidate(urlPath) issues an internal HEAD request to the target path carrying the x-prerender-revalidate header (mirroring Next.js's api-resolver). The Pages render path (dev-server and prod pages-page-data) detects that header, bypasses the fresh/stale cache-hit short-circuits, and regenerates the entry synchronously with reason "on-demand". The build and stale reasons already worked; this finishes the on-demand case. Closes #1462 * fix(pages-router): authenticate on-demand revalidation with a secret (#1462) The on-demand `revalidateReason` detection treated `x-prerender-revalidate` as an unauthenticated presence check and sent the literal value "1". Any external client could send `x-prerender-revalidate: 1` to force synchronous regeneration of any ISR page, bypassing the fresh/stale cache short-circuits — a cache-stampede/DoS vector. Mirror Next.js's `checkIsOnDemandRevalidate`: a request is only treated as on-demand revalidation when the `x-prerender-revalidate` header value EQUALS a secret (the vinext analog of `previewModeId`). Introduce a per-process revalidate secret (`getRevalidateSecret`, 256-bit random hex via Web Crypto) that `res.revalidate()` attaches to its internal loopback request, and a constant-time `isOnDemandRevalidateRequest` equality check on the receiving dev-server and Pages page handler. Header presence alone is never honored. Also fix the test's HTML-reason extraction to avoid CodeQL's incomplete-sanitization / bad-HTML-regex findings, and add a negative test asserting forged `x-prerender-revalidate` values ("1", "", a guess) are rejected while the real secret is accepted. * refactor(pages-router): share res.revalidate() impl; support unstable_onlyGenerated (#1462) Address non-blocking bonk review findings: - Extract the two near-identical `res.revalidate()` implementations (`api-handler.ts`, `pages-node-compat.ts`) into a shared `performOnDemandRevalidate` helper so the secret wiring and success detection cannot drift between dev and prod. - Align success detection with Next.js's api-resolver: accept `x-nextjs-cache: REVALIDATED`, status 200, or a 404 when the caller passed `unstable_onlyGenerated`. Add `unstable_onlyGenerated` support (`x-prerender-revalidate-if-generated` header). * docs(pages-router): note x-nextjs-cache REVALIDATED branch is parity-only (#1462) * fix(pages-router): make on-demand revalidate secret build-time (shared across isolates) The on-demand ISR revalidation secret was a per-process random value. On Cloudflare Workers, res.revalidate()'s loopback fetch() can land on a different isolate than the sender, so a per-process secret mismatches across isolates and false-rejects legitimate revalidations. Mirror Next.js's previewModeId: generate one 256-bit secret at build time (in the vinext build CLI, once per build, shared across plugin instances like __VINEXT_SHARED_BUILD_ID) and bake it SERVER-ONLY into every server bundle via a Vite define (__VINEXT_REVALIDATE_SECRET). It is byte-for-byte identical in every isolate and never reaches the client bundle. getRevalidateSecret() reads the baked constant at runtime and falls back to a process-shared random secret in dev (single-process, no regression). Also persist revalidateSecret in vinext-server.json for diagnostics/tooling parity with prerenderSecret. Constant-time safeEqual validation and reject-arrays/empty/absent behavior are unchanged. Refs #1462 * test(pages-router): cover server-only revalidate secret; drop dead manifest reader Address ask-bonk review of the build-time revalidate secret: - Add tests pinning the security-critical invariant that the __VINEXT_REVALIDATE_SECRET define is baked into server environments (rsc/ssr) but NEVER into the client bundle (the client env returns null outright). A leak would ship the secret to every browser and re-open the cache-stampede vector the equality check prevents. Also harden the existing "no-ops when defineServer unset" test to explicitly clear the env var instead of relying on it happening to be unset. - Drop the unused readRevalidateSecret export and the orphaned revalidateSecret manifest field — the runtime authenticates against the baked define (both Workers isolates and the Node prod-server run the bundle), so the on-disk copy had no consumer (knip-flagged dead code). - Hoist the client early-return in vinext:compiler-define-server so the server-only guarantee is structural (removes the redundant name check), and reword the dev-fallback comments ("absent unless set during vinext build"). Refs #1462 |
||
|
James Anderson
|
062a0b9489 | chore(deploy-suite): bump playwright/playwright-chromium to 1.60.0 after install (#1862) | ||
|
James Anderson
|
c669930f56 |
fix(build): correct CSS ordering for global-not-found (#1549) (#1858)
* fix(build): correct CSS ordering for global-not-found (#1549) * test(build): assert global-not-found CSS ordering in production (#1549) Adds a production-build regression test for #1549: builds the global-not-found-css-order fixture, serves it via the Vite preview server, and asserts the route-miss 404 document serves global-not-found's own CSS (red wins) without leaking the root layout's stylesheet (green), while matched routes still serve the layout's CSS. Verified the test fails without the framework-chunk split in client-build-config.ts (the 404 leaks the layout's green) and passes with it. * chore: update lockfile for global-not-found-css-order test fixture (#1549) * refactor(build): migrate RSC framework chunk to codeSplitting; add unit tests Address bonk review on #1858 (issue #1549): - Replace the deprecated rolldown `advancedChunks` output config with the non-deprecated `codeSplitting` form for Vite 8+, mirroring the client build. Stops the per-prod-build deprecation warning. - Derive both RSC_FRAMEWORK_CHUNK_TEST (regex) and isRscFrameworkModule (predicate) from a single FRAMEWORK_PACKAGES list so they can't drift. - Add focused unit tests for createRscFrameworkChunkOutputConfig (both the Vite 7 manualChunks and Vite 8+ codeSplitting branches) and for the framework package matching (matches react/react-dom/scheduler/ react-server-dom-webpack, excludes react-icons / @react-aria/*). |
||
|
James Anderson
|
dbd6c6ea9c |
feat(release): reclassify commits via SHA-named changeset overrides (#1859)
* feat(release): reclassify commits via SHA-named changeset overrides
Auto-changesets are regenerated from commit subjects every push and never
committed to main, so a mislabeled commit can't be fixed by editing them, and a
hand-authored changeset can only raise a bump (max), never lower it.
Add a per-commit override: commit `.changeset/<sha>.md` and the release tooling
treats that commit as the bump declared in its frontmatter (patch->fix,
minor->feat, major->feat!, empty->chore/dropped), driving both the semver bump
and the changelog section. The file is a real changeset, so changesets/action
consumes it for the bump and deletes it on release -- overrides never accumulate.
Both create-changeset.mts (bump) and version.mts (changelog grouping) honor it,
keyed off the SHA in the filename.
* chore: reclassify #1804 as a fix
Override `feat(interception): sibling-style interception routes (#1804)`
(
|
||
|
James Anderson
|
29a040dd21 |
fix(app-router): include route pattern in repeated-slash Link warning (#1554) (#1855)
* fix(app-router): include route pattern in repeated-slash Link warning (#1554) The repeated forward-slashes Link warning added in #1563 fired with the wrong "in page" segment: it used `window.location.pathname` (or `"/"` during SSR) instead of Next.js's route pattern (`router.pathname`, e.g. `/my/path/[name]`). The Next.js compat test `repeated-forward-slashes-error` asserts the *full* message including that route pattern, so it kept failing in the deploy suite despite #1563 — the `/Invalid href/` regex matched but the `toContain` of the exact message did not. Source the page segment from the current render's route pattern via a new `getCurrentRoutePathnameForWarning()` helper: the Pages Router SSR context's route pattern on the server, `window.location.pathname` on the client, falling back to `"/"`. Normal Link behaviour is unchanged. Add an SSR-level compat test (app + pages router) that renders a Link with a repeated-slash href and asserts the exact warning, plus the upstream dynamic-route fixture `pages/my/path/[name]`. Finishes #1554 (the warning wasn't firing with the right page name despite #1563). Closes #1554 * fix(app-router): read route pattern via side-effect-free accessor The previous commit imported `router.ts` into `link.tsx` to source the route pattern for the repeated-slash warning. But `router.ts` has a browser-only top-level side effect (`installWindowNext({ router })`), so pulling it into the Link shim — which is in the App Router client bundle too — clobbered `window.next.router` with the Pages Router instance, breaking App Router client navigation (useParams/prefetch/interception E2E timeouts). This is exactly why `navigation.ts` deliberately avoids importing `router.ts` and reads the Pages nav context through a `Symbol.for` handle. Mirror that pattern: add a side-effect-free `route-pattern-for-warning` module that reads the SSR route pattern through a `Symbol.for` accessor published by the server-only `router-state.ts`. The Link shim imports only this tiny module, so no Pages Router side effect leaks into the App Router bundle. Behaviour is unchanged: pages-router SSR reports `in page: '/my/path/[name]'`, client/App Router fall back to `window.location.pathname` then `"/"`. |
||
|
James Anderson
|
dd3fb05977 |
test(pages-router): cover javascript: URL blocking (#1576) (#1854)
* fix(pages-router): emit console.error for javascript: URL navigation (#1576) Finishes the Pages Router half of #1576 (the App Router warnings already land). Adds the four Pages Router scenarios from the upstream test/e2e/app-dir/javascript-urls suite — Link `href`, Link `as`, router.push, and router.replace — as e2e fixtures + a Playwright spec, plus unit coverage for the shared Link shim's dangerous-click handler. No production change is required: the Link shim already emits the canonical "Next.js has blocked a javascript: URL as a security precaution." console.error on click for both `href` and `as`, and the Pages Router router.push/replace synchronous guard from #1575 already does the same. This PR adds the missing Pages Router coverage that proves all four scenarios. Closes #1576 * test(pages-router): suppress dev error overlay before safe navigation The router.push/replace javascript: scenarios throw synchronously (the #1575 behaviour that surfaces the console.error), which trips vinext's dev error overlay. Its backdrop intercepted the follow-up safe-link click, timing out the test even though the security block already passed. Suppress the overlay via the existing disableDevErrorOverlay helper after asserting the block, matching how other pages-router/app-router specs handle intentional dev-time errors. * test(pages-router): cover javascript: URL blocking in production build The Pages Router javascript: URL guard (Link dangerous-click handler in shims/link.tsx + router.push/replace synchronous guard in shims/router.ts, both via shims/url-safety.ts) is already correct on main. The existing e2e spec, however, only runs against the dev server (vp dev), while the Next.js deploy suite exercises these scenarios against a production build where the dev error overlay is absent and the client bundle is minified. Add a pages-router-prod Playwright spec that re-runs all four upstream scenarios (Link href, Link as, router.push, router.replace) against vinext build + vinext start, so a prod-only regression is caught here. Refs #1576 |
||
|
James Anderson
|
76a39cb467 |
fix(actions): return 404 for MPA action on app with no server actions (#1340) (#1853)
* fix(actions): return 404 for MPA action on app with no server actions (#1340) A multipart form POST to an App Router page is always a server-action attempt. When the form body decodes to no action at all (e.g. the build has no server actions, so decodeAction returns null rather than throwing), vinext fell through to a 200 page render instead of Next.js' 404 + x-nextjs-action-not-found. The fetch-action variant (via the Next-Action header) already worked; only the MPA/no-JS form-POST variant was wrong. Gate the new 404 on the posted-to route being a page: route handlers run after action dispatch and legitimately accept raw multipart POSTs, so they still fall through. The page-vs-route distinction uses the static __loadPage / __loadRouteHandler markers, available before lazy module hydration. Finishes the no-server-actions MPA case of #1340 (error-boundary, content-type, and unrecognized-action handling landed in #1386/#1668). * test(actions): add MPA no-action integration test; skip redundant route match (#1340) Address ask-bonk review on #1853: - Add a dev-server integration test that POSTs multipart/form-data to a page (/about) and asserts 404 + x-nextjs-action-not-found, locking in the entry-side matchRoute + __loadPage/__loadRouteHandler classification end-to-end (previously only unit-tested by passing hasPageRoute directly). - Gate the progressive-action route match on isProgressiveServerActionRequest so matchRoute no longer runs on every App Router request — only for actual multipart/no-actionId POST candidates. |