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.

Sign up for GitHub

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 useNavigate when called from <Routes> inside a <RouterProvider> #10432

Merged
merged 1 commit into from
May 2, 2023
Merged

Fix useNavigate when called from <Routes> inside a <RouterProvider> #10432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/navigate-from-routes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Fix bug when calling `useNavigate` from `<Routes>` inside a `<RouterProvider>`
173 changes: 173 additions & 0 deletions packages/react-router/__tests__/useNavigate-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Route,
useNavigate,
useLocation,
useRoutes,
createMemoryRouter,
createRoutesFromElements,
Outlet,
Expand Down Expand Up @@ -301,6 +302,178 @@ describe("useNavigate", () => {
);
});

it("allows useNavigate usage in a mixed RouterProvider/<Routes> scenario", () => {
const router = createMemoryRouter([
{
path: "/*",
Component() {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
let navigate = useNavigate();
let location = useLocation();
return (
<>
<button
onClick={() =>
navigate(location.pathname === "/" ? "/page" : "/")
}
>
Navigate from RouterProvider
</button>
<Routes>
<Route path="/" element={<Home />} />
<Route path="/page" element={<Page />} />
</Routes>
</>
);
},
},
]);

function Home() {
let navigate = useNavigate();
return (
<>
<h1>Home</h1>
<button onClick={() => navigate("/page")}>
Navigate /page from Routes
</button>
</>
);
}

function Page() {
let navigate = useNavigate();
return (
<>
<h1>Page</h1>
<button onClick={() => navigate("/")}>
Navigate /home from Routes
</button>
</>
);
}

let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(<RouterProvider router={router} />);
});

expect(router.state.location.pathname).toBe("/");
expect(renderer.toJSON()).toMatchInlineSnapshot(`
[
<button
onClick={[Function]}
>
Navigate from RouterProvider
</button>,
<h1>
Home
</h1>,
<button
onClick={[Function]}
>
Navigate /page from Routes
</button>,
]
`);

let button = renderer.root.findByProps({
children: "Navigate from RouterProvider",
});
TestRenderer.act(() => button.props.onClick());

expect(router.state.location.pathname).toBe("/page");
expect(renderer.toJSON()).toMatchInlineSnapshot(`
[
<button
onClick={[Function]}
>
Navigate from RouterProvider
</button>,
<h1>
Page
</h1>,
<button
onClick={[Function]}
>
Navigate /home from Routes
</button>,
]
`);

button = renderer.root.findByProps({
children: "Navigate from RouterProvider",
});
TestRenderer.act(() => button.props.onClick());

expect(router.state.location.pathname).toBe("/");
expect(renderer.toJSON()).toMatchInlineSnapshot(`
[
<button
onClick={[Function]}
>
Navigate from RouterProvider
</button>,
<h1>
Home
</h1>,
<button
onClick={[Function]}
>
Navigate /page from Routes
</button>,
]
`);

button = renderer.root.findByProps({
children: "Navigate /page from Routes",
});
TestRenderer.act(() => button.props.onClick());

expect(router.state.location.pathname).toBe("/page");
expect(renderer.toJSON()).toMatchInlineSnapshot(`
[
<button
onClick={[Function]}
>
Navigate from RouterProvider
</button>,
<h1>
Page
</h1>,
<button
onClick={[Function]}
>
Navigate /home from Routes
</button>,
]
`);

button = renderer.root.findByProps({
children: "Navigate /home from Routes",
});
TestRenderer.act(() => button.props.onClick());

expect(router.state.location.pathname).toBe("/");
expect(renderer.toJSON()).toMatchInlineSnapshot(`
[
<button
onClick={[Function]}
>
Navigate from RouterProvider
</button>,
<h1>
Home
</h1>,
<button
onClick={[Function]}
>
Navigate /page from Routes
</button>,
]
`);
});

describe("navigating in effects versus render", () => {
let warnSpy: jest.SpyInstance;

Expand Down
2 changes: 2 additions & 0 deletions packages/react-router/lib/context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,13 @@ if (__DEV__) {
export interface RouteContextObject {
outlet: React.ReactElement | null;
matches: RouteMatch[];
isDataRoute: boolean;
}

export const RouteContext = React.createContext<RouteContextObject>({
outlet: null,
matches: [],
isDataRoute: false,
});

if (__DEV__) {
Expand Down
12 changes: 8 additions & 4 deletions packages/react-router/lib/hooks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@ function useIsomorphicLayoutEffect(
* @see https://reactrouter.com/hooks/use-navigate
*/
export function useNavigate(): NavigateFunction {
let isDataRouter = React.useContext(DataRouterContext) != null;
let { isDataRoute } = React.useContext(RouteContext);
// Conditional usage is OK here because the usage of a data router is static
// eslint-disable-next-line react-hooks/rules-of-hooks
return isDataRouter ? useNavigateStable() : useNavigateUnstable();
return isDataRoute ? useNavigateStable() : useNavigateUnstable();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't just look for any ancestor data router context, since that gives a false positive if we're in a descendanrt routes:

let router = createBrowserRouter([{
  path: '/*',
  Component() {
    return (
      <Routes>
        <Route path="/" element={<Home />} />
      </Routes>
    );
  }
}]);

function Home() {
  let navigate = useNavigate();
  //  ^ This should use `useNavigateUnstable` since it's the router is 
  //    unaware of the descendant routes hierarchy and cannot handle 
  //    the relative routing logic.
}

function App() {
  return <RouterProvider router={router} />
}

}

function useNavigateUnstable(): NavigateFunction {
Expand Down Expand Up @@ -697,7 +697,11 @@ export function _renderMatches(
return (
<RenderedRoute
match={match}
routeContext={{ outlet, matches }}
routeContext={{
outlet,
matches,
isDataRoute: dataRouterState != null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to get route-aware "am I in a data router" we add isDataRoute to the RouteContext

}}
children={children}
/>
);
Expand All @@ -713,7 +717,7 @@ export function _renderMatches(
component={errorElement}
error={error}
children={getChildren()}
routeContext={{ outlet: null, matches }}
routeContext={{ outlet: null, matches, isDataRoute: true }}
/>
) : (
getChildren()
Expand Down