fix: generatePath incorrectly applying parameters #9051 (#10078)

Author: Obi-Dann

.changeset/old-camels-accept.md

@@ -0,0 +1,6 @@
+---
+"@remix-run/router": patch
+"react-router": patch
+---
+
+Fix `generatePath` incorrectly applying parameters in some cases

contributors.yml

@@ -135,6 +135,7 @@
 - ms10596
 - ned-park
 - noisypigeon
+- Obi-Dann
 - omar-moquete
 - p13i
 - parched

packages/react-router/__tests__/generatePath-test.tsx

@@ -27,6 +27,20 @@ describe("generatePath", () => {
       );
       expect(generatePath("/*", {})).toBe("/");
     });
+    it("handles * in parameter values", () => {
+      expect(generatePath("/courses/:name", { name: "foo*" })).toBe(
+        "/courses/foo*"
+      );
+      expect(generatePath("/courses/:name", { name: "*foo" })).toBe(
+        "/courses/*foo"
+      );
+      expect(generatePath("/courses/:name", { name: "*f*oo*" })).toBe(
+        "/courses/*f*oo*"
+      );
+      expect(generatePath("/courses/:name", { name: "foo*", "*": "splat_should_not_be_added" })).toBe(
+        "/courses/foo*"
+      );
+    });
   });
 
   describe("with extraneous params", () => {

packages/react-router/__tests__/matchPath-test.tsx

@@ -300,6 +300,14 @@ describe("matchPath *", () => {
       pathnameBase: "/",
     });
   });
+
+  it("resolves params containing '*' correctly", () => {
+    expect(matchPath("/users/:name/*", "/users/foo*/splat")).toMatchObject({
+      params: { name: "foo*", "*": "splat" },
+      pathname: "/users/foo*/splat",
+      pathnameBase: "/users/foo*",
+    });
+  });
 });
 
 describe("matchPath warnings", () => {

packages/router/utils.ts

@@ -217,7 +217,7 @@ type _PathParam<Path extends string> =
  */
 type PathParam<Path extends string> =
   // check if path is just a wildcard
-  Path extends "*"
+  Path extends "*" | "/*"
     ? "*"
     : // look for wildcard at the end of the path
     Path extends `${infer Rest}/*`
@@ -621,7 +621,7 @@ export function generatePath<Path extends string>(
     [key in PathParam<Path>]: string | null;
   } = {} as any
 ): string {
-  let path = originalPath;
+  let path: string = originalPath;
   if (path.endsWith("*") && path !== "*" && !path.endsWith("/*")) {
     warning(
       false,
@@ -633,49 +633,46 @@ export function generatePath<Path extends string>(
     path = path.replace(/\*$/, "/*") as Path;
   }
 
-  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) => {
+  // ensure `/` is added at the beginning if the path is absolute
+  const prefix = path.startsWith("/") ? "/" : "";
+
+  const segments = path
+    .split(/\/+/)
+    .map((segment, index, array) => {
+      const isLastSegment = index === array.length - 1;
+
+      // only apply the splat if it's the last segment
+      if (isLastSegment && segment === "*") {
         const star = "*" as PathParam<Path>;
+        const starParam = params[star];
 
-        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 starParam;
+      }
+
+      const keyMatch = segment.match(/^:(\w+)(\??)$/);
+      if (keyMatch) {
+        const [, key, optional] = keyMatch;
+        let param = params[key as PathParam<Path>];
+
+        if (optional === "?") {
+          return param == null ? "" : param;
         }
 
-        // Apply the splat
-        return `${prefix}${params[star]}`;
-      })
-  );
+        if (param == null) {
+          invariant(false, `Missing ":${key}" param`);
+        }
+
+        return param;
+      }
+
+      // Remove any optional markers from optional static segments
+      return segment.replace(/\?$/g, "");
+    })
+    // Remove empty segments
+    .filter((segment) => !!segment);
+
+  return prefix + segments.join("/");
 }
 
 /**