fix(app-router): honor basePath false rewrites outside basePath#2117
Open
NathanDrake2406 wants to merge 8 commits into
Open
fix(app-router): honor basePath false rewrites outside basePath#2117NathanDrake2406 wants to merge 8 commits into
NathanDrake2406 wants to merge 8 commits into
Conversation
commit: |
App Router requests outside a configured basePath were rejected during request normalization before next.config rewrites could match. This made same-origin server-action redirects to an absolute URL outside basePath fall through to 404 even when a basePath:false external rewrite should proxy it. The broken invariant was that basePath membership has to remain visible through rewrite matching, not be collapsed into an early 404. Normalization now records whether the original request had the basePath and lets the App Router delay rejection; filesystem routes stay gated until a config or middleware rewrite makes the request routable. Adds focused handler and normalization coverage plus an E2E port of the upstream app-basepath server action redirect cases.
Out-of-basePath App Router requests with no matching rewrite fell through to renderNotFound(), rendering the app's not-found page. That page is a filesystem route, so the render loaded the app's client references. Under startProdServer's process-wide RSC globals, a second build (the basePath prod-server test) thereby corrupted the active client-reference manifest, breaking later requests on the shared server with "client reference not found" — 13 failures in tests/app-router-production-server.test.ts. normalizeRscRequest previously 404'd these requests before they reached the handler, so the not-found render was never exercised. The PR's allowOutOfBasePath path removed that early exit but left renderNotFound ungated, unlike the metadata, public-file, and page handlers. Gate renderNotFound behind canUseFilesystemRoutes() and fall through to a plain 404, matching the other filesystem handlers and Next.js behavior for paths outside basePath with no basePath:false rewrite.
NathanDrake2406
force-pushed
the
nathan/app-basepath-parity
branch
from
b37a735 to
4ae6f90
Compare
…em eligibility Out-of-basePath POST requests with an action id could still execute server actions and progressive form actions because handleServerActionRequest and handleProgressiveActionRequest were dispatched based only on method and content-type checks, without consulting the canUseFilesystemRoutes() gate that protects every other filesystem entrypoint. Both action handlers call matchRoute() internally, so they could load and execute app route code for paths that image optimisation, metadata routes, and public files are correctly blocked from reaching. Gate both action dispatch paths behind canUseFilesystemRoutes() so an out-of-basePath request that no middleware or config rewrite opted back in gets 404 before any action code runs. A middleware rewrite that changes the pathname still opens the gate, preserving the basePath:false rewrite flow. Also make applyAppMiddleware return an explicit didRewrite boolean instead of having the caller infer it by comparing cleanPathname before and after. The forensic comparison was serviceable but brittle: it proved the pathname changed, not that middleware explicitly rewrote. The explicit signal makes the boundary mechanical and robust against future code that might modify cleanPathname for non-rewrite reasons. Tests: out-of-basePath server action POST, progressive form POST, and middleware-rewrite-opts-back-in action dispatch.
Resolve conflict in app-rsc-handler.ts: merge the canUseFilesystemRoutes() gate for metadata routes with main's performance optimization (early-exit when no metadata routes exist, defer handleMetadataRouteRequest import).
Resolve conflicts in app-middleware.ts and app-rsc-handler.ts: keep hadBasePath pass-through (needed for out-of-basePath middleware evaluation) alongside main's new filePath option for middleware module resolution.
Contributor
Performance benchmarksCompared 0 improved · 4 regressed · 2 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% |
The external redirect test counted all non-_next requests and expected exactly two. A stray /favicon.ico fetch from Chromium after document navigation would break the toHaveLength(2) assertion even though the behaviour under test was correct. Filter by the two specific paths under test (/base/client and /outsideBasePath), matching the pattern the first test already uses.
Out-of-basePath App Router requests are preserved (not 404'd) so config `basePath: false` rewrites can still match them. But the handler ran `normalizeTrailingSlash` unconditionally, and that helper always rebuilds the Location as `basePath + pathname`. For a delayed-rejection out-of-basePath request `hadBasePath` is false and `pathname` is still un-stripped, so e.g. `GET /outsideBasePath/` (basePath `/base`, trailingSlash false) returned `308 Location: /base/outsideBasePath`, pushing the request back under basePath before its `basePath: false` rewrite could fire. The mirror case happens with `trailingSlash: true` and a request missing the slash. Next.js models trailing-slash normalisation as a basePath-scoped internal redirect: the auto-generated `/:path+/` rule carries the basePath (no `basePath: false`), so it compiles to `/base/:path+/` and structurally never matches out-of-basePath paths. Gate the redirect on `hadBasePath` to match, preserving out-of-basePath requests for their explicit rewrites. Adds handler tests covering both trailingSlash settings, each triggered by the request shape that exercises its redirect branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
basePathwhen abasePath: falserewrite owns that path.packages/vinext/src/server/app-rsc-request-normalization.ts,packages/vinext/src/server/app-rsc-handler.ts,tests/e2e/app-basepath/app-basepath.spec.tstest/e2e/app-dir/app-basepath/index.test.tsnow passes, including the previously failing external redirect case.Why
App Router basePath handling has two separate responsibilities: normalization must know whether the incoming URL carried the configured basePath, and rewrite matching must still be able to evaluate explicit
basePath: falseroutes. Collapsing out-of-basePath requests into an early 404 loses the information needed to match those explicit routes.normalizeRscRequest()now recordshadBasePathand can preserve out-of-basePath paths for callers that need delayed rejection.basePathunless middleware or config rewrites intentionally route elsewhere.basePath: falsecustom rewrites can match paths outside the configuredbasePath.What changed
POST /base/clientserver action redirects toorigin + "/outsideBasePath"GET /outsideBasePathreturned 404 before the external rewrite could match.GET /outsideBasePathmatches thebasePath: falseexternal rewrite and returns 200.Maintainer review path
packages/vinext/src/server/app-rsc-request-normalization.tshadBasePathresult andallowOutOfBasePathoption.packages/vinext/src/server/app-rsc-handler.tshadBasePath, config rewrites, and middleware rewrites feed the shared filesystem route gate.tests/app-rsc-handler.test.tsbasePath: falserewrites and the negative filesystem exposure guard.tests/e2e/app-basepath/app-basepath.spec.tsplaywright.config.ts,.github/workflows/ci.yml,knip.tsValidation
vp test run tests/app-rsc-handler.test.ts tests/app-rsc-request-normalization.test.tsPLAYWRIGHT_PROJECT=app-basepath vp run test:e2evp env exec --node 24 ./scripts/run-nextjs-deploy-suite.sh /Users/nathan/Projects/vinext/.refs/nextjs-v16.2.6 --retries 0 -c 1 --debug test/e2e/app-dir/app-basepath/index.test.tsvp checkvp run knipRisk / compatibility
basePath: falseApp Router rewrites.basePathor withoutbasePath: falserewrites should keep existing behaviour.Non-goals
AGENTS.md.References
basePath: falsehandling.