mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
vinext@0.2.0
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 James Anderson
|
e00687a0f6 |
fix(middleware): match Pages data request metadata (#2239)
* fix(middleware): preserve Pages data routing metadata * fix(middleware): preserve matched path on data misses * test(pages): align middleware data miss assertions * fix(pages): align dev middleware data misses * fix(pages): gate data misses on real middleware * fix(middleware): address data redirect review * Reviewed PR #2239: fixes confirmed Co-authored-by: james-elicx <james-elicx@users.noreply.github.com> * chore(tests): remove stray node_modules symlinks --------- Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com> Co-authored-by: james-elicx <james-elicx@users.noreply.github.com> |
||
|
James Anderson
|
b58adb3cfa | fix(middleware): classify Pages data requests by URL (#2039) | ||
|
James Anderson
|
68414e68a5 |
fix(router): re-enter filesystem routes after rewrites (#2032)
* fix(router): re-enter static files after i18n rewrites * fix(router): re-enter filesystem routes for all rewrites |
||
|
Nathan Nguyen
|
a3d2f92152 |
fix(router): honor hybrid pages route priority (#1997)
* fix(router): honor hybrid pages route priority * fix(router): share hybrid owner decision with client navigation PR #1997 fixed the server-side route ownership for direct document loads, but the same invariant broke for client-side soft navigations and the matching prefetch path: - navigateClientSide() always delegated to the App runtime's RSC fetch, even when the Pages route had higher priority. renderPagesFallback() short-circuits RSC requests with null, so the App catch-all won. - prefetchUrl() prefetched an RSC stream for any URL that matched an App route, again ignoring Pages ownership. - The Pages entry was loaded on every hybrid request, even when a static App route had already matched (Pages can never win in that case). Expose the Pages route manifest on the client via a new __VINEXT_PAGES_LINK_PREFETCH_ROUTES__ window global emitted by both the App and Pages browser entries. The link shim consults a shared resolveHybridClientRouteOwner helper that mirrors the server-side pagesRouteHasPriorityOverAppRoute comparison. When Pages owns the URL, the click handler issues a window.location navigation and the prefetch path returns early. Gate the renderPagesFallback call behind a static-App-route check in handleAppRscRequest: when a static App route matches, the bridge cannot win, so skip the eager Pages entry load. Centralise the comparison in a new resolveHybridRouteOwner helper so server and client reach the same answer for the same (URL, route pair). Adds the missing client-navigation coverage to the use-params e2e fixture (Link from /app/ to /pages-dir/foobar) and unit tests for the shared owner decision. * fix(router): mirror server hybrid owner decision on the client PR review flagged two split-brain bugs in the previous hybrid ownership fix: a hand-copied client comparator and a Link-only ownership gate that left programmatic App Router navigations on the wrong path. The hand-copied routePrecedence in hybrid-client-route-owner.ts omitted the static-prefix reduction that lives in routing/utils.ts#routePrecedence, and used a strict-less-than comparison that returned App for identical dynamic patterns. The server returns Pages for both cases (Pages providers sort ahead of App providers, and routePrecedence subtracts 50 per static prefix segment). The split produced a real ownership disagreement on overlapping patterns like /_sites/:slug* (Pages) vs /:slug* (App). Add a shared compareHybridRoutePatterns to routing/utils.ts as the single source of truth: the static/dynamic short-circuits plus a sortRoutes call (which carries the static-prefix reduction and the Pages-first equal-pattern tiebreak). The server pagesRouteHasPriorityOverAppRoute and the client resolveHybridClientRouteOwner both delegate to it, so they cannot diverge. Drop the hand-copied routePrecedence entirely. Wire the ownership check at the App navigation runtime boundary so useRouter().push, useRouter().replace, gesturePush, and form submits all get the same hard-nav contract as the Link click handler. Specifically: - navigateClientSide: after same-origin normalization, if Pages owns the URL, hard-navigate via window.location and return (matching the existing external-URL branch). - _appRouter.prefetch: short-circuit RSC URL construction for Pages-owned targets so we do not warm an unusable cache entry. Centralise the existing two inline hard-nav branches in navigateClientSide into a hardNavigateTo helper for clarity. Tests: - Direct unit tests for compareHybridRoutePatterns covering identical-dynamic tiebreak, static-prefix dynamic overlap, static-prefix catch-all overlap, and infix-static bonus. - Direct unit tests for resolveHybridClientRouteOwner mirroring the server assertions plus a basePath-stripping test. - e2e: useRouter().push('/pages-dir/foobar') from an App page resolves to the Pages document. - e2e: useRouter().prefetch('/pages-dir/foobar') issues zero RSC requests for the target URL. * docs(router): fix stale score in static-prefix catch-all test comment The hand-copied comparator note quoted the dynamic-segment scores (51 / 1000) for the optional-catch-all example (/_sites/:slug* vs /:slug*). The current optional-catch-all scoring actually produces 1951 vs 2000. The test assertion was correct; only the comment was stale. * test(use-params): fix direct-load single dynamic param to use /a instead of /a/b * fix(router): compare hybrid routes structurally * fix(router): reject hybrid route conflicts * fix(router): refresh hybrid route ownership * fix(router): preserve hybrid routing lifecycle * fix(router): recheck pages routes after rewrites * fix(router): preserve rewritten pages queries * fix(router): preserve rewritten route ownership * fix(router): resolve client rewrites sequentially * fix(router): apply rewrite phases sequentially * fix(router): preserve rewrite params and endpoints * fix(router): hand off endpoint navigations * fix(router): preserve rewrite fragment params * test(e2e): avoid hybrid fixture route conflicts * test(pages): await async config validation * test(hybrid): avoid duplicate page fixtures * chore(router): clarify hybrid priority semantics * refactor(router): remove duplicate app route matcher * docs(router): clarify client hybrid comparator --------- Co-authored-by: James <james@eli.cx> |
||
|
Nathan Nguyen
|
b7ef466742 |
fix(pages): preserve app props for GSSP requests (#1996)
* fix(pages): preserve app props for gssp requests
* docs(pages): update _next/data comment to reflect full props envelope
* fix(pages-router): preserve _app.getInitialProps during stale ISR regeneration
Stale ISR background regeneration called getStaticProps directly and rendered
HTML and __NEXT_DATA__ with only the page's pageProps, dropping app-level
props from _app.getInitialProps. The foreground path already loads
_app.getInitialProps first and merges its props, but the background
regeneration path skipped that step.
Extract a shared loadPagesAppInitialRenderProps helper from the production
foreground path and reuse it during background regeneration in both
pages-page-data.ts and dev-server.ts. This ensures regenerated pages carry the
same full props envelope as the original render.
Add a regression test in tests/pages-page-data.test.ts.
* fix(pages-router): skip _app.getInitialProps on ISR cache hits
The foreground render path ran _app.getInitialProps before consulting the
ISR cache, so fresh and stale cache hits executed userland App data code
even though the cached HTML was returned verbatim. That wasted work on the
hot path and ran side-effecting App code whose result the cached response
never reflected.
The eager App initial-props load happened up front, ahead of the HIT and
STALE short-circuits. Defer it until the request will actually render:
load it lazily inside the getServerSideProps / getStaticProps /
getInitialProps branches, after the cache short-circuits return.
Stale-while-revalidate already re-runs it inside the regeneration closure,
so regenerated HTML still carries app-level props. The dev SSR handler and
the production page-data resolver are updated in lockstep to preserve
dev/prod parity.
Tests cover the fresh-HIT skip and the stale foreground-skip vs
regeneration-run split at the page-data resolver boundary.
* refactor(pages-router): extract dev App getInitialProps loader into a tested helper
The dev SSR handler ran _app.getInitialProps inside an inline closure that
mutated captured pageProps/renderProps and read res directly, so its lazy
load-only-on-render behavior could only be covered by a heavyweight
integration test that vi.doMock'd the isr-cache and file-matcher modules —
a mock cascade with little signal.
Move the decision into loadDevAppInitialProps in pages-get-initial-props.ts.
It takes the App component, req/res, and an injected AppTree builder (keeping
React out of the module) and returns a discriminated result —
skip / response-sent / render — that the caller applies. The dev-server
closure is now thin glue: build the React tree, delegate, apply the result.
Adds focused unit tests that drive the helper through its real boundary
(genuine getInitialProps functions, plain req/res inputs) — no module mocks:
the skip path, App/page prop merging, pageProps normalization, the
response-sent short-circuit, and the router/ctx wiring passed to userland.
* fix(pages): preserve app props across data requests
* fix(pages): preserve fallback cache expiry
* fix(pages): preserve dev app context during ISR
* fix(pages): hydrate middleware rewrite fallbacks
* fix(pages): retarget middleware data rewrites
* fix(pages): preserve rewrite query on navigation
* fix(pages): finalize rewrite navigation state
* fix(pages): resolve rewrite data targets consistently
* fix(pages): await static props during regeneration
* fix(pages): preserve original query in resolved url
* Revert "fix(pages): await static props during regeneration"
This reverts commit
|
||
|
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. |
||
|
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>
|