[PR #420] [MERGED] fix: reject conflicting dynamic route siblings during route discovery #556

New issue

Closed

opened 2026-05-06 13:08:45 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:08:45 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/420
Author: @JaredStowell
Created: 3/10/2026
Status: ✅ Merged
Merged: 3/10/2026
Merged by: @james-elicx

Base: main ← Head: jstowell/fix-pages-router-dynamic-conflicts

📝 Commits (2)

c61f8d5 Validate route patterns in routers
6f9f253 Validate intercepting routes

📊 Changes

5 files changed (+420 additions, -6 deletions)

View changed files

📝 packages/vinext/src/routing/app-router.ts (+10 -0)
📝 packages/vinext/src/routing/pages-router.ts (+6 -6)
➕ packages/vinext/src/routing/route-validation.ts (+163 -0)
📝 tests/route-sorting.test.ts (+217 -0)
📝 tests/routing.test.ts (+24 -0)

📄 Description

Reject ambiguous dynamic route siblings during route discovery instead of silently accepting them and resolving by first-match wins.

This fixes the case where routes like pages/posts/[id].tsx and pages/posts/[slug].tsx were both registered even though they represent the same URL shape. A request like /posts/42 would match whichever route sorted first, which diverges from Next.js behavior.

Bug

Before this change, vinext treated these as distinct internal patterns:

pages/posts/[id].tsx -> /posts/:id
pages/posts/[slug].tsx -> /posts/:slug

Because the param name was preserved in the internal pattern, both routes were accepted and later matched independently. At request time, /posts/42 matched one of them by scan/sort order.

That is ambiguous. The param name differs, but the route shape does not.

Next.js rejects this configuration during route validation instead of allowing runtime first-match behavior.

What changed

added a shared route validation module based on Next.js sorted-routes
validate discovered route patterns before sorting in:
- Pages Router
- Pages API Router
- App Router
moved patternToNextFormat() into the shared validation module and re-exported it from pages-router.ts
expanded route validation coverage with targeted tests for:
- conflicting dynamic sibling pages
- conflicting dynamic sibling API routes
- conflicting dynamic sibling app routes
- reused param names in a single path
- invalid catch-all placement
- optional/required catch-all conflicts
- optional catch-all specificity conflicts
- malformed bracket / ellipsis cases
- param names differing only by non-word symbols

Example

Given:

pages/posts/[id].tsx
pages/posts/[slug].tsx

Before:

both routes were registered
/posts/42 resolved to one file by first-match wins

After:

route discovery throws
the ambiguous route set is rejected up front

Next.js reference

Adapted from:

packages/next/src/shared/lib/router/utils/sorted-routes.ts
test/unit/page-route-sorter.test.ts

Verification

Reproduced the bug on current upstream/main in a clean worktree:

both /posts/:id and /posts/:slug were registered
/posts/42 matched one file instead of rejecting the conflict

Validated the fix on top of current upstream/main with:

pnpm test tests/route-sorting.test.ts tests/routing.test.ts tests/page-extensions-routing.test.ts
pnpm run typecheck
pnpm run fmt:check tests/route-sorting.test.ts packages/vinext/src/routing/pages-router.ts packages/vinext/src/routing/app-router.ts packages/vinext/src/routing/route-validation.ts

Notes

This is intentionally scan-time validation, not match-time handling. The goal is to reject ambiguous route definitions before route precedence can hide the conflict.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/420 **Author:** [@JaredStowell](https://github.com/JaredStowell) **Created:** 3/10/2026 **Status:** ✅ Merged **Merged:** 3/10/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `jstowell/fix-pages-router-dynamic-conflicts` --- ### 📝 Commits (2) - [`c61f8d5`](https://github.com/cloudflare/vinext/commit/c61f8d51bbcd535fea9cc891756fff37b4a20ae9) Validate route patterns in routers - [`6f9f253`](https://github.com/cloudflare/vinext/commit/6f9f253ffcb6cf28dc1e99e5ee0837fa98d1c811) Validate intercepting routes ### 📊 Changes **5 files changed** (+420 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/routing/app-router.ts` (+10 -0) 📝 `packages/vinext/src/routing/pages-router.ts` (+6 -6) ➕ `packages/vinext/src/routing/route-validation.ts` (+163 -0) 📝 `tests/route-sorting.test.ts` (+217 -0) 📝 `tests/routing.test.ts` (+24 -0) </details> ### 📄 Description Reject ambiguous dynamic route siblings during route discovery instead of silently accepting them and resolving by first-match wins. This fixes the case where routes like `pages/posts/[id].tsx` and `pages/posts/[slug].tsx` were both registered even though they represent the same URL shape. A request like `/posts/42` would match whichever route sorted first, which diverges from Next.js behavior. ## Bug Before this change, vinext treated these as distinct internal patterns: - `pages/posts/[id].tsx` -> `/posts/:id` - `pages/posts/[slug].tsx` -> `/posts/:slug` Because the param name was preserved in the internal pattern, both routes were accepted and later matched independently. At request time, `/posts/42` matched one of them by scan/sort order. That is ambiguous. The param name differs, but the route shape does not. Next.js rejects this configuration during route validation instead of allowing runtime first-match behavior. ## What changed - added a shared route validation module based on Next.js `sorted-routes` - validate discovered route patterns before sorting in: - Pages Router - Pages API Router - App Router - moved `patternToNextFormat()` into the shared validation module and re-exported it from `pages-router.ts` - expanded route validation coverage with targeted tests for: - conflicting dynamic sibling pages - conflicting dynamic sibling API routes - conflicting dynamic sibling app routes - reused param names in a single path - invalid catch-all placement - optional/required catch-all conflicts - optional catch-all specificity conflicts - malformed bracket / ellipsis cases - param names differing only by non-word symbols ## Example Given: - `pages/posts/[id].tsx` - `pages/posts/[slug].tsx` Before: - both routes were registered - `/posts/42` resolved to one file by first-match wins After: - route discovery throws - the ambiguous route set is rejected up front ## Next.js reference Adapted from: - `packages/next/src/shared/lib/router/utils/sorted-routes.ts` - `test/unit/page-route-sorter.test.ts` ## Verification Reproduced the bug on current `upstream/main` in a clean worktree: - both `/posts/:id` and `/posts/:slug` were registered - `/posts/42` matched one file instead of rejecting the conflict Validated the fix on top of current `upstream/main` with: - `pnpm test tests/route-sorting.test.ts tests/routing.test.ts tests/page-extensions-routing.test.ts` - `pnpm run typecheck` - `pnpm run fmt:check tests/route-sorting.test.ts packages/vinext/src/routing/pages-router.ts packages/vinext/src/routing/app-router.ts packages/vinext/src/routing/route-validation.ts` ## Notes This is intentionally scan-time validation, not match-time handling. The goal is to reject ambiguous route definitions before route precedence can hide the conflict. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>