diff --git a/.changeset/navigate-strict-mode.md b/.changeset/navigate-strict-mode.md new file mode 100644 index 0000000000..e417ed415b --- /dev/null +++ b/.changeset/navigate-strict-mode.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix usage of `` in strict mode when using a data router diff --git a/package.json b/package.json index 6bc8bb70bc..8b95a2d15f 100644 --- a/package.json +++ b/package.json @@ -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" diff --git a/packages/react-router/__tests__/navigate-test.tsx b/packages/react-router/__tests__/navigate-test.tsx index f0b9e71644..5342758b5e 100644 --- a/packages/react-router/__tests__/navigate-test.tsx +++ b/packages/react-router/__tests__/navigate-test.tsx @@ -485,6 +485,371 @@ describe("", () => { " `); }); + + 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: () => , + }, + { + path: "b", + element:

Page B

, + }, + ], + }, + ]); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+
" + `); + }); + + 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: () => , + }, + { + path: "b", + async loader() { + await new Promise((r) => setTimeout(r, 10)); + return null; + }, + element:

Page B

, + }, + ], + }, + ], + { initialEntries: ["/a"] } + ); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+
" + `); + }); + + 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 ; + }, + }, + { + path: "b", + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 1 +

+
" + `); + 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 ; + }, + }, + { + path: "b", + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 0 +

+
" + `); + 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 ; + }, + }, + { + 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 ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 1 +

+
" + `); + 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 ; + }, + }, + { + 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 ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 1 +

+
" + `); + 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) { diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 1c526b830d..56ef4c1507 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -16,8 +16,10 @@ import { createMemoryHistory, UNSAFE_invariant as invariant, parsePath, + resolveTo, stripBasename, UNSAFE_warning as warning, + UNSAFE_getPathContributingMatches as getPathContributingMatches, } from "@remix-run/router"; import type { @@ -34,6 +36,7 @@ import { DataRouterContext, DataRouterStateContext, AwaitContext, + RouteContext, } from "./context"; import { useAsyncValue, @@ -43,6 +46,7 @@ import { useRoutes, _renderMatches, useRoutesImpl, + useLocation, } from "./hooks"; export interface RouterProviderProps { @@ -214,18 +218,24 @@ export function Navigate({ `only ever rendered in response to some user interaction or state change.` ); - let dataRouterState = React.useContext(DataRouterStateContext); + let { matches } = React.useContext(RouteContext); + let { pathname: locationPathname } = useLocation(); let navigate = useNavigate(); - React.useEffect(() => { - // Avoid kicking off multiple navigations if we're in the middle of a - // data-router navigation, since components get re-rendered when we enter - // a submitting/loading state - if (dataRouterState && dataRouterState.navigation.state !== "idle") { - return; - } - navigate(to, { replace, state, relative }); - }); + // Resolve the path outside of the effect so that when effects run twice in + // StrictMode they navigate to the same place + let path = resolveTo( + to, + getPathContributingMatches(matches).map((match) => match.pathnameBase), + locationPathname, + relative === "path" + ); + let jsonPath = JSON.stringify(path); + + React.useEffect( + () => navigate(JSON.parse(jsonPath), { replace, state, relative }), + [navigate, jsonPath, relative, replace, state] + ); return null; }