Use getTime() to check for Date equality

[davetapley]

Intended outcome:

When updating observables from an API response which contains timestamps,
if a timestamp in the response is unchanged since the last time I updated,
I don't want autorun to trigger.

Actual outcome:

Because I cast the timestamps to a JS Date,
autorun is triggered,
even if the getTime() is the same.

How to reproduce the issue:

Open console on: https://codesandbox.io/s/autorun-new-date-2np493?file=/src/index.js

Observe:

1. autorun doesn't log when clicking Finshed/Unfinished if already finished/unfinished
2. 'Set now' buttons will always autorun because time is changing
3. 'Set same' buttons differ in behavior: if 'Date' then will always autorun, if 'Time' then won't.

I'd argue that for 99.9999% of use cases you wouldn't want autorun to trigger,
because the actual 'value' hasn't changed.

See also:

• Use getTime() to check for Date equality mobx-state-tree#2080

[urugator]

Do the check before mutating the observable.
The default comparator is === , which is fast, easy to explain and isn't surprising:

const date = new Date(whatever)
o.date = date;
assert(date === o.date); // always true

EDIT: Correction, default comparer seems to be Object.is

mobx/packages/mobx/src/utils/comparer.ts

Lines 19 to 25 in 91adf79

	function defaultComparer(a: any, b: any): boolean {
	if (Object.is) {
	return Object.is(a, b)
	}
	return a === b ? a !== 0 || 1 / a === 1 / b : a !== a && b !== b
	}

but the point is the same

[davetapley]

@urugator

Do the check before mutating the observable

Well, sure that would work, but I think MobX should do it internally because:

a. For 99.9999% of use cases you wouldn't want autorun to trigger in this case, and:
b. It's more consistent with how other primitives work (at least, bool, string and number, which I tested here)

---

I'm not sure how your snippet ⬇️ fits in?

const date = new Date(whatever)
o.date = date;
assert(date === o.date); // always true

Yes, that is true, but what's relevant is:

const date = new Date(whatever)
o.date = date; // triggers autorun ✅
o.date = date; // does not trigger autorun ✅
o.date = new Date(whatever) // triggers autorun ⛔️

A patten which is quite likely if whatever comes from an external API.

[urugator]

triggers autorun

Autorun doesn't do any value diffing. The comparison is done when setting a value and if it's equal it's completely ignored - it's not set and autorun isn't notified. Autorun is guaranteed to run if state changed, so we can't mutate state without notifying autorun, we must do both or neither.

I'm not sure how your snippet ⬇️ fits in?

It's demonstrating an expectation, which would be broken if we would use a different comparison for date objects. The assigment operation would have a surprising behavior.

For 99.9999% of use cases you wouldn't want autorun to trigger in this case,

Can be said about any value like object, both builtin or proprietary. This also includes collections of value objects (array of dates etc). Btw we expose comparer.structural, which you can use:

mobx/packages/mobx/src/utils/eq.ts

Line 47 in 91adf79

// Strings, numbers, regular expressions, dates, and booleans are compared by value.

with how other primitives work

Date isn't primitive, similary to new Number(0), new Boolean(true). What's more Date is mutable, so if I assign date object somewhere and then mutate it, the expectation is that the assigned date is mutated as well.

[davetapley]

Thanks for reply / clarifications @urugator.

I put most of my thoughts here: mobxjs/mobx-state-tree#2080 (comment)

Thanks for mentioning comparer.structural, I didn't know about that.