cljs-time.extend Equiv and cljs-time.core/= return different results

[zalky]

Hi, thanks for all the work done on this excellent library!

I'm wondering if there's a reason that the Equiv implementation in cljs-time.extend and cljs-time.core/= return different results?

(require '[cljs-time.extend])

(cljs-time.core/= (t/now) (t/time-now))
;; true

(cljs.core/= (t/now) (t/time-now))
;; false

Looking at the implementation in IEquiv, it seems the behaviour is conditional on the types (in the example above, (instance? goog.date.UtcDateTime other) on L51 would return false), whereas cljs-time.core/= coerces everything to a timestamp and compares that.

Is this intentional?

[zalky]

Also, note that the IComparable implementation uses the cljs-time.core/= approach, but with no type check.

[andrewmcveigh]

I don't remember the exact reason that IEquiv ended up checking for exact types, it didn't start out that way (see c0c43b88f32fd4ccc23246a6d2f1d1248ba2d82e). But I do remember some weirdness around https://github.com/andrewmcveigh/cljs-time/blob/77649a4fac223d2b913dff426e1941f21b58397c/src/cljs_time/extend.cljs#L39 not returning true anymore.

The fact that the functionality is different is not intentional, most likely I forgot cljs-time.core/= even existed. I'd have removed cljs-time.core/= a long time ago, but people were using it.

Not sure that a subtle behaviour change at this point is a great idea though.

[zalky]

I see. Another thing I noticed is that cljs-time.core/equal? and IEquiv are different:

cljs.user> (binding [t/*ms-fn* (constantly (t/*ms-fn*))]
             (let [t1 (t/now)
                   t2 (t/time-now)]
               [(t/equal? t1 t2)
                (t/equal? t2 t1)]))
[true true]
cljs.user> (binding [t/*ms-fn* (constantly (t/*ms-fn*))]
             (let [t1 (t/now)
                   t2 (t/time-now)]
               [(= t1 t2)
                (= t2 t1)]))
[false false]

Also, is there any particular reason that (.equals t1 t2) is not used everywhere?