Produces code instead of instant with `read-string`
SevereOverfl0w opened this issue · 10 comments
❯ clj -Sdeps '{:deps {time-literals/time-literals {:mvn/version "0.1.4"}}}'
Downloading: time-literals/time-literals/0.1.4/time-literals-0.1.4.pom from https://repo.clojars.org/
Downloading: time-literals/time-literals/0.1.4/time-literals-0.1.4.jar from https://repo.clojars.org/
Clojure 1.10.1
user=> (require 'time-literals.data-readers)
nil
user=> (read-string "#time/instant \"2020-09-02T12:24:31.740Z\"")
(. java.time.Instant parse "2020-09-02T12:24:31.740Z")
user=>
Generating code seems to be the wrong thing: https://github.com/clojure/clojure/blob/653b8465845a78ef7543e0a250078eea2d56b659/src/clj/clojure/uuid.clj#L11-L13
I'm guessing it happens to work in some contexts, rather than being correct. Maybe it's needed in Cljs?
yes, this was the only way I found to get the data_readers.cljc to work cross platform.
The readme does mention this btw. Since clojure.edn/read-string does work as you'd expect, this limitation seems ok. Unless there are contexts where clojure.core/read-string is preferred? reading code perhaps?
If there is a way to avoid producing code then that would be better all round for sure
I don't know the full scope of the string, but it is trusted. So I'd like to rely on data_readers.
Could you have a .clj implementation which doesn't produce code, and a cljs one which does?
hmm, I thought I had tried that, but couldn't remember.
Just tried again (on branch https://github.com/henryw374/time-literals/tree/work-with-clojure-core-read-string) but I'm finding reading doesn't work with cljs in that case
Caused by: java.lang.IllegalStateException: Attempting to call unbound fn: #'time-literals.data-readers/date
I tried a different fix that defines forms conditionally one at a time and that won't work either. In cider it would seem that dates etc. would first be read by Clojure as java.time.(...) objects, then passed to Clojurescript for evaluation. The fact that there are so many cljs REPL types around makes this more of a can of worms I guess, so I punted on this for now.
As a workaround, I run this macro in a defonce in a namespace where I set up my systems. It will break the Clojurescript reader if you run it in the process that compiles cljs. I guess you can set up a profile/alias for cljs work that doesn't invoke this, but I haven't tried that yet.
https://github.com/euccastro/relembra/blob/master/src/clj/relembra/data_readers.clj
(I tried alter-var-rooting clojure.core/*data-readers* instead, but that wouldn't take effect for some reason; there's a ticket for that in the Clojure Jira, but I can't find it right now.)
Also note that java.time.* objects evaluate to themselves in Clojure, so this trick shouldn't break existing Clojure code that evals the result of read-string as indicated in the README.
before I forget completely... @puredanger says this is likely a bug (in clojurescript? ) see https://clojurians-log.clojureverse.org/clojure-dev/2020-09-15. There is no jira afaik.
This needs taking up with cljs-dev team, raising jira etc which I haven't got round to as yet.
I just submitted a working, tested patch for the above issue https://clojure.atlassian.net/browse/CLJS-3294, so hopefully this gets fixed soon. feel free to upvote that issue ;-)
issue mentioned has been merged, so when the next cljs version is released, I can release the master branch of this project, which fixes the issue
released version 0.1.6 with the fix. note new artifact group: https://clojars.org/com.widdindustries/time-literals