ReactiveX / rxjs

A reactive programming library for JavaScript
https://rxjs.dev
Apache License 2.0
30.7k stars 3k forks source link

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

Closed crunchie84 closed 7 years ago

crunchie84 commented 7 years ago

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.

kwonoj commented 7 years ago

This is essentially dupe of https://github.com/ReactiveX/rxjs/issues/2144, about runtime validation behavior. For this matter I'm on same page with #2144, it is designed to require caller checks signature.

crunchie84 commented 7 years ago

@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 :cry: and that combined with these hard to track issues will most likely negatively influence user adoption.

kwonoj commented 7 years ago

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

lock[bot] commented 6 years ago

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.