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

.distinctUntilChanged does not throw when comparer func is missing args #2294

Closed
crunchie84 opened this issue Jan 18, 2017 · 4 comments
Closed

Comments

@crunchie84
Copy link
Contributor

RxJS version:
5.0.3

Code to reproduce:

Rx.Observable.from([1,2,3,4,5,6])
  .map(i => ({ number: i, isEven: i % 2 == 0 }))
  .timestamp()
  .distinctUntilChanged(obj => obj.value.number)
  .subscribe(console.log)

Expected behavior:

This code should throw an error that the comparer function given does not equal the amount of parameters required.

Actual behavior:

Code executes with strange distinctUntilChanged behaviour.

Additional information:

This is more of a migration issue; When migrating from RxJS4 from usage of distinctUntilChanged with a key selector you end up with the above syntax. It does not return any error that you might have done something wrong but instead of the expected distinction of elements it only emits the first value of the complete stream because the keyselector function is passed as comparer function and it actually does not compare anything.

@crunchie84 crunchie84 changed the title .distinctUntilChanged does not throw when comparer func is incorrect format .distinctUntilChanged does not throw when comparer func is missing args Jan 18, 2017
@kwonoj
Copy link
Member

kwonoj commented Jan 18, 2017

This is essentially dupe of #2144, about runtime validation behavior. For this matter I'm on same page with #2144, it is designed to require caller checks signature.

@crunchie84
Copy link
Contributor Author

@kwonoj that issue is exactly the same problem. I tend to agree with the stance taken by the team.

My bigger concern is how to upgrade from RxJs4 towards RxJs5 without running into these kind of hard to trace bugs. It is mentioned in the migration.md but didn't really trigger my spidersense.

Luckily my testsuite is extensive for the Rx part but still the migration took a long time. This might not be the case for everyone 😢 and that combined with these hard to track issues will most likely negatively influence user adoption.

@kwonoj
Copy link
Member

kwonoj commented Apr 16, 2017

I'll use #2153 as umbrella issue to discuss runtime validations.

@kwonoj kwonoj closed this as completed Apr 16, 2017
@lock
Copy link

lock bot commented Jun 6, 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 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants