[james-elicx]

Summary

• preserve the internally matched source route when middleware hides dynamic segments from the browser URL
• distinguish browser-visible and middleware-matched target paths in interception proof metadata
• let navigation planning validate concrete dynamic route IDs without weakening stale-route safety
• strengthen the local Next.js parity fixture to start from the browser-visible rewritten URL

Validation

• vp test run tests/app-browser-interception-context.test.ts tests/app-page-render-identity.test.ts tests/navigation-planner.test.ts tests/app-rsc-route-matching.test.ts tests/app-browser-history-controller.test.ts — 95 passed
• CI=1 PLAYWRIGHT_PROJECT=app-router playwright test tests/e2e/app-router/interception-dynamic-segment-middleware.spec.ts --workers=1 --retries=0 — 4 passed
• targeted vp check on all changed files
• vp run vinext#build

The exact Next.js deploy-suite attempt stopped before deployment because the disposable worktree reuses a dependency symlink and vp env attempted to replace it non-interactively. No suite assertion was reached; CI should run the clean install/build path.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 7fdd26f

[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 7fdd26f against base de014d4 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	117.1 KB	117.3 KB	⚫ +0.2%
Client entry size (gzip)	vinext	112.5 KB	112.7 KB	⚫ +0.2%
Dev server cold start	vinext	2.59 s	2.57 s	⚫ -1.1%
Production build time	vinext	3.12 s	3.10 s	⚫ -0.6%
RSC entry closure size (gzip)	vinext	93.0 KB	93.0 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	156.9 KB	157.0 KB	⚫ +0.0%

View detailed results and traces

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

[ask-bonk]

Review: preserve interception across middleware rewrites

I reviewed the diff against main, traced the runtime call paths, and ran the targeted unit suites (app-browser-interception-context, app-page-render-identity, navigation-planner, app-rsc-route-matching, app-browser-history-controller, app-page-element-builder, app-elements — all green) plus a type check. This looks correct and well-scoped.

What the change does (and why it's sound)

• app-page-render-identity.ts / app-page-element-builder.ts — targetMatchedPathname now derives from routePath (the concretely matched route) instead of displayPathname (browser-visible URL), with displayPathname as the fallback. Since displayPathname already defaults to routePath in buildPageElements, the non-rewrite case is unchanged; only middleware-rewrite navigations see the corrected target. createAppPageRenderIdentity has a single production caller, and targetMatchedPathname is optional with a safe default, so the blast radius is contained.

• app-browser-interception-context.ts — the target-pattern check is now evaluated first, then the source match is tried against both the browser-visible currentPathname and the new middleware-matched currentMatchedPathname. This is what lets /interception-mw (visible) resolve via its rewritten /interception-mw/en match. The new param is optional and null-guarded, so existing callers are unaffected.

• app-browser-state.ts — matchedUrl now prefers interception.targetMatchedUrl with a ?? fallback to the snapshot pathname. Non-interception snapshots are unchanged.

• navigation-planner.ts — matchedUrlFromConcreteRouteId extracts the concrete URL from a route:/... id (correct: encodeRouteId stores the concrete matched pathname, not the pattern). The new branch in findRouteManifestRouteByIdOrMatchedUrl is correctly gated on route === undefined, so it does not weaken the stale-route-safety path (verified by the "does not let stale manifest route IDs override the matched URL" test, where route is defined-but-non-matching and the new branch is skipped).

Dev/prod parity

The fix lives in the shared buildPageElements helper that both the App Router dev RSC entry and prod delegate to, so parity is preserved without a separate edit. This is App Router interception logic, so the Pages Router prod server isn't in scope.

Performance

getCurrentMatchedRoutePathname() is evaluated lazily as a call argument inside the middleware-rewrite fallback branch only — after the previousNextUrl and manifest-interception branches fail — so it does not run on the common navigation path.

Tests

Good coverage: the unit tests assert the new middleware-matched source/target resolution and that displayPathname is unchanged; the planner tests cover both concrete-dynamic-URL matching and stale-route safety. The e2e fixture now starts from the browser-visible /interception-mw (which middleware rewrites to /interception-mw/en) rather than the pre-rewritten URL, which genuinely exercises the new path. The middleware fixture correctly handles the bare /interception-mw path (rewrites to /interception-mw/en).

Minor / non-blocking

• No issues that block merge. One small note: the PR body documents that the Next.js deploy-suite parity run was aborted before deployment (worktree symlink / vp env issue) rather than a passing assertion. The ported e2e spec plus the unit coverage give reasonable confidence, and CI's clean install/build path should cover the rest — worth confirming the e2e job is green before merge.

LGTM pending green CI.

[ask-bonk]

Review posted on PR #2342.

Summary

I reviewed the interception-across-middleware-rewrites fix, traced the runtime call paths, and ran the targeted unit suites (all green) plus a type check. No blocking issues found — LGTM pending green CI.

Key findings

The change is correct and well-scoped:

• targetMatchedPathname now derives from the concretely-matched routePath instead of the browser-visible displayPathname, with a safe fallback that leaves the non-rewrite case unchanged.
• The interception-context resolver now matches the source pattern against both the visible pathname and the new middleware-matched pathname — this is what makes /interception-mw resolve via its rewritten /interception-mw/en match.
• The navigation-planner's new concrete-route-id matching is correctly gated on route === undefined, so it does not weaken stale-route safety (verified against the dedicated stale-route test).

Parity & performance verified:

• The fix lives in the shared buildPageElements helper, so App Router dev/prod inherit it automatically. Pages Router isn't in scope (App Router interception logic).
• getCurrentMatchedRoutePathname() is evaluated lazily only in the fallback branch, not on the common navigation path.

Tests are solid:

• Unit tests cover the new source/target resolution and confirm displayPathname is unchanged.
• The e2e spec now starts from the browser-visible /interception-mw, genuinely exercising the rewrite path.

One non-blocking note: the PR body documents that the Next.js deploy-suite parity run aborted before deployment (worktree/vp env issue) rather than passing. Worth confirming the e2e CI job is green before merge.

github run