Skip to content
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

throttle and throttleTime broken for single values when trailing is true. #2859

Closed
Parakleta opened this issue Sep 24, 2017 · 1 comment · Fixed by #4564
Closed

throttle and throttleTime broken for single values when trailing is true. #2859

Parakleta opened this issue Sep 24, 2017 · 1 comment · Fixed by #4564
Assignees
Labels
bug Confirmed bug

Comments

@Parakleta
Copy link

Parakleta commented Sep 24, 2017

RxJS version:

5.4.3

Code to reproduce:

const after100 = ()=>Observable.interval(100).first();
Observable.interval(500).take(5)
    .throttleTime(100, undefined, {trailing;true})
    .subscribe(x=>console.log(x));
Observable.interval(500).take(5)
    .throttle(after100, {trailing:true})
    .subscribe(x=>console.log(x));
Observable.interval(500).take(5)
    .throttle(after100, {leading:true, trailing:true})
    .subscribe(x=>console.log(x));

Expected behavior:

0....1....2....3....4|   <- source stream
.0....1....2....3....4|
.0....1....2....3....4|
0....1....2....3....4|

Actual behavior:

.....................|
.....................|
00...11...22...33...4|

Additional information:

The changes introduced by #2465 were sorely needed but somewhat botched. The handling of the trailing value is different between throttle and throttleTime and wrong in both cases. In both versions if leading is disabled then the first value is discarded despite the fact that it might actually be the trailing value. In the throttle case there is also the problem that for some reason if the leading and trailing values are both enabled it keeps the first value and emits this for both (duplicating the value).

I strongly recommend that #2749 is replicated to cover the throttle version as well and merged as soon as possible. It resolves this issue, as well as #2727 which it was written to solve, and also #2466.

@benlesh benlesh self-assigned this Sep 25, 2017
@benlesh benlesh added the bug Confirmed bug label Sep 25, 2017
@benlesh benlesh mentioned this issue Sep 25, 2017
micha149 pushed a commit to micha149/rxjs that referenced this issue Feb 27, 2018
BREAKING CHANGES: This changes the behavior of throttle, in particular
throttling with both leading and trailing behaviors set to true, to more
closely match the throttling behavior of lodash and other libraries.
Throttling now starts immediately after any emission from the
observable, and values will not be double emitted for both leading and
trailing values.

fixes ReactiveX#2859
@tim-kuteev
Copy link

In both versions if leading is disabled then the first value is discarded despite the fact that it might actually be the trailing value.

Is there any chance that this one will be fixed the near future?

MatthiasKunnen added a commit to MatthiasKunnen/rxjs that referenced this issue Feb 14, 2019
…nabled

Emit single value with leading disabled, trailing enabled.

Closes ReactiveX#2859 and ReactiveX#4491.
benlesh pushed a commit that referenced this issue Apr 23, 2019
* test(throttleTime): test single value with trailing enabled

Test if throttleTime emits when only a single value is emitted from the
source with leading enabled/disabled and trailing enabled.

* fix(throttleTime): fix single value with leading disabled, trailing enabled

Emit single value with leading disabled, trailing enabled.

Closes #2859 and #4491.

* chore: improve test descriptions

And remove an asDiagram call made with the same argument.

Signed-off-by: Matthias Kunnen <[email protected]>
BioPhoton pushed a commit to BioPhoton/rxjs that referenced this issue May 15, 2019
…#4564)

* test(throttleTime): test single value with trailing enabled

Test if throttleTime emits when only a single value is emitted from the
source with leading enabled/disabled and trailing enabled.

* fix(throttleTime): fix single value with leading disabled, trailing enabled

Emit single value with leading disabled, trailing enabled.

Closes ReactiveX#2859 and ReactiveX#4491.

* chore: improve test descriptions

And remove an asDiagram call made with the same argument.

Signed-off-by: Matthias Kunnen <[email protected]>
@lock lock bot locked as resolved and limited conversation to collaborators May 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants