Non-value passed to custom computed equality check

[chrisdoble]

The documentation for computed's equals option states

This acts as a comparison function for comparing the previous value with the next value. If this function considers the previous and next values to be equal, then observers will not be re-evaluated.

I interpreted this to mean that custom equality functions will only ever be passed values that were returned from the derivation function. Is that correct? I believe I've found a situation where this isn't the case.

A snippet that reproduces the behaviour:

const mobx = require('mobx');
const observable = mobx.observable(0);
const computed = mobx.computed(() => observable.get() + 1, {
  equals: (old, new_) => {
    console.log(`old=${old}, new_=${new_}`);
    return old === new_;
  },
});

// Logs nothing
const dispose = mobx.autorun(() => computed.get());
// Logs "old=1, new_=2"
observable.set(1);
dispose();
// Logs "old=undefined, new_=2"
mobx.autorun(() => computed.get());

undefined was never returned from the derivation function () => observable.get() + 1, so I didn't expect it to be passed to the equality function. After disposing the observer and establishing a new one, I either expected the equality function not to be called, or to be called with 2 and 2.

I think this is happening because ComputedValue::value is assigned undefined when it becomes unobserved (code) and this is then passed to equals when it becomes observed again (code). Perhaps it should be assigned new CaughtException(null) as it is in the constructor (code) which would bypass the equality check (code)?

[urugator]

Seems like bug indeed. Summoning @jamiewinder @mweststrate
Btw the relevant behavior is discussed here (scroll down for more)

[jamiewinder]

I think you're right about the cause and solution to this problem. I'll add a PR to sort this in the next few days if I manage to claw some free time (unless someone beats me to it!).

[chrisdoble]

I've tried to fix this in #1213. Please let me know if I should assign it to anyone in particular.

[mweststrate]

@chrisdoble sorry for the late response! Was kinda drown in github notifications :) Anyway thanks for the thorough analysis, I am pretty sure you are right and the correct behavior imho should be that the comparer indeed shouldn't be called in these cases. Will review asap