Skip to content

Commit

Permalink
Remove getBlocker from key generation
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed Jun 29, 2023
1 parent 7220849 commit e189e42
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 15 deletions.
6 changes: 4 additions & 2 deletions packages/react-router-dom/__tests__/use-blocker-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,16 @@ describe("navigation blocking with useBlocker", () => {
});

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>
Expand All @@ -143,8 +147,6 @@ describe("navigation blocking with useBlocker", () => {

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} />);
});

Expand Down
21 changes: 8 additions & 13 deletions packages/react-router/lib/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -972,28 +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);
// 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]);
}, [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(() => {
// 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]);
}, [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 e189e42

Please sign in to comment.