Skip to content

Commit

Permalink
Fix: useResolvedPath and useNavigate use different logic (#8985)
Browse files Browse the repository at this point in the history
* share matches logic between useResolvedPath and useNavigate

* sign CLA

* Fix 2 tests and add some comments

* add changeset

* update test descriptions for clarity

* change back the ../../.. to ../..

Co-authored-by: Matt Brophy <[email protected]>
  • Loading branch information
david-crespo and brophdawg11 authored Jun 21, 2022
1 parent 2f9b155 commit e6b6811
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/silver-planes-relate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

fix: Additional logic fixed for relative navigation from index/pathless layout routes (#8985)
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- chrisngobanh
- christopherchudzicki
- cvbuelow
- david-crespo
- edwin177
- elylucas
- emzoumpo
Expand Down
8 changes: 4 additions & 4 deletions packages/react-router-dom/__tests__/link-href-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe("<Link> href", () => {
expect(renderer.root.findByType("a").props.href).toEqual("/inbox");
});

test('<Link to=".."> resolves relative to the parent route', () => {
test('<Link to=".."> resolves relative to the parent route (ignoring the index route)', () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
Expand All @@ -193,7 +193,7 @@ describe("<Link> href", () => {
);
});

expect(renderer.root.findByType("a").props.href).toEqual("/inbox");
expect(renderer.root.findByType("a").props.href).toEqual("/");
});

test('<Link to=".."> with more .. segments than parent routes resolves to the root URL', () => {
Expand Down Expand Up @@ -262,7 +262,7 @@ describe("<Link> href", () => {
expect(renderer.root.findByType("a").props.href).toEqual("/inbox");
});

test('<Link to=".."> resolves relative to the parent route', () => {
test('<Link to=".."> resolves relative to the parent route (ignoring the pathless route)', () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
Expand All @@ -278,7 +278,7 @@ describe("<Link> href", () => {
);
});

expect(renderer.root.findByType("a").props.href).toEqual("/inbox");
expect(renderer.root.findByType("a").props.href).toEqual("/");
});

test('<Link to=".."> with more .. segments than parent routes resolves to the root URL', () => {
Expand Down
42 changes: 32 additions & 10 deletions packages/react-router/lib/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,36 @@ export interface NavigateFunction {
(delta: number): void;
}

/**
* When processing relative navigation we want to ignore ancestor routes that
* do not contribute to the path, such that index/pathless layout routes don't
* interfere.
*
* For example, when moving a route element into an index route and/or a
* pathless layout route, relative link behavior contained within should stay
* the same. Both of the following examples should link back to the root:
*
* <Route path="/">
* <Route path="accounts" element={<Link to=".."}>
* </Route>
*
* <Route path="/">
* <Route path="accounts">
* <Route element={<AccountsLayout />}> // <-- Does not contribute
* <Route index element={<Link to=".."} /> // <-- Does not contribute
* </Route
* </Route>
* </Route>
*/
function getPathContributingMatches(matches: RouteMatch[]) {
return matches.filter(
(match, index) =>
index === 0 ||
(!match.route.index &&
match.pathnameBase !== matches[index - 1].pathnameBase)
);
}

/**
* Returns an imperative method for changing the location. Used by <Link>s, but
* may also be used by other elements to change the location.
Expand All @@ -155,16 +185,8 @@ export function useNavigate(): NavigateFunction {
let { matches } = React.useContext(RouteContext);
let { pathname: locationPathname } = useLocation();

// Ignore index + pathless matches
let pathContributingMatches = matches.filter(
(match, index) =>
index === 0 ||
(!match.route.index &&
match.pathnameBase !== matches[index - 1].pathnameBase)
);

let routePathnamesJson = JSON.stringify(
pathContributingMatches.map((match) => match.pathnameBase)
getPathContributingMatches(matches).map((match) => match.pathnameBase)
);

let activeRef = React.useRef(false);
Expand Down Expand Up @@ -262,7 +284,7 @@ export function useResolvedPath(to: To): Path {
let { pathname: locationPathname } = useLocation();

let routePathnamesJson = JSON.stringify(
matches.map((match) => match.pathnameBase)
getPathContributingMatches(matches).map((match) => match.pathnameBase)
);

return React.useMemo(
Expand Down

0 comments on commit e6b6811

Please sign in to comment.