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

Jump to bottom

Fix usage of Navigate in strict mode when using a data router #10435

Merged
merged 4 commits into from
May 2, 2023
Merged

Fix usage of Navigate in strict mode when using a data router #10435

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
19204a1
Fix usage of Navigate in strict mode when using a data router
brophdawg11 May 2, 2023
d942ed6
Bump bundle
brophdawg11 May 2, 2023
93ccb2b
Add tests for different setState-driven Navigte renders
brophdawg11 May 2, 2023
7f83a20
update test
brophdawg11 May 2, 2023
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
5 changes: 5 additions & 0 deletions .changeset/navigate-strict-mode.md
View file
Open in desktop
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
Open in desktop
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
368 changes: 368 additions & 0 deletions packages/react-router/__tests__/navigate-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,374 @@ describe("<Navigate>", () => {
</div>"
`);
});

it("handles sync 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",
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 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