Skip to content

Commit

Permalink
Fix relative=path issue (remix-run#11006)
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 authored and jonathanpruvost committed Nov 28, 2023
1 parent 087a4aa commit 67c3ca8
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 9 deletions.
23 changes: 23 additions & 0 deletions .changeset/fix-relative-path.md
Original file line number Diff line number Diff line change
@@ -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
<Route path="/a">
<Route path="/b" element={<Component />}>
<Route path="/c" />
</Route>
</Route>;

function Component() {
return (
<>
{/* This is now correctly relative to /a/b, not /a/b/c */}
<Link to=".." relative="path" />
<Outlet />
</>
);
}
```
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
21 changes: 21 additions & 0 deletions packages/router/__tests__/path-resolution-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down
6 changes: 2 additions & 4 deletions packages/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 23 additions & 4 deletions packages/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href> 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 <a href> works and a major reason we call this a
// "to" value instead of a "href".
while (toSegments[0] === "..") {
toSegments.shift();
routePathnameIndex -= 1;
Expand Down

0 comments on commit 67c3ca8

Please sign in to comment.