diff --git a/.changeset/sweet-chicken-suffer.md b/.changeset/sweet-chicken-suffer.md new file mode 100644 index 0000000000..7cbb3004da --- /dev/null +++ b/.changeset/sweet-chicken-suffer.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +fix: update matchPath to avoid false positives on dash-separated segments (#9300) diff --git a/packages/react-router-dom/__tests__/nav-link-active-test.tsx b/packages/react-router-dom/__tests__/nav-link-active-test.tsx index 4cc70728d3..34851b7d9d 100644 --- a/packages/react-router-dom/__tests__/nav-link-active-test.tsx +++ b/packages/react-router-dom/__tests__/nav-link-active-test.tsx @@ -218,6 +218,76 @@ describe("NavLink", () => { expect(anchor.props.className).not.toMatch("active"); }); + + it("does not match when path is a subset of the active url", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + Go to /user + + Go to /user-preferences + + + + } + > + Index

} /> + User

} /> + User Preferences

} + /> +
+
+
+ ); + }); + + 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 segment", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + Go to /user + + Go to /user-preferences + + + + } + > + Index

} /> + User

} /> + User Preferences

} + /> +
+
+
+ ); + }); + + 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", () => { diff --git a/packages/react-router/__tests__/matchPath-test.tsx b/packages/react-router/__tests__/matchPath-test.tsx index 795e889ce4..e36f73ed8a 100644 --- a/packages/react-router/__tests__/matchPath-test.tsx +++ b/packages/react-router/__tests__/matchPath-test.tsx @@ -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(); diff --git a/packages/router/utils.ts b/packages/router/utils.ts index cb3e89b85d..e60d2d901a 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -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");