Skip to content

Commit

Permalink
Fix issue with reused blockers on subsequent navigations (#10656)
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 authored Jun 30, 2023
1 parent 775bff9 commit cbda9cf
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 8 deletions.
6 changes: 6 additions & 0 deletions .changeset/fix-reused-blocker.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"react-router": patch
"@remix-run/router": patch
---

Fix issue with reused blockers on subsequent navigations
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
71 changes: 71 additions & 0 deletions packages/react-router-dom/__tests__/use-blocker-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
9 changes: 4 additions & 5 deletions packages/react-router/lib/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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;
}

/**
Expand Down
7 changes: 5 additions & 2 deletions packages/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit cbda9cf

Please sign in to comment.