fix(router): re-enter filesystem routes after rewrites

[james-elicx]

Summary

• re-enter filesystem/static matching after beforeFiles, afterFiles, and fallback rewrites
• support locale: false rewrites to public files and /_next/static assets
• preserve normal Vite static semantics in dev, including HEAD, Range, ETag/304, MIME, caching, and content length
• keep API/page/static precedence consistent across dev, Node production, and Workers

Next.js parity

Fixes test/e2e/i18n-ignore-rewrite-source-locale/rewrites.test.ts and closes broader filesystem rewrite parity across all rewrite phases.

Validation

• exact three locale/public integration assertions passed
• 349 focused tests passed
• 5 generated Worker assertions passed
• vp check, Cloudflare build, and vinext build passed
• independent review: no actionable findings

Cache Components, PPR, and resume behavior are out of scope.

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 23e1dc2

[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

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: re-enter filesystem routes after rewrites

I reviewed the full diff, traced all three runtime adapters (dev index.ts, Node prod-server.ts, Worker deploy.ts), and ran the relevant suites locally.

Verification:

• tests/pages-i18n-public-rewrite.test.ts — 13/13 pass (dev + prod)
• tests/pages-request-pipeline.test.ts + tests/deploy.test.ts — 336/336 pass
• tests/pages-router.test.ts + tests/routing.test.ts — 405/405 pass (no regressions)
• vp check on all changed source files — clean (format/lint/types)

Assessment: The core design is sound. Hoisting the filesystem/API checkpoint into a serveFilesystemRoute(pathname, phase) helper and re-running it after beforeFiles/afterFiles/fallback rewrites is the right shape, and the three adapters stay consistent:

• Node writes directly to res and returns true → { type: "handled" }
• Dev/Workers return a Response → merged with staged middleware headers
• direct phase is correctly skipped on Workers (assets served by the binding before the worker runs) and on dev (Vite serves directly), while Node keeps direct public-file serving but excludes _next/static only in the direct phase

Page-vs-rewrite precedence is preserved correctly: the afterFiles loop is gated on !pageMatch || pageMatch.route.isDynamic, so a real page (/after-control) wins over an afterFiles rewrite — confirmed by the fixture test.

The dev replay of viteServePublicMiddleware/viteServeStaticMiddleware is careful: it snapshots req.url/status/headers, settles on finish/close/next(err) with a settled guard, removes listeners on settle, and restores state on fall-through. HEAD/Range/ETag/304/MIME parity is exercised by the fixture.

Non-blocking observations

1. Worker re-entry status override (pages-request-pipeline.ts:312): mergeHeaders(served, middlewareHeaders, middlewareStatus) applies middlewareStatus as a status override to the asset response. If middleware staged a status (e.g. 200) and the asset returns 206/304, the override would replace the asset's status. This matches how render/api results already merge middlewareStatus, so it is consistent with existing behavior rather than a new regression — but worth a comment, since a static-asset Range/conditional response is a new caller of this path. Low impact (requires middleware setting a status alongside a rewrite to a static file).

2. Dev gating recomputes per request (index.ts:3739-3765): the Headers/Request/requestContextFromRequest/matchRewrite work runs for every dev request reaching that block, even non-dotted paths, before the if. Dev-only and cheap, but it could be deferred behind the existing pathname.includes(".") check to avoid the allocation on the common path.

3. Hardcoded hadBasePath: true in the dev gating matchRewrite call (index.ts:3763): for a basePath app, a dotted path outside the basePath could spuriously match a rewrite source and enter the pipeline, which then falls through harmlessly. Matches the documented middleware.ts:160 convention, so this is intentional, but a one-line note would help future readers.

None of these block merge. Nice work on the cross-runtime consistency and the thorough fixture coverage.

[ask-bonk]

Review posted to PR #2032.

Summary

I reviewed PR #2032 (fix(router): re-enter filesystem routes after rewrites) by tracing the full diff across all three runtime adapters and running the relevant test suites locally.

Verification (all green):

• tests/pages-i18n-public-rewrite.test.ts — 13/13 (dev + prod)
• tests/pages-request-pipeline.test.ts + tests/deploy.test.ts — 336/336
• tests/pages-router.test.ts + tests/routing.test.ts — 405/405 (no regressions)
• vp check on changed source — clean

Assessment: The design is sound. The serveFilesystemRoute(pathname, phase) helper is correctly re-run after each rewrite phase, and the three adapters stay consistent (Node writes to res → handled; dev/Workers return a Response; direct phase correctly skipped on Workers/dev). Page-vs-rewrite precedence is preserved via the !pageMatch || isDynamic gate, and the dev replay of Vite's static middlewares handles HEAD/Range/304/MIME with proper state snapshot/restore and listener cleanup.

Non-blocking observations I flagged:

1. Worker re-entry applies middlewareStatus as a status override to asset responses (consistent with existing render/api merge behavior, but new for Range/conditional asset responses).
2. The dev gating recomputes Headers/Request/matchRewrite per request before the dotted-path if (dev-only, minor allocation).
3. Hardcoded hadBasePath: true in dev gating — intentional per the middleware.ts:160 convention, but worth a comment.

No blockers found.

github run