From 63960a8df46636f3d68236e026f1489f8de7a4ca Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 13 Nov 2023 18:08:32 -0500 Subject: [PATCH] Fix relative=path issue (#11006) --- .changeset/fix-relative-path.md | 23 ++++++++++++++++ package.json | 2 +- .../router/__tests__/path-resolution-test.ts | 21 +++++++++++++++ packages/router/router.ts | 6 ++--- packages/router/utils.ts | 27 ++++++++++++++++--- 5 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 .changeset/fix-relative-path.md diff --git a/.changeset/fix-relative-path.md b/.changeset/fix-relative-path.md new file mode 100644 index 0000000000..3da1d1cc13 --- /dev/null +++ b/.changeset/fix-relative-path.md @@ -0,0 +1,23 @@ +--- +"@remix-run/router": patch +--- + +Fix `relative="path"` bug where relative path calculations started from the full location pathname, instead of from the current contextual route pathname. + +```jsx + + }> + + +; + +function Component() { + return ( + <> + {/* This is now correctly relative to /a/b, not /a/b/c */} + + + + ); +} +``` diff --git a/package.json b/package.json index 0709215874..ce06a4af7e 100644 --- a/package.json +++ b/package.json @@ -110,7 +110,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "49.2 kB" + "none": "49.3 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "13.9 kB" diff --git a/packages/router/__tests__/path-resolution-test.ts b/packages/router/__tests__/path-resolution-test.ts index e1c6089078..6025fd2ea9 100644 --- a/packages/router/__tests__/path-resolution-test.ts +++ b/packages/router/__tests__/path-resolution-test.ts @@ -451,6 +451,27 @@ describe("path resolution", () => { expect(router.state.location.pathname).toBe("/a/b/c/d"); router.navigate("/a/b/c/d/e/f"); + // Navigating with relative:path from mid-route-hierarchy + router.navigate("..", { relative: "path", fromRouteId: "f" }); + expect(router.state.location.pathname).toBe("/a/b/c/d/e"); + router.navigate("/a/b/c/d/e/f"); + + router.navigate("../..", { relative: "path", fromRouteId: "de" }); + expect(router.state.location.pathname).toBe("/a/b/c"); + router.navigate("/a/b/c/d/e/f"); + + router.navigate("../..", { relative: "path", fromRouteId: "bc" }); + expect(router.state.location.pathname).toBe("/a"); + router.navigate("/a/b/c/d/e/f"); + + // Go up farther than # of URL segments + router.navigate("../../../../../../../../..", { + relative: "path", + fromRouteId: "f", + }); + expect(router.state.location.pathname).toBe("/"); + router.navigate("/a/b/c/d/e/f"); + router.dispose(); }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 7422e15492..c14c4814da 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -3319,11 +3319,9 @@ function normalizeTo( ) { let contextualMatches: AgnosticDataRouteMatch[]; let activeRouteMatch: AgnosticDataRouteMatch | undefined; - if (fromRouteId != null && relative !== "path") { + if (fromRouteId) { // Grab matches up to the calling route so our route-relative logic is - // relative to the correct source route. When using relative:path, - // fromRouteId is ignored since that is always relative to the current - // location path + // relative to the correct source route contextualMatches = []; for (let match of matches) { contextualMatches.push(match); diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 51f9759125..f1b879c593 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1188,17 +1188,36 @@ export function resolveTo( // `to` values that do not provide a pathname. `to` can simply be a search or // hash string, in which case we should assume that the navigation is relative // to the current location's pathname and *not* the route pathname. - if (isPathRelative || toPathname == null) { + if (toPathname == null) { from = locationPathname; + } else if (isPathRelative) { + let fromSegments = routePathnames[routePathnames.length - 1] + .replace(/^\//, "") + .split("/"); + + if (toPathname.startsWith("..")) { + let toSegments = toPathname.split("/"); + + // With relative="path", each leading .. segment means "go up one URL segment" + while (toSegments[0] === "..") { + toSegments.shift(); + fromSegments.pop(); + } + + to.pathname = toSegments.join("/"); + } + + from = "/" + fromSegments.join("/"); } else { let routePathnameIndex = routePathnames.length - 1; if (toPathname.startsWith("..")) { let toSegments = toPathname.split("/"); - // Each leading .. segment means "go up one route" instead of "go up one - // URL segment". This is a key difference from how works and a - // major reason we call this a "to" value instead of a "href". + // With relative="route" (the default), each leading .. segment means + // "go up one route" instead of "go up one URL segment". This is a key + // difference from how works and a major reason we call this a + // "to" value instead of a "href". while (toSegments[0] === "..") { toSegments.shift(); routePathnameIndex -= 1;