Skip to content

fix(routing): prevent dynamic static-suffix routes from shadowing static routes #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JaredStowell wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(routing): prevent dynamic static-suffix routes from shadowing static routes #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/vinext/src/routing/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,11 @@ export function routePrecedence(pattern: string): number {
score -= staticPrefixCount * 50;
}

// Keep all dynamic routes below purely static routes.
// Static routes always score 0, so dynamic scores must stay positive.
if (isDynamic && score <= 0) {
score = 1;
Comment on lines +72 to +73
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid collapsing dynamic precedence scores

Clamping every dynamic route with score <= 0 to 1 removes the relative ordering between distinct dynamic patterns that were previously differentiated by infix static specificity. For example, /:a/x/y and /:a/x/:b now both score 1, so the router falls back to lexicographic order and can try /:a/x/:b first, incorrectly shadowing the more specific /:a/x/y on /foo/x/y. This regression is introduced by the new clamp because the prior scoring kept these routes distinct.

Useful? React with 👍 / 👎.

}

return score;
}
11 changes: 11 additions & 0 deletions tests/routing.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,23 @@ import { describe, it, expect } from "vitest";
import path from "node:path";
import { pagesRouter, matchRoute } from "../packages/vinext/src/routing/pages-router.js";
import { appRouter, matchAppRoute, invalidateAppRouteCache } from "../packages/vinext/src/routing/app-router.js";
import { routePrecedence } from "../packages/vinext/src/routing/utils.js";

const FIXTURE_DIR = path.resolve(
import.meta.dirname,
"./fixtures/pages-basic/pages",
);

describe("routePrecedence", () => {
it("keeps fully static routes ahead of dynamic routes with static suffixes", () => {
const staticScore = routePrecedence("/users/settings");
const dynamicWithSuffixScore = routePrecedence("/:id/settings");

expect(staticScore).toBe(0);
expect(dynamicWithSuffixScore).toBeGreaterThan(staticScore);
});
});

describe("pagesRouter - route discovery", () => {
it("discovers pages from the fixture directory", async () => {
const routes = await pagesRouter(FIXTURE_DIR);
Expand Down