Skip to content

Commit

Permalink
fix: avoid navigation loops in <Navigate> re-renders
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed Aug 3, 2022
1 parent 617b27c commit 67b4023
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/cuddly-dingos-tickle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

fix: avoid navigation loops in <Navigate> re-renders (#9124)
67 changes: 66 additions & 1 deletion packages/react-router/__tests__/navigate-test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import * as React from "react";
import * as TestRenderer from "react-test-renderer";
import { MemoryRouter, Navigate, Outlet, Routes, Route } from "react-router";
import {
DataMemoryRouter,
MemoryRouter,
Navigate,
Outlet,
Routes,
Route,
} from "react-router";
import { prettyDOM, render, screen, waitFor } from "@testing-library/react";

describe("<Navigate>", () => {
describe("with an absolute href", () => {
Expand Down Expand Up @@ -218,4 +226,61 @@ describe("<Navigate>", () => {
`);
});
});

it("does not cause dual navigations in strict mode", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<React.StrictMode>
<MemoryRouter initialEntries={["/1", "/2", "/3"]} initialIndex={2}>
<Routes>
<Route path="1" element={<h1>1</h1>} />
<Route path="2" element={<h1>2</h1>} />
<Route path="3" element={<Navigate to={-1} />} />
</Routes>
</MemoryRouter>
</React.StrictMode>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<h1>
2
</h1>
`);
});

it("does not cause navigation loops in data routers", async () => {
// Note this is not the idiomatic way to do these redirects, they should
// be done with loaders in data routers, but this is a likely scenario to
// encounter while migrating to a data router
let { container } = render(
<React.StrictMode>
<DataMemoryRouter initialEntries={["/home"]}>
<Route path="home" element={<Navigate to="/about" />} />
<Route
path="about"
element={<h1>About</h1>}
loader={() => new Promise((r) => setTimeout(() => r("ok"), 10))}
/>
</DataMemoryRouter>
</React.StrictMode>
);

await waitFor(() => screen.getByText("About"));

expect(getHtml(container)).toMatchInlineSnapshot(`
"<div>
<h1>
About
</h1>
</div>"
`);
});
});

function getHtml(container: HTMLElement) {
return prettyDOM(container, undefined, {
highlight: false,
});
}
8 changes: 8 additions & 0 deletions packages/react-router/lib/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,15 @@ export function Navigate({ to, replace, state }: NavigateProps): null {
);

let navigate = useNavigate();

// Avoid kicking off multiple navigations during dual-renders in strict mode,
// or when components get re-rendered due to global navigation-driven
// re-renders (i.e., entering a loading state)
let isMountedRef = React.useRef(false);

React.useEffect(() => {
if (isMountedRef.current) return;
isMountedRef.current = true;
navigate(to, { replace, state });
});

Expand Down

0 comments on commit 67b4023

Please sign in to comment.