fix(cli): #1719 correct dynamic route segment key parsing and fail fast on multi-segment routes#1720
Open
jstockdi wants to merge 1 commit into
Conversation
…sing and fail fast on multi-segment routes
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.
Related Issue
Resolves #1719
Documentation
N/A — no user-facing documentation changes. (The new fail-fast error message for multi-segment routes is self-documenting.)
Summary of Changes
getDynamicSegmentsFromRoute(packages/cli/src/lib/url-utils.js): derive the segment key from the bracket in the route (/\[([^\]]+)\]/) instead of an unanchored.replace(extension, ""), so a param name that contains the extension substring (e.g.[json].js,[posts].ts) no longer produces a mangled key.[json].jsnow builds correctly to/alpha/index.htmlwithparams={"json":"alpha"}.route.replace(/\[([^\]]+)\]/g, ":$1")) so a nested route no longer leaks a literal[..]into the URLPattern.generateGraph(packages/cli/src/lifecycles/graph.js): detect a route with more than one dynamic segment at graph build time andthrowa clear, actionable error (Multiple dynamic route segments are not currently supported (found in ...). Use a single [param] per route.) instead of writing a literal[a]/directory and then failing with an unrelatedTypeError: Cannot read properties of undefined (reading 'resources').getDynamicSegmentsFromRouteat both call sites.packages/cli/test/cases/build.default.dynamic-routing-param-name-ext/— asserts[json].jsbuilds to/alpha/index.htmlwith the correct params and does NOT emit a literal/[json]/directory.packages/cli/test/cases/build.default.dynamic-routing-multi-segment/— asserts a nested[a]/[b].jsroute fails fast with the clear error.Scope / follow-up
Full support for nested/multi-segment dynamic routes (
[category]/[slug]) is intentionally out of scope here — it spans the graph, prerender, bundle, execute-route-module, static-router and adapters, and the single-segment.keydata model would need to become multi-key throughout. This PR makes single-segment routes correct and converts the multi-segment case from a baffling crash into a clear limitation. Full nested-param support is a good follow-up enhancement.The fail-fast check is applied to the page branch only; API routes are exempt, since they already match multiple params at runtime via
URLPattern. Verified against the existing API dynamic-route specs (build.default.api,serve.dynamic-routing).No breaking changes. Existing single-segment dynamic-route /
getStaticPaths/ SSR behavior is unchanged (verified against the existing dynamic-routing, getStaticPaths, and SSR specs for build, serve, and develop).