Fix useLeaveDetection to always invoke latest callback
#123
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
useLeaveDetectionhook currently sets up themouseleaveevent listener only once (due to the emptyuseEffectdependency). This means the hook always invokes the very firstonLeavecallback passed to it, rather than the latest one.haiku/lib/hooks/useLeaveDetection.ts
Lines 3 to 12 in 259ab50
Now, this works in the documentation example because it uses the callback approach to update the state.
haiku/docs/src/components/Demo/LeaveDetection.js
Lines 5 to 6 in 259ab50
However, if the state is updated directly by passing a value, then the hook breaks, because it fails to invoke the latest callback everytime the
mouseleaveevent occurs, instead it keeps calling the initial callback (() => setLeaveCount(0 + 1)).A simple fix would be to add
onLeaveto theuseEffectdependency array:export function useLeaveDetection( onLeave: (this: HTMLElement, ev: MouseEvent) => any, ) { useEffect(() => { document.documentElement.addEventListener('mouseleave', onLeave); return () => document.documentElement.removeEventListener('mouseleave', onLeave); - }, []); + }, [onLeave]); }While this works, it's not ideal. Each time a new callback is passed, a new event listener is set up. Since it's easy to pass a new callback on every render, this places the burden of memoizing the callback on the user, which doesn’t make for a great API.
This PR updates the implementation to fix the stale callback issue while ensuring the listener is set up only once. I've also added a comprehensive set of tests to verify that everything works as expected.