-
Notifications
You must be signed in to change notification settings - Fork 27
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
Don't call function later if leading is true #24
Comments
There's a second issue with this use case: the 5 second timeout seems to count from the last time the event was triggered, rather than the last time it was run. So if the user types for 12 seconds, an event gets fired right away, then again about 5 seconds after the user finishes typing, i.e. 17 seconds after they started. Should that be expected? |
Yeah that's different from what is expected, it's missing a I updated this package and published it under |
The docs lay out this expectation accordingly: https://github.com/bjoerge/debounce-promise#with-leadingtrue The first return value is the value of the first invocation, immediately, and all subsequent are resolved with the last value, after the delay. This may be more appropriate for cases like e.g autosaving a form. It can happen instantly, but we also want to capture the very last call so we don't lose data |
When I set
leading: true
, I'd expect the function to only be called when it is actually triggered.Consider a typing indicator for a chat application. I'd like to send the indicator event as soon as the user starts typing, and resend it if they are still typing after every 5 seconds. Currently, if they start typing and stop after 2 seconds, it will send an event on the leading edge (good), then queue the remaining events and send a second one at the end of 5 seconds, even though by then the user isn't typing anymore.
With
leading: true
, does it make sense to expect all events that can't be sent on the leading edge to be ignored? If there are good use cases to the contrary, would it make sense to add an option to accommodate both?The text was updated successfully, but these errors were encountered: