Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: properly support index routes with a path in useResolvedPath #9486

Merged
merged 4 commits into from Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/cyan-poems-clean.md
@@ -0,0 +1,6 @@
---
"react-router-dom": patch
"@remix-run/router": patch
---

properly support index routes with a path in useResolvedPath
23 changes: 23 additions & 0 deletions packages/react-router-dom/__tests__/data-browser-router-test.tsx
Expand Up @@ -1737,6 +1737,29 @@ function testDomRouter(
await new Promise((r) => setTimeout(r, 0));
assertLocation(testWindow, "/form", "?index");
});

it("handles index routes with a path", async () => {
let { container } = render(
<TestDataRouter
window={getWindow("/foo/bar?a=1#hash")}
hydrationData={{}}
>
<Route path="/">
<Route path="foo">
<Route
index={true}
path="bar"
element={<NoActionComponent />}
/>
</Route>
</Route>
</TestDataRouter>
);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo/bar?index&a=1#hash"
);
});
});

describe("dynamic routes", () => {
Expand Down
31 changes: 1 addition & 30 deletions packages/react-router/lib/hooks.tsx
Expand Up @@ -19,6 +19,7 @@ import {
parsePath,
resolveTo,
warning,
UNSAFE_getPathContributingMatches as getPathContributingMatches,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the duped function and exported it as a private function from @remix-run/router

} from "@remix-run/router";

import type {
Expand Down Expand Up @@ -147,36 +148,6 @@ 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 Down
7 changes: 5 additions & 2 deletions packages/router/index.ts
@@ -1,4 +1,4 @@
import { convertRoutesToDataRoutes } from "./utils";
import { convertRoutesToDataRoutes, getPathContributingMatches } from "./utils";

export type {
ActionFunction,
Expand Down Expand Up @@ -79,4 +79,7 @@ export * from "./router";
///////////////////////////////////////////////////////////////////////////////

/** @internal */
export { convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes };
export {
convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes,
getPathContributingMatches as UNSAFE_getPathContributingMatches,
};
6 changes: 3 additions & 3 deletions packages/router/utils.ts
Expand Up @@ -836,6 +836,8 @@ function getInvalidPathError(
}

/**
* @private
*
* 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.
Expand All @@ -861,9 +863,7 @@ export function getPathContributingMatches<
>(matches: T[]) {
return matches.filter(
(match, index) =>
index === 0 ||
(!match.route.index &&
match.pathnameBase !== matches[index - 1].pathnameBase)
index === 0 || match.pathnameBase !== matches[index - 1].pathnameBase
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't look for match.route.index at all - just look to see if the pathnameBase changed

);
}

Expand Down