Skip to content

Commit 813a935

Browse files
ngbrownpcattori
andauthored
Fix(react-router): Correct href() to processes routes that have an extension after the parameter or are a single optional parameter (#13797)
* Add href() test for period in page url * Update href() with new implementation * Add changeset * In href(), remove need for slice and ignore trailing slashes from input * refactor --------- Co-authored-by: Pedro Cattori <[email protected]>
1 parent 51f7600 commit 813a935

File tree

3 files changed

+43
-20
lines changed

3 files changed

+43
-20
lines changed

.changeset/slow-readers-thank.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
`href()` now correctly processes routes that have an extension after the parameter or are a single optional parameter.

packages/react-router/__tests__/href-test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,35 @@ describe("href", () => {
99
expect(href("/a/:b", { b: "hello", z: "ignored" })).toBe("/a/hello");
1010
expect(href("/a/:b?", { b: "hello", z: "ignored" })).toBe("/a/hello");
1111
expect(href("/a/:b?")).toBe("/a");
12+
expect(href("/:b?")).toBe("/");
13+
expect(href("/a/:e-z", { "e-z": "hello" })).toBe("/a/hello");
1214
});
1315

1416
it("works with repeated params", () => {
1517
expect(href("/a/:b?/:b/:b?/:b", { b: "hello" })).toBe(
1618
"/a/hello/hello/hello/hello",
1719
);
20+
expect(href("/a/:c?/:b/:c?/:b", { b: "hello" })).toBe("/a/hello/hello");
1821
});
1922

2023
it("works with splats", () => {
2124
expect(href("/a/*", { "*": "b/c" })).toBe("/a/b/c");
25+
expect(href("/a/*", {})).toBe("/a");
26+
});
27+
28+
it("works with malformed splats", () => {
29+
// this is how packages\react-router\lib\router\utils.ts: compilePath() will handle these routes.
30+
expect(href("/a/z*", { "*": "b/c" })).toBe("/a/z/b/c");
31+
expect(href("/a/z*", {})).toBe("/a/z");
2232
});
2333

2434
it("throws when required params are missing", () => {
2535
expect(() => href("/a/:b")).toThrow(
2636
`Path '/a/:b' requires param 'b' but it was not provided`,
2737
);
2838
});
39+
40+
it("works with periods", () => {
41+
expect(href("/a/:b.zip", { b: "hello" })).toBe("/a/hello.zip");
42+
});
2943
});

packages/react-router/lib/href.ts

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,30 @@ export function href<Path extends keyof Args>(
2727
...args: Args[Path]
2828
): string {
2929
let params = args[0];
30-
return path
31-
.split("/")
32-
.map((segment) => {
33-
if (segment === "*") {
34-
return params ? params["*"] : undefined;
35-
}
30+
let result = path
31+
.replace(/\/*\*?$/, "") // Ignore trailing / and /*, we'll handle it below
32+
.replace(
33+
/\/:([\w-]+)(\?)?/g, // same regex as in .\router\utils.ts: compilePath().
34+
(_: string, param: string, questionMark: string | undefined) => {
35+
const isRequired = questionMark === undefined;
36+
const value = params ? params[param] : undefined;
37+
if (isRequired && value === undefined) {
38+
throw new Error(
39+
`Path '${path}' requires param '${param}' but it was not provided`,
40+
);
41+
}
42+
return value === undefined ? "" : "/" + value;
43+
},
44+
);
3645

37-
const match = segment.match(/^:([\w-]+)(\?)?/);
38-
if (!match) return segment;
39-
const param = match[1];
40-
const value = params ? params[param] : undefined;
46+
if (path.endsWith("*")) {
47+
// treat trailing splat the same way as compilePath, and force it to be as if it were `/*`.
48+
// `react-router typegen` will not generate the params for a malformed splat, causing a type error, but we can still do the correct thing here.
49+
const value = params ? params["*"] : undefined;
50+
if (value !== undefined) {
51+
result += "/" + value;
52+
}
53+
}
4154

42-
const isRequired = match[2] === undefined;
43-
if (isRequired && value === undefined) {
44-
throw Error(
45-
`Path '${path}' requires param '${param}' but it was not provided`,
46-
);
47-
}
48-
return value;
49-
})
50-
.filter((segment) => segment !== undefined)
51-
.join("/");
55+
return result || "/";
5256
}

0 commit comments

Comments
 (0)