feat(hooks): add cancel functionality to useDebouncedCallback #7965
Conversation
1d6605e to
f191802
Compare
| useCallback( | ||
| (...args: Parameters<T>) => { | ||
| const lastCallback = | ||
| useMemo(() => |
There was a problem hiding this comment.
using useMemo instead of useCallback lets us treat the callback itself as the ref since it's not created every render. Ties the leading and flush state to the callback itself, which means things will reset when options like leading, delay, etc change (which i think is desired)
|
@rtivital think this is ready for your review if you have a moment. Adds cancel functionality and cleans up the code a bit to track state in fewer places as well. |
32de21c to
7517a1f
Compare
- Add cancel method alongside existing flush method - Cancel clears pending timeouts and resets leading state without executing callback - Maintains backward compatibility with existing API - Add comprehensive test coverage (8 new tests) covering: - Basic cancellation preventing execution - Multiple cancel calls safety - Cancel and re-call behavior - Interaction with already executed callbacks - Leading mode reset behavior - Cancel/flush interaction patterns Resolves the need to cancel pending debounced executions in scenarios like component unmounting or input direction changes.
7517a1f to
2d8497b
Compare
| @@ -81,6 +91,29 @@ describe('@mantine/hooks/use-debounced-callback', () => { | |||
| expect(callback).toHaveBeenCalledWith(3); | |||
| }); | |||
|
|
|||
| // Note: this might not be desired but this is what happens | |||
There was a problem hiding this comment.
realized this was the behavior for both of these cases so i thought I'd document it with a test. We could consider doing something differently if we don't like this but it's non-regression
| expect(callback).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('does call again on leading edge if options change and it was already called before the change', () => { |
There was a problem hiding this comment.
the was behaving the opposite way before. If i change any option i don't get a fresh leading edge. My instinct is that it should reset when the options reset but again open to changing it
There was a problem hiding this comment.
Pull Request Overview
This PR adds a cancel method to the useDebouncedCallback hook in order to clear pending timeouts and reset the leading state without executing the callback, while maintaining backward compatibility. Key changes include:
- Introducing a cancel method alongside the existing flush method.
- Refactoring the callback memoization to include internal state (_isFirstCall) and method assignments.
- Updating and adding comprehensive tests to cover various cancel interaction scenarios.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/@mantine/hooks/src/use-debounced-callback/use-debounced-callback.ts | Refactor of callback handling to add cancel functionality and update state management. |
| packages/@mantine/hooks/src/use-debounced-callback/use-debounced-callback.test.ts | New tests covering cancel behavior, multiple cancels, and cancel/flush interactions. |
Comments suppressed due to low confidence (1)
packages/@mantine/hooks/src/use-debounced-callback/use-debounced-callback.ts:60
- [nitpick] It's recommended to document the purpose of the internal '_isFirstCall' property and the default method assignments to ease understanding for future maintainers.
{ flush: () => {}, cancel: () => {}, _isFirstCall: true }
|
|
||
| const lastCallback = Object.assign( | ||
| useCallback( | ||
| const lastCallback = useMemo(() => { |
There was a problem hiding this comment.
[nitpick] The self-referencing pattern via 'currentCallback' (accessing and updating _isFirstCall) can reduce clarity. Consider adding inline comments to clarify the intended state management and the role of _isFirstCall.
Copilot uses AI. Check for mistakes.
|
Thanks! |
Add cancel method alongside existing flush method
Cancel clears pending timeouts and resets leading state without executing callback
Maintains backward compatibility with existing API
Add comprehensive test coverage (8 new tests) covering:
Basic cancellation preventing execution
Multiple cancel calls safety
Cancel and re-call behavior
Interaction with already executed callbacks
Leading mode reset behavior
Cancel/flush interaction patterns
Resolves the need to cancel pending debounced executions in scenarios like component unmounting or input direction changes.