-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix inadvertent support for partial dynamic parameters #9506
Changes from all commits
c282a5d
4f3d632
a5c5059
4bbeb90
d2e941e
c2c250c
114e26f
5083d68
f136255
db7a8e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--- | ||
"react-router": patch | ||
"@remix-run/router": patch | ||
--- | ||
|
||
Stop incorrectly matching on partial named parameters, i.e. `<Route path="prefix-:param">`, 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 | ||
<Route path="prefix-:id" element={<Comp /> }> | ||
|
||
function Comp() { | ||
let params = useParams(); // { id: '123' } | ||
let id = params.id; // "123" | ||
... | ||
} | ||
|
||
// New behavior at URL /prefix-123 | ||
<Route path=":id" element={<Comp /> }> | ||
|
||
function Comp() { | ||
let params = useParams(); // { id: 'prefix-123' } | ||
let id = params.id.replace(/^prefix-/, ''); // "123" | ||
... | ||
} | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,7 +197,7 @@ type _PathParam<Path extends string> = | |
Path extends `${infer L}/${infer R}` | ||
? _PathParam<L> | _PathParam<R> | ||
: // 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 extends string>( | ||
path: Path, | ||
originalPath: Path, | ||
params: { | ||
[key in PathParam<Path>]: 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same logic as we use in |
||
} | ||
|
||
return path | ||
.replace(/:(\w+)/g, (_, key: PathParam<Path>) => { | ||
.replace(/^:(\w+)/g, (_, key: PathParam<Path>) => { | ||
invariant(params[key] != null, `Missing ":${key}" param`); | ||
return params[key]!; | ||
}) | ||
.replace(/\/:(\w+)/g, (_, key: PathParam<Path>) => { | ||
invariant(params[key] != null, `Missing ":${key}" param`); | ||
return `/${params[key]!}`; | ||
}) | ||
Comment on lines
-579
to
+598
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
.replace(/(\/?)\*/, (_, prefix, __, str) => { | ||
const star = "*" as PathParam<Path>; | ||
|
||
|
@@ -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 "/([^\\/]+)"; | ||
Comment on lines
-721
to
+739
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only match params that are at the start of a URL segment. We do not need to handle |
||
}); | ||
|
||
if (path.endsWith("*")) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lack of search/hash in this test case was causing weird
console.warn
of/threeundefinedundefined