Fix up generatePath when optional params are present (#9764)

Author: brophdawg11

Diff for: .changeset/happy-ladybugs-occur.md

@@ -0,0 +1,6 @@
+---
+"react-router": patch
+"@remix-run/router": patch
+---
+
+Fix `generatePath` when optional params are present

Diff for: packages/react-router/__tests__/generatePath-test.tsx

@@ -25,6 +25,7 @@ describe("generatePath", () => {
       expect(generatePath("*", { "*": "routing/grades" })).toBe(
         "routing/grades"
       );
+      expect(generatePath("/*", {})).toBe("/");
     });
   });
 
@@ -49,6 +50,60 @@ describe("generatePath", () => {
     });
   });
 
+  describe("with optional params", () => {
+    it("adds optional dynamic params where appropriate", () => {
+      let path = "/:one?/:two?/:three?";
+      expect(generatePath(path, { one: "uno" })).toBe("/uno");
+      expect(generatePath(path, { one: "uno", two: "dos" })).toBe("/uno/dos");
+      expect(
+        generatePath(path, {
+          one: "uno",
+          two: "dos",
+          three: "tres",
+        })
+      ).toBe("/uno/dos/tres");
+      expect(generatePath(path, { one: "uno", three: "tres" })).toBe(
+        "/uno/tres"
+      );
+      expect(generatePath(path, { two: "dos" })).toBe("/dos");
+      expect(generatePath(path, { two: "dos", three: "tres" })).toBe(
+        "/dos/tres"
+      );
+    });
+
+    it("strips optional aspects of static segments", () => {
+      expect(generatePath("/one?/two?/:three?", {})).toBe("/one/two");
+      expect(generatePath("/one?/two?/:three?", { three: "tres" })).toBe(
+        "/one/two/tres"
+      );
+    });
+
+    it("handles intermixed segments", () => {
+      let path = "/one?/:two?/three/:four/*";
+      expect(generatePath(path, { four: "cuatro" })).toBe("/one/three/cuatro");
+      expect(
+        generatePath(path, {
+          two: "dos",
+          four: "cuatro",
+        })
+      ).toBe("/one/dos/three/cuatro");
+      expect(
+        generatePath(path, {
+          two: "dos",
+          four: "cuatro",
+          "*": "splat",
+        })
+      ).toBe("/one/dos/three/cuatro/splat");
+      expect(
+        generatePath(path, {
+          two: "dos",
+          four: "cuatro",
+          "*": "splat/and/then/some",
+        })
+      ).toBe("/one/dos/three/cuatro/splat/and/then/some");
+    });
+  });
+
   it("throws only on on missing named parameters, but not missing splat params", () => {
     expect(() => generatePath(":foo")).toThrow();
     expect(() => generatePath("/:foo")).toThrow();

Diff for: packages/router/utils.ts

@@ -200,7 +200,9 @@ type _PathParam<Path extends string> =
     ? _PathParam<L> | _PathParam<R>
     : // find params after `:`
     Path extends `:${infer Param}`
-    ? Param
+    ? Param extends `${infer Optional}?`
+      ? Optional
+      : Param
     : // otherwise, there aren't any params present
       never;
 
@@ -616,7 +618,7 @@ function matchRouteBranch<
 export function generatePath<Path extends string>(
   originalPath: Path,
   params: {
-    [key in PathParam<Path>]: string;
+    [key in PathParam<Path>]: string | null;
   } = {} as any
 ): string {
   let path = originalPath;
@@ -631,27 +633,49 @@ export function generatePath<Path extends string>(
     path = path.replace(/\*$/, "/*") as Path;
   }
 
-  return path
-    .replace(/^:(\w+)/g, (_, key: PathParam<Path>) => {
-      invariant(params[key] != null, `Missing ":${key}" param`);
-      return params[key]!;
-    })
-    .replace(/\/:(\w+)/g, (_, key: PathParam<Path>) => {
-      invariant(params[key] != null, `Missing ":${key}" param`);
-      return `/${params[key]!}`;
-    })
-    .replace(/(\/?)\*/, (_, prefix, __, str) => {
-      const star = "*" as PathParam<Path>;
-
-      if (params[star] == null) {
-        // If no splat was provided, trim the trailing slash _unless_ it's
-        // the entire path
-        return str === "/*" ? "/" : "";
-      }
-
-      // Apply the splat
-      return `${prefix}${params[star]}`;
-    });
+  return (
+    path
+      .replace(
+        /^:(\w+)(\??)/g,
+        (_, key: PathParam<Path>, optional: string | undefined) => {
+          let param = params[key];
+          if (optional === "?") {
+            return param == null ? "" : param;
+          }
+          if (param == null) {
+            invariant(false, `Missing ":${key}" param`);
+          }
+          return param;
+        }
+      )
+      .replace(
+        /\/:(\w+)(\??)/g,
+        (_, key: PathParam<Path>, optional: string | undefined) => {
+          let param = params[key];
+          if (optional === "?") {
+            return param == null ? "" : `/${param}`;
+          }
+          if (param == null) {
+            invariant(false, `Missing ":${key}" param`);
+          }
+          return `/${param}`;
+        }
+      )
+      // Remove any optional markers from optional static segments
+      .replace(/\?/g, "")
+      .replace(/(\/?)\*/, (_, prefix, __, str) => {
+        const star = "*" as PathParam<Path>;
+
+        if (params[star] == null) {
+          // If no splat was provided, trim the trailing slash _unless_ it's
+          // the entire path
+          return str === "/*" ? "/" : "";
+        }
+
+        // Apply the splat
+        return `${prefix}${params[star]}`;
+      })
+  );
 }
 
 /**