diff --git a/.changeset/old-camels-accept.md b/.changeset/old-camels-accept.md new file mode 100644 index 0000000000..a809412ad3 --- /dev/null +++ b/.changeset/old-camels-accept.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix `generatePath` incorrectly applying parameters in some cases diff --git a/contributors.yml b/contributors.yml index adb8a0746c..bb1c8117fd 100644 --- a/contributors.yml +++ b/contributors.yml @@ -130,6 +130,7 @@ - ms10596 - ned-park - noisypigeon +- Obi-Dann - omar-moquete - p13i - parched diff --git a/packages/react-router/__tests__/generatePath-test.tsx b/packages/react-router/__tests__/generatePath-test.tsx index 26f036c3e0..babc21834c 100644 --- a/packages/react-router/__tests__/generatePath-test.tsx +++ b/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", () => { diff --git a/packages/react-router/__tests__/matchPath-test.tsx b/packages/react-router/__tests__/matchPath-test.tsx index e36f73ed8a..337734e0e4 100644 --- a/packages/react-router/__tests__/matchPath-test.tsx +++ b/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", () => { diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 172d52c36a..7f2d1acfde 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -621,7 +621,7 @@ export function generatePath( [key in PathParam]: 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 = path.replace(/\*$/, "/*") as Path; } - return ( - path - .replace( - /^:(\w+)(\??)/g, - (_, key: PathParam, 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, 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; + 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]; + + 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("/"); } /**