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

Min(comparer) emits comparer result value instead of original stream value #1892

Closed
gluck opened this issue Aug 22, 2016 · 3 comments
Closed
Labels
bug Confirmed bug help wanted Issues we wouldn't mind assistance with.

Comments

@gluck
Copy link
Contributor

gluck commented Aug 22, 2016

RxJS version:
5.0.0-beta.11
Code to reproduce:
Observable.range(1,2).min((a,b) => a > b ? -1 : a < b ? 1 : 0)
Expected behavior:
next(2) (reverse comparer)
Actual behavior:
next(1 | -1) (the comparer result is emitted instead)
Additional information:
existing rxjs 4 unit test showing expected behavior: min with comparer non-empty
existing rxjs 5 unit test showing the wrong behavior: should handle a constant predicate on observable with many values - 42 is emitted instead of some value from original observable.

I could work on a PR, but checking first that it's not an horrible mistake on my side, or an on-purpose change.

Thx.

@benlesh benlesh added the bug Confirmed bug label Aug 22, 2016
@kwonoj
Copy link
Member

kwonoj commented Aug 22, 2016

If you have fix for PR, welcome to have one. 👍

@gluck
Copy link
Contributor Author

gluck commented Aug 23, 2016

I'll work on one after vacation (2 weeks from now).

@kwonoj kwonoj added the help wanted Issues we wouldn't mind assistance with. label Aug 23, 2016
gluck added a commit to gluck/RxJS that referenced this issue Sep 6, 2016
min (&max) operators have now the expected behavior when used with a comparer:
- previously they were behaving like _reduce_ and emitting (last) comparer return value
- they now emit original observable min/max value by comparing the comparer return value to 0
gluck added a commit to gluck/RxJS that referenced this issue Sep 7, 2016
max operator have now the expected behavior when used with a comparer:
- previously was behaving like _reduce_ and emitting (last) comparer return value
- now emits original observable max value by comparing the comparer return value to 0
- fixes ReactiveX#1892
gluck added a commit to gluck/RxJS that referenced this issue Sep 7, 2016
min operator have now the expected behavior when used with a comparer:
- previously was behaving like _reduce_ and emitting (last) comparer return value
- now emits original observable min value by comparing the comparer return value to 0
- fixes ReactiveX#1892
gluck added a commit to gluck/RxJS that referenced this issue Sep 7, 2016
max operator have now the expected behavior when used with a comparer:
- previously was behaving like _reduce_ and emitting (last) comparer return value
- now emits original observable max value by comparing the comparer return value to 0
- fixes ReactiveX#1892
gluck added a commit to gluck/RxJS that referenced this issue Sep 7, 2016
min operator have now the expected behavior when used with a comparer:
- previously was behaving like _reduce_ and emitting (last) comparer return value
- now emits original observable min value by comparing the comparer return value to 0
- fixes ReactiveX#1892
gluck added a commit to gluck/RxJS that referenced this issue Sep 8, 2016
min operator have now the expected behavior when used with a comparer:
- previously was behaving like _reduce_ and emitting (last) comparer return value
- now emits original observable min value by comparing the comparer return value to 0
- fixes ReactiveX#1892
@kwonoj kwonoj closed this as completed in f454e93 Sep 9, 2016
kwonoj added a commit that referenced this issue Sep 9, 2016
fix(min/max): do not return comparer values (#1892)
@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Confirmed bug help wanted Issues we wouldn't mind assistance with.
Projects
None yet
Development

No branches or pull requests

3 participants