Skip to content

Commit

Permalink
Fix loop in unstable_useBlocker when used with an unstable blocker fu…
Browse files Browse the repository at this point in the history
…nction (#10652)

* Fix loop in unstable_useBlocker when used with an unstable blocker function

* Remove getBlocker from key generation
  • Loading branch information
brophdawg11 authored Jun 29, 2023
1 parent 6c23794 commit 0ffa3dc
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/fix-blocker-loop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Fix loop in `unstable_useBlocker` when used with an unstable blocker function
44 changes: 44 additions & 0 deletions packages/react-router-dom/__tests__/use-blocker-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,50 @@ describe("navigation blocking with useBlocker", () => {
act(() => root.unmount());
});

it("handles unstable blocker function identities", async () => {
let count = 0;
router = createMemoryRouter([
{
element: React.createElement(() => {
// New function identity on each render
let b = useBlocker(() => false);
blocker = b;
if (++count > 50) {
throw new Error("useBlocker caused a re-render loop!");
}
return (
<div>
<Link to="/about">/about</Link>
<Outlet />
</div>
);
}),
children: [
{
path: "/",
element: <h1>Home</h1>,
},
{
path: "/about",
element: <h1>About</h1>,
},
],
},
]);

act(() => {
root = ReactDOM.createRoot(node);
root.render(<RouterProvider router={router} />);
});

expect(node.querySelector("h1")?.textContent).toBe("Home");

act(() => click(node.querySelector("a[href='/about']")));
expect(node.querySelector("h1")?.textContent).toBe("About");

act(() => root.unmount());
});

describe("on <Link> navigation", () => {
describe("blocker returns false", () => {
beforeEach(() => {
Expand Down
15 changes: 13 additions & 2 deletions packages/react-router/lib/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -972,12 +972,23 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker {
[basename, shouldBlock]
);

// This effect is in charge of blocker key assignment and deletion (which is
// tightly coupled to the key)
React.useEffect(() => {
let key = String(++blockerId);
setBlocker(router.getBlocker(key, blockerFunction));
setBlockerKey(key);
return () => router.deleteBlocker(key);
}, [router, setBlocker, setBlockerKey, blockerFunction]);
}, [router]);

// This effect handles assigning the blockerFunction. This is to handle
// unstable blocker function identities, and happens only after the prior
// 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 !== "") {
setBlocker(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
Expand Down

0 comments on commit 0ffa3dc

Please sign in to comment.