Skip to content

Commit

Permalink
Pick up dashes in dynamic parameter names (#11160)
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed Jan 9, 2024
1 parent c951e7c commit d103537
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/dynamic-param-dash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

Fix bug where dashes were not picked up in dynamic parameter names
6 changes: 6 additions & 0 deletions packages/react-router/__tests__/generatePath-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ describe("generatePath", () => {
// incorrect usage but worked in 6.3.0 so keep it to avoid the regression
expect(generatePath("/courses/*", { "*": 0 })).toBe("/courses/0");
});

it("handles dashes in dynamic params", () => {
expect(generatePath("/courses/:foo-bar", { "foo-bar": "baz" })).toBe(
"/courses/baz"
);
});
});

describe("with extraneous params", () => {
Expand Down
38 changes: 38 additions & 0 deletions packages/react-router/__tests__/path-matching-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,44 @@ describe("path matching", () => {

expect(pickPaths(routes, "/page")).toEqual(["page"]);
});

test("dynamic segments can contain dashes", () => {
let routes = [
{
path: ":foo-bar",
},
{
path: "foo-bar",
},
];

expect(matchRoutes(routes, "/foo-bar")).toMatchInlineSnapshot(`
[
{
"params": {},
"pathname": "/foo-bar",
"pathnameBase": "/foo-bar",
"route": {
"path": "foo-bar",
},
},
]
`);
expect(matchRoutes(routes, "/whatever")).toMatchInlineSnapshot(`
[
{
"params": {
"foo-bar": "whatever",
},
"pathname": "/whatever",
"pathnameBase": "/whatever",
"route": {
"path": ":foo-bar",
},
},
]
`);
});
});

describe("path matching with a basename", () => {
Expand Down
15 changes: 9 additions & 6 deletions packages/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ function rankRouteBranches(branches: RouteBranch[]): void {
);
}

const paramRe = /^:\w+$/;
const paramRe = /^:[\w-]+$/;
const dynamicSegmentValue = 3;
const indexRouteValue = 2;
const emptySegmentValue = 1;
Expand Down Expand Up @@ -822,7 +822,7 @@ export function generatePath<Path extends string>(
return stringify(params[star]);
}

const keyMatch = segment.match(/^:(\w+)(\??)$/);
const keyMatch = segment.match(/^:([\w-]+)(\??)$/);
if (keyMatch) {
const [, key, optional] = keyMatch;
let param = params[key as PathParam<Path>];
Expand Down Expand Up @@ -967,10 +967,13 @@ 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, isOptional) => {
params.push({ paramName, isOptional: isOptional != null });
return isOptional ? "/?([^\\/]+)?" : "/([^\\/]+)";
});
.replace(
/\/:([\w-]+)(\?)?/g,
(_: string, paramName: string, isOptional) => {
params.push({ paramName, isOptional: isOptional != null });
return isOptional ? "/?([^\\/]+)?" : "/([^\\/]+)";
}
);

if (path.endsWith("*")) {
params.push({ paramName: "*" });
Expand Down

0 comments on commit d103537

Please sign in to comment.