search

cognitect / transit-cljs

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

Add support for uuid? #41

Closed mfikes closed 2 years ago

mfikes commented 6 years ago

There is code in transit-cljs that supports transparently treating core ClojureScript UUID types and the Transit-specific UUID type as being equivalent.

Recently, cljs.core/uuid? was added, but it returns false for Transit-specific UUIDs.

Perhaps this could be fixed by doing something like (extend-type ty/UUID IUUID) in the Transit cljs core namespace.

swannodette commented 6 years ago

Good idea

swannodette commented 6 years ago

I think we should test that the protocol exists (defprotocol generates a named JavaScript function) and only then extend. Then we can maintain compatibility with versions of ClojureScript without this protocol.

swannodette commented 6 years ago

See also #18

souenzzo commented 5 years ago

Oneline to reproduce clj -Srepro -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.439"} com.cognitect/transit-cljs {:mvn/version "0.8.256"}}}' -m cljs.main -re node -e "(require '[cognitect.transit :as t]) (let [data (random-uuid) data' (t/read (t/reader :json) (t/write (t/writer :json) data))] (prn {:misc (mapv (juxt identity uuid? type) [data data']) :eq? (= data data')}))"

https://github.com/cognitect/transit-cljs/blob/master/README.md#contributing

Because transit is incorporated into products and client projects, we prefer to do development internally and are not accepting pull requests or patches.

Is it still true? Even if not merged, a pull request with implementation / testing would speed up the process?

swannodette commented 5 years ago

@souenzzo thanks for the bump, will take a look at this tomorrow.

harold commented 5 years ago

First, thanks for this library, it is a key piece of infrastructure that we use every single day; and if it didn't exist we would have to start by building it.

I encountered this:

dev:cljs.user=> x
#uuid "987126fe-7dbb-40b0-9bbe-004a4611f2dc"
dev:cljs.user=> (uuid? x)
false
dev:cljs.user=> (type x)
#object[Transit$UUID]

Thanks for your time and consideration.

antonmos commented 5 years ago

(extend-type com.cognitect.transit.types/UUID IUUID) does work but it would be great to get it fixed in the core. Thanks!

conjurernix commented 2 years ago

This looks like it hasn't been fixed yet. When reading UUIDs from transit returns something of type Transit/UUID, and the predicate uuid? returns false. What's worse is that they print the same and it took me a while to realise what's wrong.

swannodette commented 2 years ago

Fixed in master.