cognitect / transit-cljs

Transit for ClojureScript
http://transit-format.org
Apache License 2.0
322 stars 20 forks source link

Transit `TaggedValue` should implement `IComparable` #28

Open domkm opened 7 years ago

domkm commented 7 years ago

Does it make sense for Transit BigIntegers and BigDecimals to be comparable?

The specific issue that I ran into is that Transit values cannot be indexed by DataScript:

Uncaught Error: Cannot compare [TaggedValue: n, 37502364230516917834597274526242608522740435501611847496456113203943353843903] to [TaggedValue: n, 37746944873274413711073286601178981016490453790917975016109108374206467768511]
    at cljs$core$compare (core.cljs:2241)
    at datascript$db$cmp_val (db.cljc:288)
    at datascript$db$cmp_datoms_avet (db.cljc:311)
    at datascript$btset$binary_search_l (btset.cljc:72)
    at datascript$btset$_seek (btset.cljc:951)
    at datascript$btset$_slice (btset.cljc:979)
    at Function.datascript.btset.slice.cljs$core$IFn$_invoke$arity$3 (btset.cljc:990)
    at datascript$btset$slice (btset.cljc:985)
    at Function.datascript.btset.slice.cljs$core$IFn$_invoke$arity$2 (btset.cljc:988)
    at datascript$btset$slice (btset.cljc:985)
theronic commented 6 years ago

Maybe raise this on DataScript repo?

tonsky commented 6 years ago

Not related to DataScript at all.

TaggedValue shouldn’t implement IComparable, it’s a sign your transit reader isn’t doing its job. It’s a wrapper to store anything transit doesn’t know how to read. Find what to translate it to, register transit handler and make sure the type you’ll be translating to is comparable. Fix that, and DataScript will be happy.