diff --git a/.changeset/witty-buttons-shake.md b/.changeset/witty-buttons-shake.md new file mode 100644 index 0000000000..249aa6ed85 --- /dev/null +++ b/.changeset/witty-buttons-shake.md @@ -0,0 +1,48 @@ +--- +"@react-router/dev": patch +--- + +Fix href types for optional dynamic params + +7.6.1 introduced fixes for `href` when using optional static segments, +but those fixes caused regressions with how optional dynamic params worked in 7.6.0: + +```ts +// 7.6.0 +href("/users/:id?"); // ✅ +href("/users/:id?", { id: 1 }); // ✅ + +// 7.6.1 +href("/users/:id?"); // ❌ +href("/users/:id?", { id: 1 }); // ❌ +``` + +Now, optional static segments are expanded into different paths for `href`, but optional dynamic params are not. +This way `href` can unambiguously refer to an exact URL path, all while keeping the number of path options to a minimum. + +```ts +// 7.6.2 + +// path: /users/:id?/edit? +href(" +// ^ suggestions when cursor is here: +// +// /users/:id? +// /users/:id?/edit +``` + +Additionally, you can pass `params` from component props without needing to narrow them manually: + +```ts +declare const params: { id?: number }; + +// 7.6.0 +href("/users/:id?", params); + +// 7.6.1 +href("/users/:id?", params); // ❌ +"id" in params ? href("/users/:id", params) : href("/users"); // works... but is annoying + +// 7.6.2 +href("/users/:id?", params); // restores behavior of 7.6.0 +``` diff --git a/integration/typegen-test.ts b/integration/typegen-test.ts index 3ce1e638c2..c96e7e1ca0 100644 --- a/integration/typegen-test.ts +++ b/integration/typegen-test.ts @@ -537,11 +537,11 @@ test.describe("typegen", () => { href("/required-param/:req") href("/required-param/:req", { req: "hello" }) + // @ts-expect-error href("/optional-param") - href("/optional-param/:opt", { opt: "hello" }) // @ts-expect-error + href("/optional-param/:opt", { opt: "hello" }) href("/optional-param/:opt?") - // @ts-expect-error href("/optional-param/:opt?", { opt: "hello" }) href("/leading-and-trailing-slash") diff --git a/packages/react-router-dev/typegen/generate.ts b/packages/react-router-dev/typegen/generate.ts index aa094a086a..e1f9c805eb 100644 --- a/packages/react-router-dev/typegen/generate.ts +++ b/packages/react-router-dev/typegen/generate.ts @@ -77,7 +77,8 @@ export function generateRoutes(ctx: Context): Array { // pages const fullpath = Route.fullpath(lineage); if (!fullpath) continue; - const pages = explodeOptionalSegments(fullpath); + + const pages = expand(fullpath); pages.forEach((page) => allPages.add(page)); // routePages @@ -348,48 +349,30 @@ function paramsType(path: string) { ); } -// https://github.com/remix-run/react-router/blob/7a7f4b11ca8b26889ad328ba0ee5a749b0c6939e/packages/react-router/lib/router/utils.ts#L894C1-L937C2 -function explodeOptionalSegments(path: string): string[] { - let segments = path.split("/"); - if (segments.length === 0) return []; - - let [first, ...rest] = segments; +function expand(fullpath: string): Set { + function recurse(segments: Array, index: number): Array { + if (index === segments.length) return [""]; + const segment = segments[index]; - // Optional path segments are denoted by a trailing `?` - let isOptional = first.endsWith("?"); - // Compute the corresponding required segment: `foo?` -> `foo` - let required = first.replace(/\?$/, ""); + const isOptional = segment.endsWith("?"); + const isDynamic = segment.startsWith(":"); + const required = segment.replace(/\?$/, ""); - if (rest.length === 0) { - // Interpret empty string as omitting an optional segment - // `["one", "", "three"]` corresponds to omitting `:two` from `/one/:two?/three` -> `/one/three` - return isOptional ? [required, ""] : [required]; - } + const keep = !isOptional || isDynamic; + const kept = isDynamic ? segment : required; - let restExploded = explodeOptionalSegments(rest.join("/")); + const withoutSegment = recurse(segments, index + 1); + const withSegment = withoutSegment.map((rest) => [kept, rest].join("/")); - let result: string[] = []; - - // All child paths with the prefix. Do this for all children before the - // optional version for all children, so we get consistent ordering where the - // parent optional aspect is preferred as required. Otherwise, we can get - // child sections interspersed where deeper optional segments are higher than - // parent optional segments, where for example, /:two would explode _earlier_ - // then /:one. By always including the parent as required _for all children_ - // first, we avoid this issue - result.push( - ...restExploded.map((subpath) => - subpath === "" ? required : [required, subpath].join("/") - ) - ); - - // Then, if this is an optional value, add all child versions without - if (isOptional) { - result.push(...restExploded); + if (keep) return withSegment; + return [...withoutSegment, ...withSegment]; } - // for absolute paths, ensure `/` instead of empty segment - return result.map((exploded) => - path.startsWith("/") && exploded === "" ? "/" : exploded - ); + const segments = fullpath.split("/"); + const expanded = new Set(); + for (let result of recurse(segments, 0)) { + if (result !== "/") result = result.replace(/\/$/, ""); + expanded.add(result); + } + return expanded; }