fix: ensure consistency in generatePath/compilePath for partial splats

.changeset/angry-pens-flow.md

@@ -0,0 +1,6 @@
+---
+"react-router": patch
+"@remix-run/router": patch
+---
+
+fix: Permit partial splat route segment matching (i.e., `/prefix-*`) (#9238)

.github/workflows/test.yml

@@ -40,6 +40,9 @@ jobs:
       - name: Build
         run: yarn build
 
+      - name: Lint
+        run: yarn lint
+
       - name: Test
         run: yarn test
 

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

@@ -298,38 +298,24 @@ describe("matchPath *", () => {
   });
 });
 
-describe("matchPath warnings", () => {
-  let consoleWarn: jest.SpyInstance<void, any>;
-  beforeEach(() => {
-    consoleWarn = jest.spyOn(console, "warn").mockImplementation();
-  });
-
-  afterEach(() => {
-    consoleWarn.mockRestore();
-  });
-
-  describe("when the pattern has a trailing *", () => {
-    it("issues a warning and matches the remaining portion of the pathname", () => {
-      expect(matchPath("/files*", "/files/mj.jpg")).toMatchObject({
-        params: { "*": "mj.jpg" },
-        pathname: "/files/mj.jpg",
-        pathnameBase: "/files",
-      });
-      expect(consoleWarn).toHaveBeenCalledTimes(1);
+describe("when the pattern has a trailing *", () => {
+  it("matches the remaining portion of the pathname", () => {
+    expect(matchPath("/files*", "/files/mj.jpg")).toMatchObject({
+      params: { "*": "/mj.jpg" },
+      pathname: "/files/mj.jpg",
+      pathnameBase: "/files",
+    });
 
-      expect(matchPath("/files*", "/files/")).toMatchObject({
-        params: { "*": "" },
-        pathname: "/files/",
-        pathnameBase: "/files",
-      });
-      expect(consoleWarn).toHaveBeenCalledTimes(2);
+    expect(matchPath("/files*", "/files/")).toMatchObject({
+      params: { "*": "/" },
+      pathname: "/files/",
+      pathnameBase: "/files",
+    });
 
-      expect(matchPath("/files*", "/files")).toMatchObject({
-        params: { "*": "" },
-        pathname: "/files",
-        pathnameBase: "/files",
-      });
-      expect(consoleWarn).toHaveBeenCalledTimes(3);
+    expect(matchPath("/files*", "/files")).toMatchObject({
+      params: { "*": "" },
+      pathname: "/files",
+      pathnameBase: "/files",
     });
   });
 });

packages/react-router/__tests__/path-matching-test.tsx

@@ -1,11 +1,19 @@
 import type { RouteObject } from "react-router";
 import { matchRoutes } from "react-router";
 
-function pickPaths(routes: RouteObject[], pathname: string): string[] | null {
+function pickPaths(routes: RouteObject[], pathname: string) {
   let matches = matchRoutes(routes, pathname);
   return matches && matches.map((match) => match.route.path || "");
 }
 
+function pickPathsAndParams(routes: RouteObject[], pathname: string) {
+  let matches = matchRoutes(routes, pathname);
+  return (
+    matches &&
+    matches.map((match) => ({ path: match.route.path, params: match.params }))
+  );
+}
+
 describe("path matching", () => {
   test("root vs. dynamic", () => {
     let routes = [{ path: "/" }, { path: ":id" }];
@@ -267,4 +275,123 @@ describe("path matching with splats", () => {
       pathnameBase: "/courses",
     });
   });
+
+  test("supports partial path matching with named parameters", () => {
+    let routes = [{ path: "/prefix:id" }];
+    expect(matchRoutes(routes, "/prefixabc")).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "params": Object {
+            "id": "abc",
+          },
+          "pathname": "/prefixabc",
+          "pathnameBase": "/prefixabc",
+          "route": Object {
+            "path": "/prefix:id",
+          },
+        },
+      ]
+    `);
+    expect(matchRoutes(routes, "/prefix/abc")).toMatchInlineSnapshot(`null`);
+  });
+
+  test("supports partial path matching with splat parameters", () => {
+    let routes = [{ path: "/prefix*" }];
+    expect(matchRoutes(routes, "/prefix/abc")).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "params": Object {
+            "*": "/abc",
+          },
+          "pathname": "/prefix/abc",
+          "pathnameBase": "/prefix",
+          "route": Object {
+            "path": "/prefix*",
+          },
+        },
+      ]
+    `);
+    expect(matchRoutes(routes, "/prefixabc")).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "params": Object {
+            "*": "abc",
+          },
+          "pathname": "/prefixabc",
+          "pathnameBase": "/prefix",
+          "route": Object {
+            "path": "/prefix*",
+          },
+        },
+      ]
+    `);
+  });
+});
+
+describe("route scoring", () => {
+  test("splat routes versus dynamic routes", () => {
+    let routes = [
+      { path: "nested/prefix-:param/static/prefix-*" }, // Score 43
+      { path: "nested/prefix-:param/static/*" }, // Score 33
+      { path: "nested/prefix-:param/static" }, // Score 34
+      { path: "nested/prefix-:param/*" }, // Score 22
+      { path: "nested/:param/static" }, // Score 28
+      { path: "nested/static" }, // Score 24
+      { path: "nested/prefix-:param" }, // Score 23
+      { path: "nested/prefix-*" }, // Score 22
+      { path: "nested/:param" }, // Score 17
+      { path: "static" }, // Score 13
+      { path: "prefix-:param" }, // Score 12
+      { path: "prefix-*" }, // Score 11
+      { path: ":param" }, // Score 6
+      { path: "*" }, // Score 1
+    ];
+
+    // Matches are defined as [A, B, C], as in:
+    //   "URL A should match path B with params C"
+    let matches: Array<[string, string, Record<string, string>]> = [
+      [
+        "/nested/prefix-foo/static/prefix-bar/baz",
+        "nested/prefix-:param/static/prefix-*",
+        { param: "foo", "*": "bar/baz" },
+      ],
+      [
+        "/nested/prefix-foo/static/bar/baz",
+        "nested/prefix-:param/static/*",
+        { param: "foo", "*": "bar/baz" },
+      ],
+      [
+        "/nested/prefix-foo/static/bar",
+        "nested/prefix-:param/static/*",
+        { param: "foo", "*": "bar" },
+      ],
+      [
+        "/nested/prefix-foo/static",
+        "nested/prefix-:param/static",
+        { param: "foo" },
+      ],
+      [
+        "/nested/prefix-foo/bar",
+        "nested/prefix-:param/*",
+        { param: "foo", "*": "bar" },
+      ],
+      ["/nested/foo/static", "nested/:param/static", { param: "foo" }],
+      ["/nested/static", "nested/static", {}],
+      ["/nested/prefix-foo", "nested/prefix-:param", { param: "foo" }],
+      ["/nested/foo", "nested/:param", { param: "foo" }],
+      ["/static", "static", {}],
+      ["/prefix-foo", "prefix-:param", { param: "foo" }],
+      ["/prefix-foo/bar", "prefix-*", { "*": "foo/bar" }],
+      ["/foo", ":param", { param: "foo" }],
+      ["/foo/bar/baz", "*", { "*": "foo/bar/baz" }],
+    ];
+
+    // Ensure order agnostic by testing route definitions forward + backwards
+    [...matches, ...matches.reverse()].forEach(([url, path, params]) =>
+      expect({
+        url,
+        matches: pickPathsAndParams(routes, url),
+      }).toEqual({ url, matches: [{ path, params }] })
+    );
+  });
 });

packages/router/utils.ts

@@ -374,36 +374,47 @@ function rankRouteBranches(branches: RouteBranch[]): void {
 }
 
 const paramRe = /^:\w+$/;
-const dynamicSegmentValue = 3;
-const indexRouteValue = 2;
+const partialParamRe = /:\w+$/;
+const splatRe = /^\*$/;
+const partialSplatRe = /\*$/;
+
+const staticSegmentValue = 10; /*         /static         */
+const partialDynamicSegmentValue = 9; /*  /prefix-:param  */
+const partialSplatValue = 8; /*           /prefix-*       */
+const dynamicSegmentValue = 3; /*         /:param         */
+const indexRouteValue = 2; /*             /               */
 const emptySegmentValue = 1;
-const staticSegmentValue = 10;
-const splatPenalty = -2;
-const isSplat = (s: string) => s === "*";
+const splatPenalty = -2; /*               /*              */
 
 function computeScore(path: string, index: boolean | undefined): number {
   let segments = path.split("/");
-  let initialScore = segments.length;
-  if (segments.some(isSplat)) {
-    initialScore += splatPenalty;
-  }
+  let score = segments.length;
 
   if (index) {
-    initialScore += indexRouteValue;
+    score += indexRouteValue;
   }
 
-  return segments
-    .filter((s) => !isSplat(s))
-    .reduce(
-      (score, segment) =>
-        score +
-        (paramRe.test(segment)
-          ? dynamicSegmentValue
-          : segment === ""
-          ? emptySegmentValue
-          : staticSegmentValue),
-      initialScore
-    );
+  // Penalties apply globally, so only subtract them once
+  let splatPenaltyApplied = false;
+
+  segments.forEach((segment) => {
+    if (splatRe.test(segment)) {
+      score += splatPenaltyApplied ? 0 : splatPenalty;
+      splatPenaltyApplied = true;
+    } else if (partialSplatRe.test(segment)) {
+      score += partialSplatValue;
+    } else if (paramRe.test(segment)) {
+      score += dynamicSegmentValue;
+    } else if (partialParamRe.test(segment)) {
+      score += partialDynamicSegmentValue;
+    } else if (segment === "") {
+      score += emptySegmentValue;
+    } else {
+      score += staticSegmentValue;
+    }
+  });
+
+  return score;
 }
 
 function compareIndexes(a: number[], b: number[]): number {
@@ -608,14 +619,6 @@ function compilePath(
   caseSensitive = false,
   end = true
 ): [RegExp, string[]] {
-  warning(
-    path === "*" || !path.endsWith("*") || path.endsWith("/*"),
-    `Route path "${path}" will be treated as if it were ` +
-      `"${path.replace(/\*$/, "/*")}" because the \`*\` character must ` +
-      `always follow a \`/\` in the pattern. To get rid of this warning, ` +
-      `please change the route path to "${path.replace(/\*$/, "/*")}".`
-  );
-
   let paramNames: string[] = [];
   let regexpSource =
     "^" +
@@ -628,12 +631,17 @@ function compilePath(
         return "([^\\/]+)";
       });
 
-  if (path.endsWith("*")) {
+  if (path === "*" || path.endsWith("/*")) {
+    // Full splat segment
     paramNames.push("*");
     regexpSource +=
       path === "*" || path === "/*"
         ? "(.*)$" // Already matched the initial /, just match the rest
         : "(?:\\/(.+)|\\/*)$"; // Don't include the / in params["*"]
+  } else if (path.endsWith("*")) {
+    // Partial splat segment
+    paramNames.push("*");
+    regexpSource += "(.*)$";
   } else {
     regexpSource += end
       ? "\\/*$" // When matching to the end, ignore trailing slashes