refactor(routing): extract App Router route graph builder

[NathanDrake2406]

What this changes

Extracts App Router route graph construction into packages/vinext/src/routing/app-route-graph.ts.

app-router.ts now stays focused on the public appRouter() cache wrapper and request-time trie matching, while the new graph builder owns filesystem scanning, route metadata materialization, validation, synthetic parallel slot routes, and exported route shape types.

Why

Will make #726 easier
The App Router routing module had grown into a mixed-purpose file that combined discovery, graph materialization, cache ownership, and request matching. That made route graph changes noisy to review and more likely to collide with App Router runtime entry work.

This keeps the graph-building pressure isolated inside routing code and avoids touching generated entry templates such as entries/app-rsc-entry.ts.

Approach

• Move route shape types and discovery helpers into routing/app-route-graph.ts.
• Add buildAppRouteGraph(appDir, matcher) as the direct construction boundary.
• Keep appRouter() behavior-compatible by delegating to the builder, then caching the returned routes as before.
• Keep matchAppRoute() and trie caching in app-router.ts, since those are request-time matching concerns rather than graph construction concerns.
• Add a direct graph-builder test covering pages, route handlers, layouts, and inherited parallel slots.

Validation

• vp test run tests/app-route-graph.test.ts tests/routing.test.ts tests/route-sorting.test.ts tests/page-extensions-routing.test.ts
  • 4 files passed, 148 tests passed
• vp run vinext#build
  • completed, and the build output includes dist/routing/app-route-graph.js
• vp check
  • passed with 1071 formatted files and no warnings, lint errors, or type errors in 457 files

Note: the first vp check run happened before the package build had generated workspace package output and failed on benchmarks/vinext/vite.config.ts resolving vinext. After vp run vinext#build, rerunning vp check passed.

Risks / follow-ups

The main risk is accidental drift in synthetic parallel slot route materialization. I covered that with the new graph-builder test plus the existing routing and route sorting suites that exercise slot inheritance, conflict errors, intercepting routes, catch-all validation, and route ordering.

No generated App Router entry files are touched in this PR.

[pkg-pr-new]

Open in StackBlitz

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

commit: 3f2072b

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[james-elicx]

/bigbonk review

[ask-bonk]

Clean extraction. The move is mechanically faithful — I diffed the original appRouter body against the new buildAppRouteGraph and the only intentional change is dropping the unused _appDir parameter from discoverSlotSubRoutes. All 11 consumers of AppRoute and the single consumer of computeRootParamNames continue to import from app-router.ts via the re-exports, so nothing breaks downstream. ParallelSlot and InterceptingRoute have zero direct importers outside the routing module, so omitting them from the re-exports is correct.

Two minor observations below — neither is blocking.

[ask-bonk]

Nit: The test covers the happy path well (pages, route handlers, layouts, inherited parallel slots). If you want to strengthen the graph-builder test as a follow-up, consider adding cases for:

• Synthetic parallel slot sub-routes (@slot/subpage/page.tsx creating a new URL route)
• Route group transparency (e.g. (marketing)/about/page.tsx → /about)
• page.tsx + route.ts conflict at the same segment (should throw)

These are already covered by the existing routing.test.ts suite so nothing is at risk here — it's just a matter of whether you want buildAppRouteGraph to be independently testable for those behaviors without going through the appRouter cache wrapper.

[ask-bonk]

Nit: The return type Promise<{ routes: AppRoute[] }> wraps the array in an object. Right now routes is the only field, so appRouter immediately unwraps it with graph.routes. If you anticipate adding more graph metadata later (e.g. a manifest hash, diagnostic warnings, or the root layout path), the object wrapper makes sense as a forward-compatible seam. If not, returning AppRoute[] directly would be simpler. Either way is fine — just flagging the trade-off.

[ask-bonk]

Review submitted. Here's a summary of what I found:

Approved. This is a clean mechanical extraction with no behavioral changes.

Key findings:

• The code moved from app-router.ts to app-route-graph.ts is identical to the original, with one intentional cleanup: the unused _appDir parameter was dropped from discoverSlotSubRoutes.
• All 11 consumers of AppRoute and the single consumer of computeRootParamNames import from app-router.ts, which re-exports them — no downstream breakage.
• ParallelSlot and InterceptingRoute have zero direct importers outside the routing module, so correctly omitted from re-exports.
• No package.json exports changes needed since routing modules are internal.
• The appRouter() function now delegates to buildAppRouteGraph() and caches the result, preserving the same cache semantics.

Two minor nits posted:

1. The new test covers the happy path well but could be extended to cover synthetic slot sub-routes, route groups, and page/route conflicts as a follow-up.
2. The { routes: AppRoute[] } wrapper return type is forward-compatible but could be simplified to AppRoute[] if no additional metadata fields are planned.

github run