diff --git a/.changeset/itchy-items-hang.md b/.changeset/itchy-items-hang.md new file mode 100644 index 0000000000..ce2e6f5dfe --- /dev/null +++ b/.changeset/itchy-items-hang.md @@ -0,0 +1,5 @@ +--- +"react-router-dom": patch +--- + +Fix `NavLink` `active` logic when `to` location has a trailing slash diff --git a/contributors.yml b/contributors.yml index 14ba24c3e3..cb41d0b014 100644 --- a/contributors.yml +++ b/contributors.yml @@ -31,6 +31,7 @@ - bbrowning918 - BDomzalski - bhbs +- bilalk711 - bobziroll - BrianT1414 - brockross diff --git a/docs/components/nav-link.md b/docs/components/nav-link.md index e67c0079e5..b81514c4d7 100644 --- a/docs/components/nav-link.md +++ b/docs/components/nav-link.md @@ -93,12 +93,14 @@ You can pass a render prop as children to customize the content of the `` | `/tasks` | true | -| `` | `/tasks/123` | true | -| `` | `/tasks` | true | -| `` | `/tasks/123` | false | +| Link | Current URL | isActive | +| ------------------------------ | ------------ | -------- | +| `` | `/tasks` | true | +| `` | `/tasks/123` | true | +| `` | `/tasks` | true | +| `` | `/tasks/123` | false | +| `` | `/tasks` | false | +| `` | `/tasks/` | true | **A note on links to the root route** 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 8e597acc48..f6ba739be1 100644 --- a/packages/react-router-dom/__tests__/nav-link-active-test.tsx +++ b/packages/react-router-dom/__tests__/nav-link-active-test.tsx @@ -106,6 +106,26 @@ describe("NavLink", () => { expect(anchor.props.className).toMatch("active"); }); + it("when the current URL has a trailing slash", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + Home} + /> + + + ); + }); + + let anchor = renderer.root.findByType("a"); + + expect(anchor.props.className).toMatch("active"); + }); + it("applies its className correctly when provided as a function", () => { let renderer: TestRenderer.ReactTestRenderer; TestRenderer.act(() => { @@ -433,6 +453,32 @@ describe("NavLink", () => { expect(anchor.props.className).toMatch("active"); }); + it("In case of trailing slash at the end of link", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + Home + + + } + > + Child} /> + + + + ); + }); + + let anchor = renderer.root.findByType("a"); + expect(anchor.props.className).toMatch("active"); + }); + describe("when end=true", () => { it("does not apply the default 'active' className to the underlying ", () => { let renderer: TestRenderer.ReactTestRenderer; @@ -462,6 +508,68 @@ describe("NavLink", () => { expect(anchor.props.className).not.toMatch("active"); }); + + it("Handles trailing slashes accordingly when the URL does not have a trailing slash", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + + Home + + + Home + + + + } + > + Child} /> + + + + ); + }); + + let anchors = renderer.root.findAllByType("a"); + expect(anchors.map((a) => a.props.className)).toEqual(["active", ""]); + }); + + it("Handles trailing slashes accordingly when the URL has a trailing slash", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + + Home + + + Home + + + + } + > + Child} /> + + + + ); + }); + + let anchors = renderer.root.findAllByType("a"); + expect(anchors.map((a) => a.props.className)).toEqual(["", "active"]); + }); }); }); diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 9735b2bda7..d5b3933459 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -964,11 +964,20 @@ export const NavLink = React.forwardRef( toPathname = toPathname.toLowerCase(); } + // If the `to` has a trailing slash, look at that exact spot. Otherwise, + // we're looking for a slash _after_ what's in `to`. For example: + // + // and + // both want to look for a / at index 6 to match URL `/users/matt` + const endSlashPosition = + toPathname !== "/" && toPathname.endsWith("/") + ? toPathname.length - 1 + : toPathname.length; let isActive = locationPathname === toPathname || (!end && locationPathname.startsWith(toPathname) && - locationPathname.charAt(toPathname.length) === "/"); + locationPathname.charAt(endSlashPosition) === "/"); let isPending = nextLocationPathname != null &&