diff --git a/.changeset/navigate-in-effect.md b/.changeset/navigate-in-effect.md new file mode 100644 index 0000000000..bffcb59964 --- /dev/null +++ b/.changeset/navigate-in-effect.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix detection of `useNavigate` in the render cycle by setting the `activeRef` in a layout effect, allowing the `navigate` function to be passed to child components and called in a `useEffect` there. diff --git a/package.json b/package.json index f0d70a7e3e..f86805f9ae 100644 --- a/package.json +++ b/package.json @@ -108,10 +108,10 @@ "none": "45.8 kB" }, "packages/react-router/dist/react-router.production.min.js": { - "none": "13 kB" + "none": "13.1 kB" }, "packages/react-router/dist/umd/react-router.production.min.js": { - "none": "15.3 kB" + "none": "15.4 kB" }, "packages/react-router-dom/dist/react-router-dom.production.min.js": { "none": "12 kB" diff --git a/packages/react-router/__tests__/useNavigate-test.tsx b/packages/react-router/__tests__/useNavigate-test.tsx index cd9d9d2433..6c601a82c5 100644 --- a/packages/react-router/__tests__/useNavigate-test.tsx +++ b/packages/react-router/__tests__/useNavigate-test.tsx @@ -301,6 +301,211 @@ describe("useNavigate", () => { ); }); + describe("navigating in effects versus render", () => { + let warnSpy: jest.SpyInstance; + + beforeEach(() => { + warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + describe("MemoryRouter", () => { + it("does not allow navigation from the render cycle", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + } /> + About} /> + + + ); + }); + + function Home() { + let navigate = useNavigate(); + navigate("/about"); + return

Home

; + } + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ Home +

+ `); + expect(warnSpy).toHaveBeenCalledWith( + "You should call navigate() in a React.useEffect(), not when your component is first rendered." + ); + }); + + it("allows navigation from effects", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + } /> + About} /> + + + ); + }); + + function Home() { + let navigate = useNavigate(); + React.useEffect(() => navigate("/about"), [navigate]); + return

Home

; + } + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("allows navigation in child useEffects", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + } /> + About} /> + + + ); + }); + + function Parent() { + let navigate = useNavigate(); + let onChildRendered = React.useCallback( + () => navigate("/about"), + [navigate] + ); + return ; + } + + function Child({ onChildRendered }) { + React.useEffect(() => onChildRendered()); + return null; + } + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); + }); + + describe("RouterProvider", () => { + it("does not allow navigation from the render cycle", async () => { + let router = createMemoryRouter([ + { + index: true, + Component() { + let navigate = useNavigate(); + navigate("/about"); + return

Home

; + }, + }, + { + path: "about", + element:

About

, + }, + ]); + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ Home +

+ `); + expect(warnSpy).toHaveBeenCalledWith( + "You should call navigate() in a React.useEffect(), not when your component is first rendered." + ); + }); + + it("allows navigation from effects", () => { + let router = createMemoryRouter([ + { + index: true, + Component() { + let navigate = useNavigate(); + React.useEffect(() => navigate("/about"), [navigate]); + return

Home

; + }, + }, + { + path: "about", + element:

About

, + }, + ]); + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("allows navigation in child useEffects", () => { + let router = createMemoryRouter([ + { + index: true, + Component() { + let navigate = useNavigate(); + let onChildRendered = React.useCallback( + () => navigate("/about"), + [navigate] + ); + return ; + }, + }, + { + path: "about", + element:

About

, + }, + ]); + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + function Child({ onChildRendered }) { + React.useEffect(() => onChildRendered()); + return null; + } + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); + }); + }); + describe("with state", () => { it("adds the state to location.state", () => { function Home() { diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index e21e533195..be846ef51a 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -150,6 +150,23 @@ export interface NavigateFunction { (delta: number): void; } +const navigateEffectWarning = + `You should call navigate() in a React.useEffect(), not when ` + + `your component is first rendered.`; + +// Mute warnings for calls to useNavigate in SSR environments +function useIsomorphicLayoutEffect( + cb: Parameters[0] +) { + let isStatic = React.useContext(NavigationContext).static; + if (!isStatic) { + // We should be able to get rid of this once react 18.3 is released + // See: https://github.com/facebook/react/pull/26395 + // eslint-disable-next-line react-hooks/rules-of-hooks + React.useLayoutEffect(cb); + } +} + /** * Returns an imperative method for changing the location. Used by s, but * may also be used by other elements to change the location. @@ -180,18 +197,16 @@ function useNavigateUnstable(): NavigateFunction { ); let activeRef = React.useRef(false); - React.useEffect(() => { + useIsomorphicLayoutEffect(() => { activeRef.current = true; }); let navigate: NavigateFunction = React.useCallback( (to: To | number, options: NavigateOptions = {}) => { - warning( - activeRef.current, - `You should call navigate() in a React.useEffect(), not when ` + - `your component is first rendered.` - ); + warning(activeRef.current, navigateEffectWarning); + // Short circuit here since if this happens on first render the navigate + // is useless because we haven't wired up our history listener yet if (!activeRef.current) return; if (typeof to === "number") { @@ -931,8 +946,19 @@ function useNavigateStable(): NavigateFunction { let { router } = useDataRouterContext(DataRouterHook.UseNavigateStable); let id = useCurrentRouteId(DataRouterStateHook.UseNavigateStable); + let activeRef = React.useRef(false); + useIsomorphicLayoutEffect(() => { + activeRef.current = true; + }); + let navigate: NavigateFunction = React.useCallback( (to: To | number, options: NavigateOptions = {}) => { + warning(activeRef.current, navigateEffectWarning); + + // Short circuit here since if this happens on first render the navigate + // is useless because we haven't wired up our router subscriber yet + if (!activeRef.current) return; + if (typeof to === "number") { router.navigate(to); } else {