-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: change args for useThrottledCallback
and useDebouncedCallback
#132
Conversation
Codecov Report
@@ Coverage Diff @@
## master #132 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 15 32 +17
Lines 111 543 +432
Branches 14 100 +86
==========================================
+ Hits 111 543 +432
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think I'm less convinced that the dependency list shouldn't be the last argument. How do other libraries handle this?
Thinking about this more, I'm wondering if we should move to an options
object here like lodash.
Disagree about options object, since is longer record: useDebouncedEffect(()=>{}, [], 200, 600); vs useDebouncedEffect(()=>{}, [], { delay: 200, maxWait: 600 }); Options object is more obvious though, but most IDE highlight arguments so this should not be an issue. If there would be more options - i would agree, but at leas for now, on my taste, arguments are better solution |
Have no idea, havent watched any other solutions =) Ed: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kk, you've convinced me. Until we have more args this should be fine.
27fd0e3
to
72c491a
Compare
BREAKING CHANGE: `delay` and `deps` arguments are swapped position for `useThrottledCallback` and `useDebouncedCallback` hooks to be aligned with `useCallback` signature.
72c491a
to
c383b58
Compare
# [3.0.0](v2.2.0...v3.0.0) (2021-06-16) ### Bug Fixes * rename useThrottleCallback and useDebounceCallback ([#130](#130)) ([77f66d7](77f66d7)), closes [#129](#129) ### Features * add `maxWait` parameter to `useDebouncedCallback` hook ([#131](#131)) ([600baa8](600baa8)) * change args for `useThrottledCallback` and `useDebouncedCallback` ([#132](#132)) ([131d98e](131d98e)) * new hooks `useDebouncedEffect` and `useDebouncedState` ([#133](#133)) ([1d164ff](1d164ff)) ### BREAKING CHANGES * `delay` and `deps` arguments are swapped position for `useThrottledCallback` and `useDebouncedCallback` hooks to be aligned with `useCallback` signature. * `useDebounceCallback` renamed to `useDebouncedCallback` `useThrottleCallback` renamed to `useThrottledCallback`
🎉 This PR is included in version 3.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
useThrottledCallback
anduseDebouncedCallback
has different arguments order comparing touseCallback
(delay argument is before deps list). It is mentally harder to handle such differences.BREAKING CHANGE:
delay
anddeps
arguments are swapped position foruseThrottledCallback
anduseDebouncedCallback
hooks to be aligned withuseCallback
signature.merge after #131
Checklist