Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sweet-chicken-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

fix: update matchPath to avoid false positives on dash-separated segments (#9300)
70 changes: 70 additions & 0 deletions packages/react-router-dom/__tests__/nav-link-active-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,76 @@ describe("NavLink", () => {

expect(anchor.props.className).not.toMatch("active");
});

it("does not match when <Link to> path is a subset of the active url", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/user-preferences"]}>
<Routes>
<Route
path="/"
element={
<div>
<NavLink to="user">Go to /user</NavLink>
<NavLink to="user-preferences">
Go to /user-preferences
</NavLink>
<Outlet />
</div>
}
>
<Route index element={<p>Index</p>} />
<Route path="user" element={<p>User</p>} />
<Route
path="user-preferences"
element={<p>User Preferences</p>}
/>
</Route>
</Routes>
</MemoryRouter>
);
});

let anchors = renderer.root.findAllByType("a");

expect(anchors.map((a) => a.props.className)).toEqual(["", "active"]);
});

it("does not match when active url is a subset of a <Route path> segment", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/user"]}>
<Routes>
<Route
path="/"
element={
<div>
<NavLink to="user">Go to /user</NavLink>
<NavLink to="user-preferences">
Go to /user-preferences
</NavLink>
<Outlet />
</div>
}
>
<Route index element={<p>Index</p>} />
<Route path="user" element={<p>User</p>} />
<Route
path="user-preferences"
element={<p>User Preferences</p>}
/>
</Route>
</Routes>
</MemoryRouter>
);
});

let anchors = renderer.root.findAllByType("a");

expect(anchors.map((a) => a.props.className)).toEqual(["active", ""]);
});
});

describe("when it matches just the beginning but not to the end", () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/react-router/__tests__/matchPath-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ describe("matchPath", () => {
it("fails to match a pathname where the segments do not match", () => {
expect(matchPath({ path: "/users", end: false }, "/")).toBeNull();
expect(matchPath({ path: "/users", end: false }, "/users2")).toBeNull();
expect(matchPath({ path: "/users", end: false }, "/users-2")).toBeNull();
expect(matchPath({ path: "/users", end: false }, "/users~2")).toBeNull();
expect(matchPath({ path: "/users", end: false }, "/users@2")).toBeNull();
expect(matchPath({ path: "/users", end: false }, "/users.2")).toBeNull();
expect(
matchPath({ path: "/users/mj", end: false }, "/users/mj2")
).toBeNull();
Expand Down
22 changes: 13 additions & 9 deletions packages/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,16 +634,20 @@ function compilePath(
path === "*" || path === "/*"
? "(.*)$" // Already matched the initial /, just match the rest
: "(?:\\/(.+)|\\/*)$"; // Don't include the / in params["*"]
} else if (end) {
// When matching to the end, ignore trailing slashes
regexpSource += "\\/*$";
} else if (path !== "" && path !== "/") {
// If our path is non-empty and contains anything beyond an initial slash,
// then we have _some_ form of path in our regex so we should expect to
// match only if we find the end of this path segment. Look for an optional
// non-captured trailing slash (to match a portion of the URL) or the end
// of the path (if we've matched to the end). We used to do this with a
// word boundary but that gives false positives on routes like
// /user-preferences since `-` counts as a word boundary.
regexpSource += "(?:(?=\\/|$))";
} else {
regexpSource += end
? "\\/*$" // When matching to the end, ignore trailing slashes
: // Otherwise, match a word boundary or a proceeding /. The word boundary restricts
// parent routes to matching only their own words and nothing more, e.g. parent
// route "/home" should not match "/home2".
// Additionally, allow paths starting with `.`, `-`, `~`, and url-encoded entities,
// but do not consume the character in the matched path so they can match against
// nested paths.
"(?:(?=[@.~-]|%[0-9A-F]{2})|\\b|\\/|$)";
// Nothing to match for "" or "/"
}

let matcher = new RegExp(regexpSource, caseSensitive ? undefined : "i");
Expand Down