Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix loop in unstable_useBlocker when used with an unstable blocker function #10652

Merged
merged 2 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
42 changes: 42 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,48 @@ describe("navigation blocking with useBlocker", () => {
act(() => root.unmount());
});

it("handles unstable blocker function identities", async () => {
router = createMemoryRouter([
{
element: React.createElement(() => {
// New function identity on each render
let b = useBlocker(() => false);
blocker = b;
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);
// TODO: Unsure if there's any way to detect the infinite render loop
// here? Right now without the fix the this test just hangs...
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
20 changes: 18 additions & 2 deletions packages/react-router/lib/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -974,10 +974,26 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker {

React.useEffect(() => {
let key = String(++blockerId);
setBlocker(router.getBlocker(key, blockerFunction));
// Since blockerFunction may change on re-renders (either based on a
// controlled input value changing or based on the calling context not
// leveraging React.useCallback()), we separate the key generation from
// the blockerFunction assignment. All blockers will start in a
// non-blocking idle/unblocked state, and then the effect below will
// assign the blockerFunction to the keyed blocker initially and upon
// changes to the blocker function
setBlocker(router.getBlocker(key, () => false));
setBlockerKey(key);
return () => router.deleteBlocker(key);
}, [router, setBlocker, setBlockerKey, blockerFunction]);
}, [router, setBlocker, setBlockerKey]);

React.useEffect(() => {
// Assign the blockerFunction only after the prior effect setBlockerKey
// has taken effect so we don't get an orphaned blockerFunction in the
// router with a key of ""
if (blockerKey !== "") {
setBlocker(router.getBlocker(blockerKey, blockerFunction));
}
}, [blockerFunction, blockerKey, router]);

// Prefer the blocker from state since DataRouterContext is memoized so this
// ensures we update on blocker state updates
Expand Down