fix(routing): omit empty optional catch-all params#987
Conversation
Optional catch-all routes currently materialize the missing segment list as an empty array. Next.js leaves the param key absent when the optional catch-all capture group does not match, so pages and app routes can observe a different params shape at the zero-segment path. The route trie treated zero remaining segments as a captured value. The fix keeps the route match while returning an empty params object for that zero-segment branch, and applies the same normalization to standalone app RSC pattern matching. Tests cover shared trie matching, app RSC matching, app route discovery, pages route discovery, and hyphenated optional catch-all params.
commit: |
|
/bigbonk review |
There was a problem hiding this comment.
Clean, well-scoped parity fix. The change is correct: Next.js's route-matcher.ts only writes a param when the regex capture is defined, so optional catch-all with zero segments should produce {}, not { path: [] }. Both changed files (route-trie.ts and app-rsc-route-matching.ts) are consistent with each other, the test coverage is thorough (trie boundary, RSC matcher boundary, integration fixtures with hyphenated names), and downstream consumers are already safe:
resolveAppPageSegmentParamsinapp-page-params.tshandles bothundefinedand empty arrays viaisEmptyOptionalCatchAllpickRouteParamsinapp-page-request.tsexplicitly checksvalue !== undefinedbuildUrlFromParamsinprerender.tsproduces the same URL for bothundefinedand[]- The test fixture pages already use the
params.path ?? []pattern, which is the standard Next.js idiom
The createParams() / createRouteParams() helper extraction is a nice cleanup too — reduces duplication of Object.create(null) and makes the null-prototype intent obvious at a glance.
One minor nit below on the linear matcher parity test, but not blocking.
| if (pp.endsWith("*")) { | ||
| if (i !== patternParts.length - 1) return null; | ||
| const paramName = pp.slice(1, -1); | ||
| params[paramName] = urlParts.slice(i); | ||
| const remaining = urlParts.slice(i); | ||
| if (remaining.length > 0) { | ||
| params[paramName] = remaining; | ||
| } | ||
| return params; |
There was a problem hiding this comment.
Nit: this linearMatchPattern helper is the reference implementation that the parity fuzz test (below) compares the trie against. Updating it in the same PR as the trie change is correct — the two must stay in sync — but it's worth a one-line comment here noting why, so a future reader doesn't wonder if the test is just tautologically matching the implementation.
Something like:
| if (pp.endsWith("*")) { | |
| if (i !== patternParts.length - 1) return null; | |
| const paramName = pp.slice(1, -1); | |
| params[paramName] = urlParts.slice(i); | |
| const remaining = urlParts.slice(i); | |
| if (remaining.length > 0) { | |
| params[paramName] = remaining; | |
| } | |
| return params; | |
| if (pp.endsWith("*")) { | |
| if (i !== patternParts.length - 1) return null; | |
| const paramName = pp.slice(1, -1); | |
| const remaining = urlParts.slice(i); | |
| // Match Next.js: omit key when zero segments captured (parity with trie) | |
| if (remaining.length > 0) { | |
| params[paramName] = remaining; | |
| } | |
| return params; | |
| } |
|
Review submitted. Here's a summary of what I found: Verdict: Approved This is a clean, well-scoped Next.js parity fix. The change correctly aligns vinext with Next.js behavior where optional catch-all routes with zero matched segments produce Key findings from the review:
|
What this changes
Optional catch-all routes now omit the param key when the route matches with zero remaining segments. For example,
/shopmatching/shop/[[...path]]now produces{}instead of{ path: [] }, while/shop/a/bstill produces{ path: ["a", "b"] }.Why
Next.js treats the zero-segment optional catch-all case as a missing capture, not as an empty captured array. The relevant source path is:
route-regex.tsemits an optional repeated capture for optional catch-all params.route-matcher.tsonly writes a param when the regex capture is defined.get-dynamic-param.test.tscovers the downstream app-router interpretation of an absent optional catch-all param as anulltree segment.vinext's route trie instead materialized the zero-segment optional catch-all as
[], which made app/pages route params observably diverge from Next.js.Approach
The fix keeps app shape in codegen and behavior in normal modules:
Validation
vp test run tests/route-trie.test.ts tests/app-rsc-route-matching.test.ts tests/routing.test.tsvp test run tests/pages-router.test.ts tests/app-router.test.ts -t "optional catch-all"vp check packages/vinext/src/routing/route-trie.ts packages/vinext/src/server/app-rsc-route-matching.ts tests/route-trie.test.ts tests/app-rsc-route-matching.test.ts tests/routing.test.tsRisks / follow-ups
This is a compatibility change for the zero-segment optional catch-all params shape. Code that depended on vinext's previous
[]value for missing optional catch-all params will need to use the sameparams.slug ?? []pattern that works in Next.js.