diff --git a/.changeset/happy-balloons-buy.md b/.changeset/happy-balloons-buy.md new file mode 100644 index 0000000000..1c612530da --- /dev/null +++ b/.changeset/happy-balloons-buy.md @@ -0,0 +1,26 @@ +--- +"react-router": patch +"@remix-run/router": patch +--- + +Stop incorrectly matching on partial named parameters, i.e. ``, to align with how splat parameters work. If you were previously relying on this behavior then it's recommended to extract the static portion of the path at the `useParams` call site: + +```jsx +// Old behavior at URL /prefix-123 + }> + +function Comp() { + let params = useParams(); // { id: '123' } + let id = params.id; // "123" + ... +} + +// New behavior at URL /prefix-123 + }> + +function Comp() { + let params = useParams(); // { id: 'prefix-123' } + let id = params.id.replace(/^prefix-/, ''); // "123" + ... +} +``` diff --git a/package.json b/package.json index d7b6afc2db..9838ae7e99 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "35.5 kB" + "none": "36 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "12.5 kB" diff --git a/packages/react-router/__tests__/generatePath-test.tsx b/packages/react-router/__tests__/generatePath-test.tsx index 9276055aa5..105a2ed63f 100644 --- a/packages/react-router/__tests__/generatePath-test.tsx +++ b/packages/react-router/__tests__/generatePath-test.tsx @@ -13,6 +13,12 @@ describe("generatePath", () => { expect(generatePath("/courses/:id", { id: "routing" })).toBe( "/courses/routing" ); + expect( + generatePath("/courses/:id/student/:studentId", { + id: "routing", + studentId: "matt", + }) + ).toBe("/courses/routing/student/matt"); expect(generatePath("/courses/*", { "*": "routing/grades" })).toBe( "/courses/routing/grades" ); @@ -51,6 +57,8 @@ describe("generatePath", () => { }); it("only interpolates and does not add slashes", () => { + let consoleWarn = jest.spyOn(console, "warn").mockImplementation(() => {}); + expect(generatePath("*")).toBe(""); expect(generatePath("/*")).toBe("/"); @@ -63,10 +71,32 @@ describe("generatePath", () => { expect(generatePath("*", { "*": "bar" })).toBe("bar"); expect(generatePath("/*", { "*": "bar" })).toBe("/bar"); - expect(generatePath("foo:bar", { bar: "baz" })).toBe("foobaz"); - expect(generatePath("/foo:bar", { bar: "baz" })).toBe("/foobaz"); + // No support for partial dynamic params + expect(generatePath("foo:bar", { bar: "baz" })).toBe("foo:bar"); + expect(generatePath("/foo:bar", { bar: "baz" })).toBe("/foo:bar"); + + // Partial splats are treated as independent path segments + expect(generatePath("foo*", { "*": "bar" })).toBe("foo/bar"); + expect(generatePath("/foo*", { "*": "bar" })).toBe("/foo/bar"); + + // Ensure we warn on partial splat usages + expect(consoleWarn.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "Route path \\"foo*\\" will be treated as if it were \\"foo/*\\" because the \`*\` character must always follow a \`/\` in the pattern. To get rid of this warning, please change the route path to \\"foo/*\\".", + ], + Array [ + "Route path \\"/foo*\\" will be treated as if it were \\"/foo/*\\" because the \`*\` character must always follow a \`/\` in the pattern. To get rid of this warning, please change the route path to \\"/foo/*\\".", + ], + Array [ + "Route path \\"foo*\\" will be treated as if it were \\"foo/*\\" because the \`*\` character must always follow a \`/\` in the pattern. To get rid of this warning, please change the route path to \\"foo/*\\".", + ], + Array [ + "Route path \\"/foo*\\" will be treated as if it were \\"/foo/*\\" because the \`*\` character must always follow a \`/\` in the pattern. To get rid of this warning, please change the route path to \\"/foo/*\\".", + ], + ] + `); - expect(generatePath("foo*", { "*": "bar" })).toBe("foobar"); - expect(generatePath("/foo*", { "*": "bar" })).toBe("/foobar"); + consoleWarn.mockRestore(); }); }); diff --git a/packages/react-router/__tests__/path-matching-test.tsx b/packages/react-router/__tests__/path-matching-test.tsx index 4f4a4ad32e..f45377c8c8 100644 --- a/packages/react-router/__tests__/path-matching-test.tsx +++ b/packages/react-router/__tests__/path-matching-test.tsx @@ -275,6 +275,58 @@ describe("path matching with splats", () => { pathnameBase: "/courses", }); }); + + test("does not support partial path matching with named parameters", () => { + let routes = [{ path: "/prefix:id" }]; + expect(matchRoutes(routes, "/prefix:id")).toMatchInlineSnapshot(` + Array [ + Object { + "params": Object {}, + "pathname": "/prefix:id", + "pathnameBase": "/prefix:id", + "route": Object { + "path": "/prefix:id", + }, + }, + ] + `); + expect(matchRoutes(routes, "/prefixabc")).toEqual(null); + expect(matchRoutes(routes, "/prefix/abc")).toEqual(null); + }); + + test("does not support partial path matching with splat parameters", () => { + let consoleWarn = jest.spyOn(console, "warn").mockImplementation(() => {}); + 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(`null`); + + // Should warn on each invocation of matchRoutes + expect(consoleWarn.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "Route path \\"/prefix*\\" will be treated as if it were \\"/prefix/*\\" because the \`*\` character must always follow a \`/\` in the pattern. To get rid of this warning, please change the route path to \\"/prefix/*\\".", + ], + Array [ + "Route path \\"/prefix*\\" will be treated as if it were \\"/prefix/*\\" because the \`*\` character must always follow a \`/\` in the pattern. To get rid of this warning, please change the route path to \\"/prefix/*\\".", + ], + ] + `); + + consoleWarn.mockRestore(); + }); }); describe("path matchine with optional segments", () => { diff --git a/packages/react-router/__tests__/useRoutes-test.tsx b/packages/react-router/__tests__/useRoutes-test.tsx index 18e200f1fa..91de38a632 100644 --- a/packages/react-router/__tests__/useRoutes-test.tsx +++ b/packages/react-router/__tests__/useRoutes-test.tsx @@ -76,6 +76,8 @@ describe("useRoutes", () => { }); it("returns null when no route matches", () => { + let spy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let routes = [{ path: "one", element:

one

}]; const NullRenderer = (props: { routes: RouteObject[] }) => { @@ -97,9 +99,13 @@ describe("useRoutes", () => { is null `); + + spy.mockRestore(); }); it("returns null when no route matches and a `location` prop is passed", () => { + let spy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let routes = [{ path: "one", element:

one

}]; const NullRenderer = (props: { @@ -114,7 +120,10 @@ describe("useRoutes", () => { TestRenderer.act(() => { renderer = TestRenderer.create( - + ); }); @@ -124,6 +133,8 @@ describe("useRoutes", () => { is null `); + + spy.mockRestore(); }); describe("warns", () => { diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 72699b4ee1..eeb33573e5 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -197,7 +197,7 @@ type _PathParam = Path extends `${infer L}/${infer R}` ? _PathParam | _PathParam : // find params after `:` - Path extends `${string}:${infer Param}` + Path extends `:${infer Param}` ? Param : // otherwise, there aren't any params present never; @@ -570,16 +570,32 @@ function matchRouteBranch< * @see https://reactrouter.com/docs/en/v6/utils/generate-path */ export function generatePath( - path: Path, + originalPath: Path, params: { [key in PathParam]: string; } = {} as any ): string { + let path = originalPath; + if (path.endsWith("*") && path !== "*" && !path.endsWith("/*")) { + warning( + false, + `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(/\*$/, "/*")}".` + ); + path = path.replace(/\*$/, "/*") as Path; + } + return path - .replace(/:(\w+)/g, (_, key: PathParam) => { + .replace(/^:(\w+)/g, (_, key: PathParam) => { invariant(params[key] != null, `Missing ":${key}" param`); return params[key]!; }) + .replace(/\/:(\w+)/g, (_, key: PathParam) => { + invariant(params[key] != null, `Missing ":${key}" param`); + return `/${params[key]!}`; + }) .replace(/(\/?)\*/, (_, prefix, __, str) => { const star = "*" as PathParam; @@ -718,9 +734,9 @@ function compilePath( .replace(/\/*\*?$/, "") // Ignore trailing / and /*, we'll handle it below .replace(/^\/*/, "/") // Make sure it has a leading / .replace(/[\\.*+^$?{}|()[\]]/g, "\\$&") // Escape special regex chars - .replace(/:(\w+)/g, (_: string, paramName: string) => { + .replace(/\/:(\w+)/g, (_: string, paramName: string) => { paramNames.push(paramName); - return "([^\\/]+)"; + return "/([^\\/]+)"; }); if (path.endsWith("*")) {