diff --git a/.changeset/lucky-lamps-lick.md b/.changeset/lucky-lamps-lick.md new file mode 100644 index 0000000000..68602ffcf6 --- /dev/null +++ b/.changeset/lucky-lamps-lick.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix `unstable_useBlocker` infinite loop diff --git a/contributors.yml b/contributors.yml index 269b93026a..b37f961516 100644 --- a/contributors.yml +++ b/contributors.yml @@ -62,6 +62,7 @@ - emzoumpo - engpetermwangi - ericschn +- fernandojbf - FilipJirsak - frontsideair - fyzhu diff --git a/packages/react-router/__tests__/useBlocker-test.tsx b/packages/react-router/__tests__/useBlocker-test.tsx new file mode 100644 index 0000000000..c1bfb1294d --- /dev/null +++ b/packages/react-router/__tests__/useBlocker-test.tsx @@ -0,0 +1,448 @@ +import * as React from "react"; +import * as TestRenderer from "react-test-renderer"; +import { + useNavigate, + unstable_useBlocker, + createMemoryRouter, + RouterProvider, +} from "react-router"; + +describe("useBlocker", () => { + describe("should block navigation", () => { + it("passing true as argument", () => { + const ref = React.createRef(); + function Home() { + const result = unstable_useBlocker(true); + let navigate = useNavigate(); + + // @ts-expect-error + ref.current = result; + + function handleClick() { + navigate("/about"); + } + + return ( +
+

Home

+ +
+ ); + } + const routes = [ + { + path: "/home", + element: , + }, + { path: "/about", element:

About

}, + ]; + + const router = createMemoryRouter(routes, { + initialEntries: ["/home"], + initialIndex: 1, + }); + + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + let button = renderer.root.findByType("button"); + TestRenderer.act(() => button.props.onClick()); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +
+

+ Home +

+ +
+ `); + + expect(ref.current).toMatchObject({ + location: { + hash: "", + key: expect.any(String), + pathname: "/about", + search: "", + state: null, + }, + proceed: expect.any(Function), + reset: expect.any(Function), + state: "blocked", + }); + }); + + it("passing a function that returns true", () => { + const fn = jest.fn().mockReturnValue(true); + const ref = React.createRef(); + + function Home() { + const result = unstable_useBlocker(fn); + let navigate = useNavigate(); + + // @ts-expect-error + ref.current = result; + + function handleClick() { + navigate("/about"); + } + + return ( +
+

Home

+ +
+ ); + } + const routes = [ + { + path: "/home", + element: , + }, + { path: "/about", element:

About

}, + ]; + + const router = createMemoryRouter(routes, { + initialEntries: ["/home"], + initialIndex: 1, + }); + + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + let button = renderer.root.findByType("button"); + TestRenderer.act(() => button.props.onClick()); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +
+

+ Home +

+ +
+ `); + + expect(ref.current).toMatchObject({ + location: { + hash: "", + key: expect.any(String), + pathname: "/about", + search: "", + state: null, + }, + proceed: expect.any(Function), + reset: expect.any(Function), + state: "blocked", + }); + }); + }); + + describe("should NOT block navigation", () => { + it("passing false as argument", () => { + const ref = React.createRef(); + function Home() { + const result = unstable_useBlocker(false); + let navigate = useNavigate(); + + // @ts-expect-error + ref.current = result; + + function handleClick() { + navigate("/about"); + } + + return ( +
+

Home

+ +
+ ); + } + const routes = [ + { + path: "/home", + element: , + }, + { path: "/about", element:

About

}, + ]; + + const router = createMemoryRouter(routes, { + initialEntries: ["/home"], + initialIndex: 1, + }); + + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + let button = renderer.root.findByType("button"); + TestRenderer.act(() => button.props.onClick()); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + + expect(ref.current).toMatchObject({ + location: undefined, + proceed: undefined, + reset: undefined, + state: "unblocked", + }); + }); + + it("passing a function that returns false", () => { + const fn = jest.fn().mockReturnValue(false); + + const ref = React.createRef(); + function Home() { + const result = unstable_useBlocker(fn); + let navigate = useNavigate(); + + // @ts-expect-error + ref.current = result; + function handleClick() { + navigate("/about"); + } + + return ( +
+

Home

+ +
+ ); + } + const routes = [ + { + path: "/home", + element: , + }, + { path: "/about", element:

About

}, + ]; + + const router = createMemoryRouter(routes, { + initialEntries: ["/home"], + initialIndex: 1, + }); + + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + let button = renderer.root.findByType("button"); + TestRenderer.act(() => button.props.onClick()); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + + expect(ref.current).toMatchObject({ + location: undefined, + proceed: undefined, + reset: undefined, + state: "unblocked", + }); + }); + }); + + describe("should stop blocking after change (update)", () => { + it("should stop blocking after changing state (boolean)", () => { + const ref = React.createRef(); + function Home() { + const [state, setState] = React.useState(true); + const result = unstable_useBlocker(state); + let navigate = useNavigate(); + + // @ts-expect-error + ref.current = result; + + function enableBlock() { + setState(false); + } + + function handleClick() { + navigate("/about"); + } + + return ( +
+

Home

+ + +
+ ); + } + const routes = [ + { + path: "/home", + element: , + }, + { path: "/about", element:

About

}, + ]; + + const router = createMemoryRouter(routes, { + initialEntries: ["/home"], + initialIndex: 1, + }); + + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + let button = renderer.root.findAllByType("button")[1]; + TestRenderer.act(() => button.props.onClick()); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +
+

+ Home +

+ + +
+ `); + + expect(ref.current).toMatchObject({ + location: { + hash: "", + key: expect.any(String), + pathname: "/about", + search: "", + state: null, + }, + proceed: expect.any(Function), + reset: expect.any(Function), + state: "blocked", + }); + + // update blocker to stop blocking + // @ts-expect-error + let updater = renderer.root.findAllByType("button")[0]; + TestRenderer.act(() => updater.props.onClick()); + + // @ts-expect-error + button = renderer.root.findAllByType("button")[1]; + TestRenderer.act(() => button.props.onClick()); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); + + it("should stop blocking after changing state (function)", () => { + const ref = React.createRef(); + function Home() { + const [state, setState] = React.useState(true); + const result = unstable_useBlocker(() => state); + let navigate = useNavigate(); + + // @ts-expect-error + ref.current = result; + + function enableBlock() { + setState(false); + } + + function handleClick() { + navigate("/about"); + } + + return ( +
+

Home

+ + +
+ ); + } + const routes = [ + { + path: "/home", + element: , + }, + { path: "/about", element:

About

}, + ]; + + const router = createMemoryRouter(routes, { + initialEntries: ["/home"], + initialIndex: 1, + }); + + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create(); + }); + + // @ts-expect-error + let button = renderer.root.findAllByType("button")[1]; + TestRenderer.act(() => button.props.onClick()); + + expect(ref.current).toMatchObject({ + location: { + hash: "", + key: expect.any(String), + pathname: "/about", + search: "", + state: null, + }, + proceed: expect.any(Function), + reset: expect.any(Function), + state: "blocked", + }); + + // update blocker to stop blocking + // @ts-expect-error + let updater = renderer.root.findAllByType("button")[0]; + TestRenderer.act(() => updater.props.onClick()); + + // @ts-expect-error + button = renderer.root.findAllByType("button")[1]; + TestRenderer.act(() => button.props.onClick()); + + // @ts-expect-error + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); + }); +}); diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 0135ea0bcc..0a9e96b79e 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -926,8 +926,6 @@ export function useAsyncError(): unknown { return value?._error; } -let blockerId = 0; - /** * Allow the application to block navigations within the SPA and present the * user a confirmation dialog to confirm the navigation. Mostly used to avoid @@ -935,11 +933,10 @@ let blockerId = 0; * cross-origin navigations. */ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker { + let blockerId = React.useId(); let { router, basename } = useDataRouterContext(DataRouterHook.UseBlocker); let state = useDataRouterState(DataRouterStateHook.UseBlocker); - let [blockerKey, setBlockerKey] = React.useState(""); - let [blocker, setBlocker] = React.useState(IDLE_BLOCKER); let blockerFunction = React.useCallback( (arg) => { if (typeof shouldBlock !== "function") { @@ -973,17 +970,13 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker { ); React.useEffect(() => { - let key = String(++blockerId); - setBlocker(router.getBlocker(key, blockerFunction)); - setBlockerKey(key); - return () => router.deleteBlocker(key); - }, [router, setBlocker, setBlockerKey, blockerFunction]); + router.getBlocker(blockerId, blockerFunction); + return () => router.deleteBlocker(blockerId); + }, [router, blockerId, blockerFunction]); // Prefer the blocker from state since DataRouterContext is memoized so this // ensures we update on blocker state updates - return blockerKey && state.blockers.has(blockerKey) - ? state.blockers.get(blockerKey)! - : blocker; + return state.blockers.get(blockerId) || IDLE_BLOCKER; } /**