Fix issue with reused blockers on subsequent navigations (#10656)

remix-run · Jun 30, 2023 · cbda9cf · cbda9cf
1 parent 775bff9
commit cbda9cf
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 8 deletions.
diff --git a/.changeset/fix-reused-blocker.md b/.changeset/fix-reused-blocker.md
@@ -0,0 +1,6 @@
+---
+"react-router": patch
+"@remix-run/router": patch
+---
+
+Fix issue with reused blockers on subsequent navigations
diff --git a/package.json b/package.json
@@ -109,7 +109,7 @@
   },
   "filesize": {
     "packages/router/dist/router.umd.min.js": {
-      "none": "46.5 kB"
+      "none": "46.6 kB"
     },
     "packages/react-router/dist/react-router.production.min.js": {
       "none": "13.8 kB"

diff --git a/packages/react-router-dom/__tests__/use-blocker-test.tsx b/packages/react-router-dom/__tests__/use-blocker-test.tsx
@@ -158,6 +158,77 @@ describe("navigation blocking with useBlocker", () => {
     act(() => root.unmount());
   });
 
+  it("handles reused blocker in a layout route", async () => {
+    router = createMemoryRouter([
+      {
+        Component() {
+          let blocker = useBlocker(true);
+          return (
+            <div>
+              <Link to="/one">/one</Link>
+              <Link to="/two">/two</Link>
+              <Outlet />
+              <p>{blocker.state}</p>
+              {blocker.state === "blocked" ? (
+                <button onClick={() => blocker.proceed?.()}>Proceed</button>
+              ) : null}
+            </div>
+          );
+        },
+        children: [
+          {
+            path: "/",
+            element: <h1>Home</h1>,
+          },
+          {
+            path: "/one",
+            element: <h1>One</h1>,
+          },
+          {
+            path: "/two",
+            element: <h1>Two</h1>,
+          },
+        ],
+      },
+    ]);
+
+    act(() => {
+      root = ReactDOM.createRoot(node);
+      root.render(<RouterProvider router={router} />);
+    });
+
+    // Start on /
+    expect(node.querySelector("h1")?.textContent).toBe("Home");
+    expect(node.querySelector("p")?.textContent).toBe("unblocked");
+    expect(node.querySelector("button")).toBeNull();
+
+    // Blocked navigation to /one
+    act(() => click(node.querySelector("a[href='/one']")));
+    expect(node.querySelector("h1")?.textContent).toBe("Home");
+    expect(node.querySelector("p")?.textContent).toBe("blocked");
+    expect(node.querySelector("button")?.textContent).toBe("Proceed");
+
+    // Proceed to /one
+    act(() => click(node.querySelector("button")));
+    expect(node.querySelector("h1")?.textContent).toBe("One");
+    expect(node.querySelector("p")?.textContent).toBe("unblocked");
+    expect(node.querySelector("button")).toBeNull();
+
+    // Blocked navigation to /two
+    act(() => click(node.querySelector("a[href='/two']")));
+    expect(node.querySelector("h1")?.textContent).toBe("One");
+    expect(node.querySelector("p")?.textContent).toBe("blocked");
+    expect(node.querySelector("button")?.textContent).toBe("Proceed");
+
+    // Proceed to /two
+    act(() => click(node.querySelector("button")));
+    expect(node.querySelector("h1")?.textContent).toBe("Two");
+    expect(node.querySelector("p")?.textContent).toBe("unblocked");
+    expect(node.querySelector("button")).toBeNull();
+
+    act(() => root.unmount());
+  });
+
   describe("on <Link> navigation", () => {
     describe("blocker returns false", () => {
       beforeEach(() => {

diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx
@@ -939,7 +939,6 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker {
   let state = useDataRouterState(DataRouterStateHook.UseBlocker);
 
   let [blockerKey, setBlockerKey] = React.useState("");
-  let [blocker, setBlocker] = React.useState<Blocker>(IDLE_BLOCKER);
   let blockerFunction = React.useCallback<BlockerFunction>(
     (arg) => {
       if (typeof shouldBlock !== "function") {
@@ -986,15 +985,15 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker {
   // key of "".  Until then we just have the IDLE_BLOCKER.
   React.useEffect(() => {
     if (blockerKey !== "") {
-      setBlocker(router.getBlocker(blockerKey, blockerFunction));
+      router.getBlocker(blockerKey, blockerFunction);
     }
   }, [router, blockerKey, blockerFunction]);
 
-  // Prefer the blocker from state since DataRouterContext is memoized so this
-  // ensures we update on blocker state updates
+  // 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)
     ? state.blockers.get(blockerKey)!
-    : blocker;
+    : IDLE_BLOCKER;
 }
 
 /**

diff --git a/packages/router/router.ts b/packages/router/router.ts
@@ -1026,8 +1026,11 @@ export function createRouter(init: RouterInit): Router {
 
     // On a successful navigation we can assume we got through all blockers
     // so we can start fresh
-    let blockers = new Map();
-    blockerFunctions.clear();
+    let blockers = state.blockers;
+    if (blockers.size > 0) {
+      blockers = new Map(blockers);
+      blockers.forEach((_, k) => blockers.set(k, IDLE_BLOCKER));
+    }
 
     // Always respect the user flag.  Otherwise don't reset on mutation
     // submission navigations unless they redirect