fix(bufferCount): set default value for skip argument, do not emit em… #271
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
…pty buffer at the end
| @@ -33,7 +33,7 @@ class BufferCountSubscriber<T> extends Subscriber<T> { | |||
| const count = (this.count += 1); | |||
| const destination = this.destination; | |||
| const bufferSize = this.bufferSize; | |||
| const startBufferEvery = this.startBufferEvery; | |||
| const startBufferEvery = (this.startBufferEvery == null) ? bufferSize : this.startBufferEvery; | |||
There was a problem hiding this comment.
Some of other operator such as windowCount has default value as 0 instead of null. Not sure if there's consistent recommendation or guidelines, so did not modify default value itself but simply take buffersize if value's not specified.
|
Good catch. I suspect we'll be finding lots of bugs like this as we convert tests over to use the Virtual Time Scheduler and expand testing to cover more scenarios. |
|
Thanks @kwonoj! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
…pty buffer at the end.
Found while try to write micro perf test for bufferCount, when bufferCount does not have explicit skip argument (startBufferEvery parameter) it only emits very first buffer element then completes observable. This PR aligns bufferCount behaves as same as current bufferWithCount (https://github.com/Reactive-Extensions/RxJS/blob/master/doc/api/core/operators/bufferwithcount.md) by set default skip argument to size of buffer if it's not specified.