fix(throttleTime): ensure the spacing between throttles is always at least the throttled amount #5687
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cartant
approved these changes
Sep 3, 2020
spec/operators/throttleTime-spec.ts
Outdated
| const e1subs = ' ^------------------!'; | ||
| const t = time(' ----| '); | ||
| // ----|---|----|---|---| | ||
| const expected = '-----y--------x---x---|'; |
There was a problem hiding this comment.
question/nitpick: Should this really delay/throttle the completion of the source? This isn't the behaviour that I would have expected. Whatever the case, this test's description doesn't seem to describe what it's actually testing, IMO. If this is how it's supposed to behave, I think the description should explicitly state that complete notifications that are received whilst throttling will be delayed.
There was a problem hiding this comment.
Ah... yeah, I think if it has to trailing value we should probably complete it.. I'll make that change and merge.
…least the throttled amount Works to align the behavior with expectations set by lodash's throttle - Updates tests - Ensures trailing throttle will wait to notify and then complete - Ensures that every time we emit a value a new throttle period starts fixes ReactiveX#3712 related ReactiveX#4864 fixes ReactiveX#2727 closes ReactiveX#4727 related ReactiveX#4429
benlesh
force-pushed
the
Issue-3712-throttleTime-ensure-proper-spacing
branch
from
cf7c5e5 to
d256401
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Works to align the behavior with expectations set by lodash's throttle
fixes #3712
related #4864
fixes #2727
closes #4727
related #4429