distinctUntilKeyChanged throws exception when one of the values on the stream is null/undefined.

[mdarefull]

RxJS version:
5.5.2
Code to reproduce:
Apply it to a stream that produces null/undefined values.

Expected behavior:
If both values are undefined or null both can be considered the same. We should just perform the key inspection if both values are defined.

Actual behavior:
It will throw an exception about trying to access a property on undefined. This essentially makes the operator useless if there can be null/undefined values on the stream.

Additional information:
Let me know if you need to discuss the issue further.

[cartant]

IMO the effected error is appropriate, as the behaviour of distinctUntilKeyChanged is predicated on a key being accessible.

If you require null or undefined values to be treated in a specific way, use distinctUntilChanged, instead, and specify a compare function:

distinctUntilChanged((a, b) => {
  if (a === b) return true;
  if (a && b && a.someKey === b.someKey) return true;
  return false;
})

[mdarefull]

We ended with that same snippet on our project, literally everywhere where we were previously using distinctUntilKeyChanged.

Theoretically, you might be correct, but I found it, from an usability point of view, a defect and a design flaw. We might not have control over the value emitted on our different flows or even, we might not even be sure if they will ever emit null or undefined.
I found 'distinctUntilKeyChanged' to be of use when the references may be different but the underlying objects are considered the same, usually based on its id.

There can be countless use-cases on which a flow can emit null/undefined, usually flagging a 'not value', like when selecting elements, as in our case. Furthermore, if we would want to make our code robust and aren't sure if our flows would ever emit such value, there's no easy way to protect our code, because filtering it might be unacceptable so we would need to get rid of 'distinctUntilKeyChanged' and implement that same function all over our projects, undermining the sole reason 'distinctUntilKeyChanged' was created at first place.

Even more, If both objects are the same reference, THEY WILL have the same key, and if one value is undefined and the next one is well-defined, definitely they will be different.
So I can't think of any use-case in which including that on the 'distinctUntilKeyChanged' implementation wouldn't be an improvement, even more, it would make the operator more robust.

All of the above, of course, IMHO.

[cartant]

I'm presuming that you aren't repeating the actual snippet wherever distinctUntilChanged is used and are instead using a named compare function. You can further simplify the situation be creating a user-land operator. Making it easier to do so was one of the motivations for the changes in v6.

Just create a pipeable operator with an appropriate name - distinctUntilRobustKeyChanged or whatever - and use that instead, passing it to pipe. If you are using a version of RxJS that predates 5.5, just pass the pipeable operator to let, instead.

The reasons I believe the behaviour you are seeking is application-specific are as follows:

• Not everyone has streams in which null and undefined are expected values. And my preference is for errors to be thrown as close to the source as possible. I don't want to be poking around in the UI or a complicated, composed observable trying to figure out why something is null or undefined. I'd like it to explode early.

• The docs for distinctUntilKeyChanged are clear:

  ... that are distinct by comparison from the previous item, using a property accessed by using the key provided to check if the two items are distinct

  That is, the value of the key is used to determine equivalence. If null and undefined are to receive special handling - because it's known that an attempt to access the key will effect an error - precisely what equivalence means would need to be spelled out. If two elements have keys that are inaccessible are they equivalent or not?

  Also, the current behaviour of distinctUntilKeyChanged is easily inferred: null or undefined elements will effect an error - which is easy to understand. If they receive special handling, inferring whether or not elements with inaccessible keys are considered equivalent is more difficult.

I think the current API is simple, easy to understand and easy to extend with a user-land operator.

[kwonoj]

I'm with @cartant here, especially we have core api surface around distinct* for supporting different usecases already as suggested above.

[lock]

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.