Skip to content

fix(dev): fix regression: typegen for optional dynamic params #13725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .changeset/witty-buttons-shake.md
Original file line number Diff line number Diff line change
@@ -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
```
4 changes: 2 additions & 2 deletions integration/typegen-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
61 changes: 22 additions & 39 deletions packages/react-router-dev/typegen/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ export function generateRoutes(ctx: Context): Array<VirtualFile> {
// pages
const fullpath = Route.fullpath(lineage);
if (!fullpath) continue;
const pages = explodeOptionalSegments(fullpath);

const pages = expand(fullpath);
pages.forEach((page) => allPages.add(page));

// routePages
Expand Down Expand Up @@ -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<string> {
function recurse(segments: Array<string>, index: number): Array<string> {
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<string>();
for (let result of recurse(segments, 0)) {
if (result !== "/") result = result.replace(/\/$/, "");
expanded.add(result);
}
return expanded;
}