Skip to content

Commit

Permalink
Fix usage of Navigate in strict mode when using a data router (#10435)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brophdawg11
brophdawg11 committed May 2, 2023
1 parent 5e195ec commit c4e9607
Show file tree
Hide file tree
Showing 4 changed files with 392 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/navigate-strict-mode.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Fix usage of `<Navigate>` in strict mode when using a data router
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@
"none": "45 kB"
},
"packages/react-router/dist/react-router.production.min.js": {
"none": "13.1 kB"
"none": "13.3 kB"
},
"packages/react-router/dist/umd/react-router.production.min.js": {
"none": "15.4 kB"
"none": "15.6 kB"
},
"packages/react-router-dom/dist/react-router-dom.production.min.js": {
"none": "11.8 kB"
Expand Down
365 changes: 365 additions & 0 deletions packages/react-router/__tests__/navigate-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,371 @@ describe("<Navigate>", () => {
</div>"
`);
});

it("handles sync relative navigations in StrictMode using a data router", async () => {
const router = createMemoryRouter([
{
path: "/",
children: [
{
index: true,
// This is a relative navigation from the current location of /a.
// Ensure we don't route from / -> /b -> /b/b
Component: () => <Navigate to={"b"} replace />,
},
{
path: "b",
element: <h1>Page B</h1>,
},
],
},
]);

let { container } = render(
<React.StrictMode>
<RouterProvider router={router} />
</React.StrictMode>
);

await waitFor(() => screen.getByText("Page B"));

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

it("handles async relative navigations in StrictMode using a data router", async () => {
const router = createMemoryRouter(
[
{
path: "/a",
children: [
{
index: true,
// This is a relative navigation from the current location of /a.
// Ensure we don't route from /a -> /a/b -> /a/b/b
Component: () => <Navigate to={"b"} replace />,
},
{
path: "b",
async loader() {
await new Promise((r) => setTimeout(r, 10));
return null;
},
element: <h1>Page B</h1>,
},
],
},
],
{ initialEntries: ["/a"] }
);

let { container } = render(
<React.StrictMode>
<RouterProvider router={router} />
</React.StrictMode>
);

await waitFor(() => screen.getByText("Page B"));

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

it("handles setState in render in StrictMode using a data router (sync loader)", async () => {
let renders: number[] = [];
const router = createMemoryRouter([
{
path: "/",
children: [
{
index: true,
Component() {
let [count, setCount] = React.useState(0);
if (count === 0) {
setCount(1);
}
return <Navigate to={"b"} replace state={{ count }} />;
},
},
{
path: "b",
Component() {
let { state } = useLocation() as { state: { count: number } };
renders.push(state.count);
return (
<>
<h1>Page B</h1>
<p>{state.count}</p>
</>
);
},
},
],
},
]);

let navigateSpy = jest.spyOn(router, "navigate");

let { container } = render(
<React.StrictMode>
<RouterProvider router={router} />
</React.StrictMode>
);

await waitFor(() => screen.getByText("Page B"));

expect(getHtml(container)).toMatchInlineSnapshot(`
"<div>
<h1>
Page B
</h1>
<p>
1
</p>
</div>"
`);
expect(navigateSpy).toHaveBeenCalledTimes(2);
expect(navigateSpy.mock.calls[0]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 1 } },
]);
expect(navigateSpy.mock.calls[1]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 1 } },
]);
expect(renders).toEqual([1, 1]);
});

it("handles setState in effect in StrictMode using a data router (sync loader)", async () => {
let renders: number[] = [];
const router = createMemoryRouter([
{
path: "/",
children: [
{
index: true,
Component() {
// When state managed by react and changes during render, we'll
// only "see" the value from the first pass through here in our
// effects
let [count, setCount] = React.useState(0);
React.useEffect(() => {
if (count === 0) {
setCount(1);
}
}, [count]);
return <Navigate to={"b"} replace state={{ count }} />;
},
},
{
path: "b",
Component() {
let { state } = useLocation() as { state: { count: number } };
renders.push(state.count);
return (
<>
<h1>Page B</h1>
<p>{state.count}</p>
</>
);
},
},
],
},
]);

let navigateSpy = jest.spyOn(router, "navigate");

let { container } = render(
<React.StrictMode>
<RouterProvider router={router} />
</React.StrictMode>
);

await waitFor(() => screen.getByText("Page B"));

expect(getHtml(container)).toMatchInlineSnapshot(`
"<div>
<h1>
Page B
</h1>
<p>
0
</p>
</div>"
`);
expect(navigateSpy).toHaveBeenCalledTimes(2);
expect(navigateSpy.mock.calls[0]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 0 } },
]);
expect(navigateSpy.mock.calls[1]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 0 } },
]);
expect(renders).toEqual([0, 0]);
});

it("handles setState in render in StrictMode using a data router (async loader)", async () => {
let renders: number[] = [];
const router = createMemoryRouter([
{
path: "/",
children: [
{
index: true,
Component() {
let [count, setCount] = React.useState(0);
if (count === 0) {
setCount(1);
}
return <Navigate to={"b"} replace state={{ count }} />;
},
},
{
path: "b",
async loader() {
await new Promise((r) => setTimeout(r, 10));
return null;
},
Component() {
let { state } = useLocation() as { state: { count: number } };
renders.push(state.count);
return (
<>
<h1>Page B</h1>
<p>{state.count}</p>
</>
);
},
},
],
},
]);

let navigateSpy = jest.spyOn(router, "navigate");

let { container } = render(
<React.StrictMode>
<RouterProvider router={router} />
</React.StrictMode>
);

await waitFor(() => screen.getByText("Page B"));

expect(getHtml(container)).toMatchInlineSnapshot(`
"<div>
<h1>
Page B
</h1>
<p>
1
</p>
</div>"
`);
expect(navigateSpy).toHaveBeenCalledTimes(2);
expect(navigateSpy.mock.calls[0]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 1 } },
]);
expect(navigateSpy.mock.calls[1]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 1 } },
]);
// /a/b rendered with the same state value both times
expect(renders).toEqual([1, 1]);
});

it("handles setState in effect in StrictMode using a data router (async loader)", async () => {
let renders: number[] = [];
const router = createMemoryRouter([
{
path: "/",
children: [
{
index: true,
Component() {
// When state managed by react and changes during render, we'll
// only "see" the value from the first pass through here in our
// effects
let [count, setCount] = React.useState(0);
React.useEffect(() => {
if (count === 0) {
setCount(1);
}
}, [count]);
return <Navigate to={"b"} replace state={{ count }} />;
},
},
{
path: "b",
async loader() {
await new Promise((r) => setTimeout(r, 10));
return null;
},
Component() {
let { state } = useLocation() as { state: { count: number } };
renders.push(state.count);
return (
<>
<h1>Page B</h1>
<p>{state.count}</p>
</>
);
},
},
],
},
]);

let navigateSpy = jest.spyOn(router, "navigate");

let { container } = render(
<React.StrictMode>
<RouterProvider router={router} />
</React.StrictMode>
);

await waitFor(() => screen.getByText("Page B"));

expect(getHtml(container)).toMatchInlineSnapshot(`
"<div>
<h1>
Page B
</h1>
<p>
1
</p>
</div>"
`);
expect(navigateSpy).toHaveBeenCalledTimes(3);
expect(navigateSpy.mock.calls[0]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 0 } },
]);
expect(navigateSpy.mock.calls[1]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 0 } },
]);
// StrictMode only applies the double-effect execution on component mount,
// not component update
expect(navigateSpy.mock.calls[2]).toMatchObject([
{ pathname: "/b" },
{ state: { count: 1 } },
]);
// /a/b rendered with the latest state value both times
expect(renders).toEqual([1, 1]);
});
});

function getHtml(container: HTMLElement) {
Expand Down
Loading

0 comments on commit c4e9607

Please sign in to comment.