fix typegen for optional dynamic params

Author: pcattori

.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 convenience of 7.6.0
+```

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")

packages/react-router-dev/typegen/generate.ts

@@ -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
@@ -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;
 }