From 96d6c564caf8491cb755195756456afcd77d2c9c Mon Sep 17 00:00:00 2001 From: KagaKyouichirou <1085770948@qq.com> Date: Sun, 30 Mar 2025 21:32:53 +0800 Subject: [PATCH 1/4] test case: useBlocker facing unstable router object --- .../__tests__/useBlocker-test.tsx | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 packages/react-router/__tests__/useBlocker-test.tsx diff --git a/packages/react-router/__tests__/useBlocker-test.tsx b/packages/react-router/__tests__/useBlocker-test.tsx new file mode 100644 index 0000000000..2d1561b7af --- /dev/null +++ b/packages/react-router/__tests__/useBlocker-test.tsx @@ -0,0 +1,147 @@ +import * as React from "react"; +import { render, screen, act } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { + Blocker, + Link, + Outlet, + RouterProvider, + createHashRouter, + useBlocker, + useLocation, +} from "react-router"; + +const Confirm: React.FC<{ blocker: Blocker }> = ({ blocker }) => { + const blocked = blocker.state === "blocked"; + return blocked ? ( +
+ + +
+ ) : null; +}; + +describe("useBlocker", () => { + it("is defensive against an unstable router object", async () => { + const A: React.FC<{ foo: boolean }> = ({ foo }) => { + return ( + <> +

A

+ TO HOME + {`foo: ${foo}`} + + ); + }; + + const B: React.FC<{ + setFoo: React.Dispatch>; + }> = ({ setFoo }) => { + let blocker = useBlocker(true); + return ( + <> +

B

+ + TO HOME + + setFoo((foo) => !foo)} + > + {"click to re-construct router"} + + + + ); + }; + + const Root: React.FC = () => { + const location = useLocation(); + return ( +
+

ROOT

+
{location.pathname}
+ +
+ ); + }; + + const TestUnstableRouterObject: React.FC = () => { + const [foo, setFoo] = React.useState(false); + const router = React.useMemo(() => { + console.log("reconstructing router... foo is: ", foo); + return createHashRouter([ + { + path: "/", + Component: Root, + children: [ + { + index: true, + element: ( + <> +

HOME

+ + TO PAGE A + + + TO PAGE B + + + ), + }, + { + path: "a", + element: , + }, + { + path: "b", + element: , + }, + ], + }, + ]); + }, [foo]); + + return ; + }; + + render(); + + let expectLocation = (location: string) => + expect( + screen.getByTestId("location").textContent + ).toEqual(location); + + expectLocation("/"); + + await userEvent.click(screen.getByTestId("linkToPageB")); + expectLocation("/b"); + + await userEvent.click( + screen.getByTestId("spanReconstructRouter") + ); + await userEvent.click(screen.getByTestId("linkBToHome")); + + await userEvent.click(screen.getByTestId("bttnConfirm")); + expectLocation("/"); + + await userEvent.click(screen.getByTestId("linkToPageA")); + expectLocation("/a"); + + expect( + screen.getByTestId("spanPageADisplayFoo").textContent + ).toEqual("foo: true"); + }); +}); From 15074a56096caa96a798d0fd166206d75207999b Mon Sep 17 00:00:00 2001 From: KagaKyouichirou <1085770948@qq.com> Date: Sun, 30 Mar 2025 21:34:10 +0800 Subject: [PATCH 2/4] fix useBlocker: avoid stale-capturing of blockerKey by making it non-reative value --- packages/react-router/lib/hooks.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 4699b3817a..bf0d7f6cba 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -1257,7 +1257,6 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker { let { router, basename } = useDataRouterContext(DataRouterHook.UseBlocker); let state = useDataRouterState(DataRouterStateHook.UseBlocker); - let [blockerKey, setBlockerKey] = React.useState(""); let blockerFunction = React.useCallback( (arg) => { if (typeof shouldBlock !== "function") { @@ -1290,11 +1289,13 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker { [basename, shouldBlock] ); + let blockerKeyRef = React.useRef(""); + // This effect is in charge of blocker key assignment and deletion (which is // tightly coupled to the key) React.useEffect(() => { let key = String(++blockerId); - setBlockerKey(key); + blockerKeyRef.current = key; return () => router.deleteBlocker(key); }, [router]); @@ -1303,11 +1304,12 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker { // effect so we don't get an orphaned blockerFunction in the router with a // key of "". Until then we just have the IDLE_BLOCKER. React.useEffect(() => { - if (blockerKey !== "") { - router.getBlocker(blockerKey, blockerFunction); + if (blockerKeyRef.current !== "") { + router.getBlocker(blockerKeyRef.current, blockerFunction); } - }, [router, blockerKey, blockerFunction]); + }, [router, blockerFunction]); + let blockerKey = blockerKeyRef.current; // Prefer the blocker from `state` not `router.state` since DataRouterContext // is memoized so this ensures we update on blocker state updates return blockerKey && state.blockers.has(blockerKey) From 5e617a1fa931bdd1036a102b81d2acafbf7be13c Mon Sep 17 00:00:00 2001 From: KagaKyouichirou <1085770948@qq.com> Date: Sun, 30 Mar 2025 22:17:22 +0800 Subject: [PATCH 3/4] add @KagaKyouichirou to contributors.yml --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index dd0d8c165f..a879a08077 100644 --- a/contributors.yml +++ b/contributors.yml @@ -169,6 +169,7 @@ - jungwoo3490 - justjavac - kachun333 +- KagaKyouichirou - Kakamotobi - kantuni - kapil-patel From b21c95560f4b32f2e3b92c8ab81336923d3f0977 Mon Sep 17 00:00:00 2001 From: Fan YANG Date: Mon, 31 Mar 2025 09:50:54 +0800 Subject: [PATCH 4/4] fix linting --- packages/react-router/__tests__/useBlocker-test.tsx | 4 ++-- packages/react-router/lib/hooks.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-router/__tests__/useBlocker-test.tsx b/packages/react-router/__tests__/useBlocker-test.tsx index 2d1561b7af..56226103bd 100644 --- a/packages/react-router/__tests__/useBlocker-test.tsx +++ b/packages/react-router/__tests__/useBlocker-test.tsx @@ -1,8 +1,8 @@ import * as React from "react"; -import { render, screen, act } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import type { Blocker } from "react-router"; import { - Blocker, Link, Outlet, RouterProvider, diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index bf0d7f6cba..f7ff075067 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -1290,7 +1290,7 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker { ); let blockerKeyRef = React.useRef(""); - + // This effect is in charge of blocker key assignment and deletion (which is // tightly coupled to the key) React.useEffect(() => {