Skip to content

fix(dart_frog_gen): order route mounts by specificity#1969

Open
Yusufihsangorgel wants to merge 1 commit into
dart-frog-dev:mainfrom
Yusufihsangorgel:codegen-mount-order
Open

fix(dart_frog_gen): order route mounts by specificity#1969
Yusufihsangorgel wants to merge 1 commit into
dart-frog-dev:mainfrom
Yusufihsangorgel:codegen-mount-order

Conversation

@Yusufihsangorgel

@Yusufihsangorgel Yusufihsangorgel commented Jul 8, 2026

Copy link
Copy Markdown

READY

Non-breaking bug fix — no public API change. This only changes the
ordering in which generated route mounts are registered on the root router,
so requests resolve to the correct handler. No public API changes, no change
to which routes exist or their signatures, and generated output stays
byte-for-byte compatible aside from mount order. It does not warrant a major
version bump.

Description

dart_frog build / dart_frog dev can generate a root router where a short
prefix mount is registered before the longer literal paths nested under it.
With the layout from the issue:

routes/api/v1/event/[eventId]/tasks/
  index.dart          # GET  .../tasks
  [taskId].dart       # any  .../tasks/<taskId>   (flat dynamic file)
  add/index.dart      # POST .../tasks/add
  start/index.dart    # POST .../tasks/start

codegen emitted:

final router = Router()
  ..mount('/api/v1/event/<eventId>/tasks', ...)        // generic — first
  ..mount('/api/v1/event/<eventId>/tasks/start', ...)
  ..mount('/api/v1/event/<eventId>/tasks/add', ...);   // specific — last

Router.mount('/…/tasks', …) also matches /…/tasks/<rest> and falls through
on a 404, so POST /…/tasks/add was captured by the tasks router and served
by its /<taskId> file with taskId == "add" instead of hitting add/.

Root cause

The order of the flat root-level mounts is the order of
RouteConfiguration.directories. Since #1931 (which removed the
.reversed in the pre-gen hooks to fix #1930), that list is the raw
depth-first order — parent directory before its children — i.e. the generic
prefix is mounted first.

#1930 and #1959 are the two mirror images of the same ordering problem:

Issue Static route lives in… Dynamic route lives in… Correct order
#1930 /books mount (popular.dart) /books/<id> mount /books before /books/<id>
#1959 /tasks/add mount /tasks mount ([taskId].dart) /tasks/add before /tasks

Neither "generic first" nor "longest first" satisfies both. The fix orders the
mounts by segment specificity, most specific first, where per position a
static segment out-ranks the end of a shorter route, which in turn out-ranks a
dynamic segment:

  • static (2) > end-of-path (1) > dynamic (0)

That yields /tasks/add before /tasks (#1959) while keeping /books
before /books/<id> (#1930), because /books ends where /books/<id>
has a dynamic segment.

Fix

buildRouteConfiguration now sorts directories with a new
compareRouteDirectorySpecificity comparator (ties fall back to the route
string for a stable, deterministic order). This is done in dart_frog_gen,
the single source both the dev-server and prod-server hooks consume, keeping
the ordering decision in one place — consistent with #1931 which made the
hooks trust the order coming out of buildRouteConfiguration.

Generated output after the change:

final router = Router()
  ..mount('/api/v1/event/<eventId>/tasks/add', ...)
  ..mount('/api/v1/event/<eventId>/tasks/start', ...)
  ..mount('/api/v1/event/<eventId>/tasks', ...);

Verified end to end (local CLI + real generated prod build):

POST /api/v1/event/E1/tasks/add    -> ADD task for event=E1        (was: DYNAMIC taskId=add)
POST /api/v1/event/E1/tasks/start  -> START task for event=E1      (was: DYNAMIC taskId=start)
POST /api/v1/event/E1/tasks/XYZ    -> DYNAMIC taskId=XYZ event=E1  (unchanged — real dynamic still works)
GET  /api/v1/event/E1/tasks        -> LIST tasks for event=E1      (unchanged)

Note on #1958

The same ordering change also helps
#1958
(_middleware.dart executed multiple times per request), but does not fully
close it, so this PR does not claim it.

For the minimal layout in #1958 (routes/v2/_middleware.dart,
routes/v2/index.dart, routes/v2/person/[personId]/address.dart) the parent
middleware went from 2 executions → 1 for a single
GET /v2/person/123/address: with the specificity order,
/v2/person/<personId> is mounted before /v2, so it matches first and
returns 200 without the request ever falling through the /v2 prefix mount.

However, the root cause of #1958 is architectural. dart_frog flattens nested
routers into root-level mount()s, and every Router.mount('/prefix', …)
registers a /prefix/<rest> fallthrough matcher. Whenever a request has to
pass an intermediate prefix mount that 404s — e.g. add routes/v2/person/index.dart
so a /v2/person mount exists — that prefix still matches, runs the cascaded
parent middleware, 404s, and falls through, so the middleware runs twice again.
Ordering cannot avoid that; a full fix needs a change to how nested routers are
mounted, which is out of scope here.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant