fix(pages): apply fallback rewrites after API misses

[james-elicx]

Summary

• apply fallback rewrites only after no filesystem API route claims the request
• share route-claim logic across Pages dev, built Node production, and the generated Pages Worker
• preserve external proxy method, query, headers, and body
• hand internal fallback destinations to App route handlers with post-middleware request overrides and staged response metadata intact

Regression coverage

• a real Pages fixture proxies an unmatched POST through a fallback rewrite to a local upstream in dev and production
• covers basePath, has/missing conditions, local Pages API precedence, merged query, headers, and JSON body
• a real hybrid Pages/App fixture rewrites an unmatched API path to an App catch-all route handler and verifies middleware cookie overrides, response headers, URL/query/body/params, and production parity
• the external fixture returns 404 and the hybrid fixture returns 500 before the fix

Validation

• focused Pages fixture, hybrid dev, and built production tests
• pages request-pipeline, deploy/Worker wiring, and App RSC rewrite tests
• vinext build and touched-file vp check
• independent cumulative parity review: clean

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2827

npm i https://pkg.pr.new/create-vinext-app@2827

npm i https://pkg.pr.new/@vinext/types@2827

npm i https://pkg.pr.new/vinext@2827

commit: 64e3fba

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[github-actions]

Performance benchmarks

Compared 64e3fba against base 3c5cea4 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	134.6 KB	134.7 KB	⚫ +0.0%
Client entry size (gzip)	vinext	122.2 KB	122.2 KB	⚫ +0.0%
Dev server cold start	vinext	2.32 s	2.32 s	⚫ +0.1%
Production build time	vinext	2.41 s	2.40 s	⚫ -0.4%
RSC entry closure size (gzip)	vinext	113.8 KB	113.9 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	191.4 KB	191.4 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): apply fallback rewrites after API misses

Reviewed the full diff against Next.js execution-order semantics, dev/prod parity, and the middleware-context handoff protocol. The core fix is correct and well-tested. One latent correctness concern in the dev handoff guard, plus a couple of smaller notes.

The fix is sound

The central change — gating handleResolvedApiRoute() on deps.matchApiRoute so an /api/* filesystem miss continues through afterFiles/fallback rewrites instead of committing to the API 404 — correctly mirrors Next.js's documented order (API filesystem check runs before afterFiles/fallback rewrites, and only a real match owns the request). pages-request-pipeline.ts:630-633 is the right seam, and threading matchApiRoute through the pipeline deps, the generated pages-server-entry (matchApiRoute), pages-router-entry.ts, and prod-server.ts keeps dev / built-Node / Worker in parity. The generated matchApiRoute only scans Pages apiRoutes — correct for pure-Pages production, since hybrid production resolves App fallback destinations through the App RSC handler's own rewrite logic (confirmed by the parameterized app-rsc-handler.test.ts covering both beforeFiles and fallback).

The app-middleware.ts refactor is behaviorally equivalent for the normal (non-forwarded) path: context.requestHeaders ??= new Headers(context.headers) then applyMiddlewareRequestHeaders(context.requestHeaders) reads the same x-middleware-* protocol keys the old applyMiddlewareRequestHeaders(context.headers) read. The new branch correctly enables the request-header-only (q without h) forwarded case that the old if (context.headers) gate would have skipped.

Test coverage is strong: real dev+prod Pages fixture (basePath, has/missing, local API precedence, merged query, headers, JSON body, external proxy method/body), and a hybrid App fixture whose fallback has condition keys on a cookie set by middleware — which is exactly what proves the rule is evaluated against post-middleware request-header overrides.

Concern: URL-equality guard conflates i18n locale stripping with a config rewrite (dev)

index.ts:5663:

if (pipelineResult.apiUrl !== routeUrl) {
  forwardInternalApiRewriteToApp(pipelineResult.apiUrl);
}

routeUrl is the full pre-strip URL, but pipelineResult.apiUrl is apiLookupUrl = stripI18nLocaleForApiRoute(resolvedUrl, i18nConfig). For an i18n app (the app-basic fixture configures i18n: { locales: ["en","fr"] }), a locale-prefixed API request to an App route handler with no config rewrite — e.g. /fr/api/foo → App /api/foo — yields apiUrl = /api/foo but routeUrl = /fr/api/foo, so apiUrl !== routeUrl is true and forwardInternalApiRewriteToApp("/api/foo") fires spuriously, injecting a VINEXT_MW_CTX_HEADER with r: /api/foo (a phantom rewrite) into the App RSC handoff.

The URL-equality check is being used as a proxy for "did a config rewrite change the request?", but it also trips on i18n locale normalization (and potentially query-order/encoding normalization between routeUrl and resolvedUrl). API routes are rarely locale-prefixed in practice, so real-world impact is low, but this is an accidental divergence worth closing. Consider tracking an explicit "a config rewrite fired" signal from the pipeline (the pipeline already has configRewriteFired) and surfacing it on the api intent, rather than inferring it from URL inequality.

Minor notes

• forwardInternalApiRewriteToApp duplicates the stagedHeaders flatten loop that flushStagedHeaders already implements (index.ts:5574-5582 vs 5591-5598). Not a bug, but the two could share one helper.
• The new matchApiRoute dev closure (index.ts:5459) re-implements the pages→app precedence check that the api-intent branch also does at 5626-5636, but with subtly different semantics: the closure returns any App match (to keep the request flowing), while the branch applies pagesRouteHasPriorityOverAppRoute. That's intentional and correct here, but a short comment noting the two serve different purposes would help future readers.

Everything I ran is green: pages-request-pipeline.test.ts (97), app-rsc-handler.test.ts (169), the new dev + prod hybrid fallback tests, and vp check on all five touched source files.

[ask-bonk]

Review posted to PR #2827.

Summary

I reviewed PR #2827 (fix(pages): apply fallback rewrites after API misses) for correctness, dev/prod parity, and Next.js behavioral fidelity.

The fix is sound. The core change — gating handleResolvedApiRoute() on deps.matchApiRoute so an /api/* filesystem miss continues through afterFiles/fallback rewrites instead of committing to a 404 — correctly matches Next.js's documented execution order (API filesystem check runs before afterFiles/fallback). The matchApiRoute threading keeps dev, built-Node, and Worker paths in parity, and the app-middleware.ts refactor is behaviorally equivalent on the normal path while enabling the new request-header-only forwarded case.

One latent concern I flagged (index.ts:5663): the pipelineResult.apiUrl !== routeUrl guard used to decide whether to hand off to the App RSC plugin conflates i18n locale stripping with an actual config rewrite. In an i18n hybrid app (the app-basic fixture configures en/fr), a locale-prefixed API request to an App route handler with no rewrite would spuriously trigger the handoff with a phantom r: rewrite. Impact is low (API routes rarely locale-prefixed) but it's an accidental divergence; I suggested surfacing the pipeline's existing configRewriteFired signal instead of inferring from URL inequality.

Minor notes: a duplicated stagedHeaders flatten loop, and two matchApiRoute precedence checks with intentionally different semantics that could use a clarifying comment.

All tests I ran are green: pages-request-pipeline (97), app-rsc-handler (169), the new dev+prod hybrid fallback tests, and vp check on all five touched source files.

github run

[ask-bonk]

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: ProviderInitError

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): apply fallback rewrites after API misses

Re-reviewed after the second commit (64e3fba). The core fix is correct and well-tested, and the follow-up commit already resolves the i18n concern raised in the previous review by switching from the apiUrl !== routeUrl guard to pipelineResult.configRewriteFired. I ran the touched test suites and vp check; all green.

The fix is sound

Gating handleResolvedApiRoute() on deps.matchApiRoute (pages-request-pipeline.ts:635) so an /api/* filesystem miss continues through afterFiles/fallback rewrites instead of committing to the API 404 correctly mirrors Next.js's documented order (the API filesystem check runs before afterFiles/fallback rewrites; only a real match owns the request). Threading matchApiRoute through the pipeline deps, the generated pages-server-entry, pages-router-entry.ts, and prod-server.ts keeps dev / built-Node / Worker in parity.

The previous-review concern — that the dev handoff guard conflated i18n locale stripping with a config rewrite — is fixed in 64e3fba. index.ts:5666 now gates forwardInternalApiRewriteToApp on pipelineResult.configRewriteFired, and configRewriteFired is set to true only at actual rewrite sites (pipeline lines 602/702/756/816/902), never by stripI18nLocaleForApiRoute. So a locale-prefixed API request with no config rewrite no longer injects a phantom r: rewrite into the App RSC handoff. The new hybrid-i18n-api-handoff fixture asserts exactly this (/fr/api/direct → 404, /api/direct → 200) in both dev and production, and it's ported with a link to Next.js's i18n-api-support test.

The app-middleware.ts refactor is behaviorally equivalent on the normal path: applyMiddlewareRequestHeaders only reads the x-middleware-* protocol keys from its argument and mutates the global headers context, so passing context.requestHeaders (a copy of context.headers, which still carries those keys) produces the same result as the old applyMiddlewareRequestHeaders(context.headers). The new q branch correctly enables the request-header-only forwarded case (data.q carries the encoded protocol keys via encodeMiddlewareRequestHeaders), which the old if (context.headers) gate would have skipped.

Test coverage is strong: a real dev+prod Pages fixture (basePath, has/missing, local-API precedence, merged query, preserved method/headers/JSON body via an external upstream) and a hybrid App fixture whose fallback has condition keys on a cookie set by middleware — proving the rule is evaluated against post-middleware request-header overrides and the App handoff carries them.

Non-blocking: pre-existing i18n locale-prefixed-API divergence

Not introduced by this PR, but surfaced by the new matchApiRoute gate and worth tracking. Next.js's resolve-routes.ts (checkLocaleApi + the check_fs guard) explicitly skips the filesystem API check when the request carries an explicit locale prefix:

const checkLocaleApi = (pathname) =>
  config.i18n && pathname === urlNoQuery &&
  initialLocaleResult?.detectedLocale &&
  pathHasPrefix(initialLocaleResult.pathname, '/api')

That is exactly why Next.js's i18n-api-support e2e test expects /fr/api/hello (with pages/api/hello present) to fall through to the fallback rewrite (returns Example Domain), not to be served by the pages/api/hello handler. vinext's stripI18nLocaleForApiRoute instead strips the prefix whenever hadPrefix is true, so /fr/api/hello matches pages/api/hello and is served directly. The PR's new fixture only covers the no-rewrite case (/fr/api/direct → 404), which matches Next.js by coincidence (nothing claims it and nothing rewrites it), so the divergence isn't exercised.

This is a pre-existing behavior of stripI18nLocaleForApiRoute (unchanged by this PR), so it's not a blocker here — but consider a follow-up to port the "locale-prefixed API + fallback rewrite" case from Next.js's i18n-api-support test and align stripI18nLocaleForApiRoute with the detectedLocale-skips-filesystem-API semantics.

Minor notes

• index.ts:5461 (dev matchApiRoute closure) and index.ts:5626-5636 (api-intent precedence branch) both do a pages→app precedence check but with intentionally different semantics: the closure returns any App match to keep the request flowing, while the branch applies pagesRouteHasPriorityOverAppRoute. Correct as-is; a one-line comment noting the two serve different purposes would help future readers.
• The generated matchApiRoute (pages-server-entry.ts:368) re-runs resolvePagesI18nRequest on a url the pipeline has already passed as apiLookupUrl (locale-stripped). Harmless (near no-op), just redundant.

Validation

• tests/pages-request-pipeline.test.ts (99), tests/app-rsc-handler.test.ts + tests/app-router-next-config-dev.test.ts (187), tests/hybrid-i18n-api-handoff.test.ts (2, dev+prod), tests/deploy.test.ts (312) — all green
• vp check on all five touched source files — clean

[ask-bonk]

Review posted to PR #2827.

Summary

I reviewed PR #2827 (fix(pages): apply fallback rewrites after API misses), including the second commit 64e3fba that landed after the earlier review round.

The fix is sound. Gating handleResolvedApiRoute() on deps.matchApiRoute so an /api/* filesystem miss continues through afterFiles/fallback rewrites correctly matches Next.js's documented execution order. matchApiRoute is threaded through the pipeline, generated entry, pages-router-entry.ts, and prod-server.ts, keeping dev / built-Node / Worker in parity.

The previous review's i18n concern is already fixed. The 64e3fba commit replaced the apiUrl !== routeUrl guard with pipelineResult.configRewriteFired (index.ts:5666), and configRewriteFired is only set at real rewrite sites — never by stripI18nLocaleForApiRoute. The new hybrid-i18n-api-handoff fixture asserts this in both dev and prod.

The app-middleware.ts refactor is behaviorally equivalent on the normal path and correctly enables the new request-header-only forwarded case (the q branch decodes the encoded protocol keys).

One non-blocking finding I flagged: a pre-existing i18n divergence (not introduced by this PR). Next.js's resolve-routes.ts explicitly skips the filesystem API check for locale-prefixed API paths (checkLocaleApi), so /fr/api/hello falls through to fallback rewrites rather than being served by pages/api/hello. vinext's stripI18nLocaleForApiRoute strips the prefix and serves it directly. The new fixture only covers the no-rewrite case (/fr/api/direct → 404), which matches Next.js coincidentally. Suggested a follow-up to port Next.js's "locale-prefixed API + fallback rewrite" case.

Minor notes: two intentionally-different matchApiRoute precedence checks that could use a clarifying comment, and a redundant resolvePagesI18nRequest re-run in the generated entry.

All tests I ran are green: pages-request-pipeline (99), app-rsc-handler + app-router-next-config-dev (187), hybrid-i18n-api-handoff (2 dev+prod), deploy (312), and vp check on all five touched source files.

github run