fix(routing): accept any non-] chars in dynamic segment names (Next.js parity)#6
Closed
NathanDrake2406 wants to merge 1 commit into
Closed
fix(routing): accept any non-] chars in dynamic segment names (Next.js parity)#6NathanDrake2406 wants to merge 1 commit into
NathanDrake2406 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Aligns vinext’s route parsing with Next.js behavior by allowing dynamic segment names to include any character except ], fixing cases where valid Next.js routes were previously treated as literal segments.
Changes:
- Relaxed dynamic segment regexes in App Router graph building and Pages Router file-to-route conversion to accept any non-
]param name content. - Updated
patternToNextFormatand the router shim’s param-name extraction to handle broader param-name characters. - Added a unit test for App Router graph materialization and corrected a comment about
routePrecedencepotentially going negative.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/app-route-graph.test.ts | Adds coverage for dotted/colon/@ param names being materialized into /:param patterns. |
| packages/vinext/src/shims/router.ts | Broadens param-name extraction from bracket patterns and internal :param patterns. |
| packages/vinext/src/routing/utils.ts | Updates comment to reflect that dynamic route precedence scores can be negative. |
| packages/vinext/src/routing/route-validation.ts | Relaxes patternToNextFormat param-name matching to allow non-/ characters. |
| packages/vinext/src/routing/pages-router.ts | Relaxes Pages Router dynamic segment parsing regexes for Next.js parity. |
| packages/vinext/src/routing/app-route-graph.ts | Relaxes App Router dynamic segment parsing regexes for Next.js parity. |
Comments suppressed due to low confidence (1)
packages/vinext/src/routing/pages-router.ts:143
- These regex changes affect Pages Router filename→pattern conversion, but the PR only adds coverage for the App Router graph builder. There are existing Pages Router routing tests (e.g. for hyphenated params) but none covering dotted/colon/@ param names; adding a Pages Router-specific test would help ensure parity and prevent regressions in this code path.
// Catch-all: [...slug] -> :slug+ (param names may contain any non-] chars)
// Matches Next.js PARAMETER_PATTERN.
const catchAllMatch = segment.match(/^\[\.\.\.([^\]]+)\]$/);
if (catchAllMatch) {
if (i !== segments.length - 1) return null;
isDynamic = true;
params.push(catchAllMatch[1]);
urlSegments.push(`:${catchAllMatch[1]}+`);
continue;
}
// Optional catch-all: [[...slug]] -> :slug* (param names may contain any non-] chars)
const optionalCatchAllMatch = segment.match(/^\[\[\.\.\.([^\]]+)\]\]$/);
if (optionalCatchAllMatch) {
if (i !== segments.length - 1) return null;
isDynamic = true;
params.push(optionalCatchAllMatch[1]);
urlSegments.push(`:${optionalCatchAllMatch[1]}*`);
continue;
}
// Dynamic segment: [id] -> :id (param names may contain any non-] chars)
const dynamicMatch = segment.match(/^\[([^\]]+)\]$/);
if (dynamicMatch) {
isDynamic = true;
params.push(dynamicMatch[1]);
urlSegments.push(`:${dynamicMatch[1]}`);
continue;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+286
to
+293
| await writeAppFile(appDir, "[repo:name]/page.tsx", EMPTY_PAGE); | ||
| await writeAppFile(appDir, "products/[variant.id]/page.tsx", EMPTY_PAGE); | ||
| await writeAppFile(appDir, "users/[user@domain]/page.tsx", EMPTY_PAGE); | ||
|
|
||
| const graph = await buildAppRouteGraph(appDir, createValidFileMatcher()); | ||
| const patterns = graph.routes.map((r) => r.pattern); | ||
|
|
||
| expect(patterns).toContain("/:repo:name"); |
…s parity) Next.js PARAMETER_PATTERN accepts any non-] characters inside brackets: [repo:name], [variant.id], [user@domain] vinext was restricting param names to [\w-]+, silently treating segments with dots, colons, or @ as literal URLs. This fix aligns the regexes with Next.js: - app-route-graph.ts: ^\[([^\]]+)\]$ (was [\w-]+) - pages-router.ts: same - route-validation.ts: patternToNextFormat now backtracks over +/* suffixes - shims/router.ts: extractRouteParamNames now accepts dotted/colon names Also corrects a misleading comment in routePrecedence: dynamic routes CAN score negative, but the trie matcher (route-trie.ts) always checks static children before dynamic children at each node, so purely-static routes still win at request time. Next.js reference: https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/get-dynamic-param.ts#L175 Closes #7
NathanDrake2406
force-pushed
the
fix/routing-dynamic-segments-and-precedence
branch
from
May 4, 2026 09:46
a109d11 to
e795196
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aligns vinext's dynamic segment name parsing with Next.js
PARAMETER_PATTERN, which accepts any non-\]characters inside brackets. vinext was restricting param names to[\w-]+, silently treating valid Next.js segments like[repo:name],[variant.id], and[user@domain]as literal URL segments.Changes
Regex fixes (4 files)
routing/app-route-graph.ts/^\[\.\.\.([\w-]+)\]$//^\[\.\.\.([^\]]+)\]$/routing/pages-router.tsrouting/route-validation.ts/:([\w-]+)\+/g/:([^/]+)\+/gshims/router.ts\[{1,2}(?:\.\.\.)?([\w-]+)\]{1,2}\[{1,2}(?:\.\.\.)?([^\]]+)\]{1,2}Comment fix
routing/utils.ts: Removed the incorrect claim thatroutePrecedence"never goes negative." Dynamic routes can score negative (e.g./a/:b/c/:d= −346), but this is harmless because the trie matcher (route-trie.ts) always checks static children before dynamic children at each node, so purely-static routes still win at request time.Next.js reference
PARAMETER_PATTERN: packages/next/src/shared/lib/router/utils/get-dynamic-param.ts#L175[^\]]+group matches any character except\], which is why[repo:name]is valid in Next.js.Test coverage
Added a test in
tests/app-route-graph.test.tsthat verifies routes with dotted, colon, and at-sign param names are materialized correctly.Bug context
Bug candidate #7 from internal audit. Bug #8 (
routePrecedencenegative scores) was investigated and determined to be a false positive — the trie-based matcher inherently handles static-before-dynamic priority regardless of sort-order score.