fix href optionals

Author: pcattori

integration/typegen-test.ts

@@ -99,15 +99,15 @@ test.describe("typegen", () => {
           import type { Expect, Equal } from "../expect-type"
           import type { Route } from "./+types/only-optional"
           export function loader({ params }: Route.LoaderArgs) {
-            type Test = Expect<Equal<typeof params, { id?: string }>>
+            type Test = Expect<Equal<typeof params.id, string | undefined>>
             return null
           }
         `,
         "app/routes/optional-then-required.tsx": tsx`
           import type { Expect, Equal } from "../expect-type"
           import type { Route } from "./+types/optional-then-required"
           export function loader({ params }: Route.LoaderArgs) {
-            type Test = Expect<Equal<typeof params, { id: string }>>
+            type Test = Expect<Equal<typeof params.id,  string>>
             return null
           }
         `,
@@ -116,11 +116,12 @@ test.describe("typegen", () => {
           import type { Route } from "./+types/required-then-optional"
 
           export function loader({ params }: Route.LoaderArgs) {
-            type Test = Expect<Equal<typeof params, { id: string }>>
+            type Test = Expect<Equal<typeof params.id, string>>
             return null
           }
         `,
       });
+      console.log({ cwd });
       const proc = typecheck(cwd);
       expect(proc.stdout.toString()).toBe("");
       expect(proc.stderr.toString()).toBe("");
@@ -497,13 +498,17 @@ test.describe("typegen", () => {
         import { type RouteConfig, route } from "@react-router/dev/routes";
 
         export default [
+          route("optional-static/opt?", "routes/optional-static.tsx"),
           route("no-params", "routes/no-params.tsx"),
           route("required-param/:req", "routes/required-param.tsx"),
           route("optional-param/:opt?", "routes/optional-param.tsx"),
           route("/leading-and-trailing-slash/", "routes/leading-and-trailing-slash.tsx"),
           route("some-other-route", "routes/some-other-route.tsx"),
         ] satisfies RouteConfig;
       `,
+      "app/routes/optional-static.tsx": tsx`
+        export default function Component() {}
+      `,
       "app/routes/no-params.tsx": tsx`
         export default function Component() {}
       `,
@@ -522,13 +527,22 @@ test.describe("typegen", () => {
         // @ts-expect-error
         href("/does-not-exist")
 
+        href("/optional-static")
+        href("/optional-static/opt")
+        // @ts-expect-error
+        href("/optional-static/opt?")
+
         href("/no-params")
 
         // @ts-expect-error
         href("/required-param/:req")
         href("/required-param/:req", { req: "hello" })
 
+        href("/optional-param")
+        href("/optional-param/:opt", { opt: "hello" })
+        // @ts-expect-error
         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

@@ -7,6 +7,7 @@ import type { Context } from "./context";
 import * as Params from "./params";
 import * as Route from "./route";
 import type { RouteManifestEntry } from "../config/routes";
+import exp from "constants";
 
 export type VirtualFile = { filename: string; content: string };
 
@@ -59,7 +60,7 @@ export function generateRoutes(ctx: Context): Array<VirtualFile> {
   // precompute
   const fileToRoutes = new Map<string, Set<string>>();
   const lineages = new Map<string, Array<RouteManifestEntry>>();
-  const pages = new Set<string>();
+  const allPages = new Set<string>();
   const routeToPages = new Map<string, Set<string>>();
   for (const route of Object.values(ctx.config.routes)) {
     // fileToRoutes
@@ -75,9 +76,10 @@ export function generateRoutes(ctx: Context): Array<VirtualFile> {
     lineages.set(route.id, lineage);
 
     // pages
-    const page = Route.fullpath(lineage);
-    if (!page) continue;
-    pages.add(page);
+    const fullpath = Route.fullpath(lineage);
+    if (!fullpath) continue;
+    const pages = explodeOptionalSegments(fullpath);
+    pages.forEach((page) => allPages.add(page));
 
     // routePages
     lineage.forEach(({ id }) => {
@@ -86,7 +88,7 @@ export function generateRoutes(ctx: Context): Array<VirtualFile> {
         routePages = new Set<string>();
         routeToPages.set(id, routePages);
       }
-      routePages.add(page);
+      pages.forEach((page) => routePages.add(page));
     });
   }
 
@@ -107,7 +109,7 @@ export function generateRoutes(ctx: Context): Array<VirtualFile> {
         }
       ` +
       "\n\n" +
-      Babel.generate(pagesType(pages)).code +
+      Babel.generate(pagesType(allPages)).code +
       "\n\n" +
       Babel.generate(routeFilesType({ fileToRoutes, routeToPages })).code,
   };
@@ -346,3 +348,49 @@ 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;
+
+  // Optional path segments are denoted by a trailing `?`
+  let isOptional = first.endsWith("?");
+  // Compute the corresponding required segment: `foo?` -> `foo`
+  let required = first.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];
+  }
+
+  let restExploded = explodeOptionalSegments(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);
+  }
+
+  // for absolute paths, ensure `/` instead of empty segment
+  return result.map((exploded) =>
+    path.startsWith("/") && exploded === "" ? "/" : exploded
+  );
+}