Fork me on GitHub
#cljs-dev
<
2019-06-10
>
kommen11:06:16

related to this: https://clojure.atlassian.net/projects/CLJS/issues/CLJS-3097 I was caught of guard that tools.reader is aot compiled and bundled with clojurescript takes presedence over anything one specifies in their deps.edn when using the clojurescript jar, but of course this does not apply when using clojurescript as a :local/root or :git/url dependency

kommen11:06:33

I guess the reason for the bundling tools.reader is to make it easier to use without any clojure dependency managment?

dnolen13:06:34

As far I could tell Clojure has no good way for specifying what to AOT and what not to

dnolen13:06:43

AOTed ClojureScript cuts time to load by more than half

dnolen13:06:55

I believe there's a sources JAR if you want to avoid these issues

dnolen13:06:51

also I'm not sure what you're saying above is completely true? If I recall it's just non-deterministic

alexmiller13:06:59

AOT intrinsically relies on transitively compiling all code. As CLJS depends on tools.reader, really both must be aot'ed.

alexmiller13:06:44

as a general rule, putting jars in Maven that include other deps is dirty as it creates issues like the one above

alexmiller13:06:34

ideal solutions would be to depend on the aot-classifier version of tools.reader as an external dependency (this has lots of other ramifications for packaging and deployment to be weighed) or to vendor the library code into your own namespaces so there is no conflict (with the consequence that you can't override the tools.reader version). Clojure does basically the latter for example with the ASM library.

alexmiller13:06:44

all of these have tradeoffs. there are no easy answers.

dnolen13:06:10

yeah, my thinking was that we release fairly often and bump tools.reader - also tools.reader has a been quite good about not introducing breaking changes

alexmiller13:06:49

yeah, cljs is usually pretty up to date on it so it's not common to need an override

dnolen13:06:13

but of course there are cases where you might need a newer tools.reader than the one we specify - I think the sources JAR is the way to go if you end up there

kommen15:06:50

thank you both! now that I’m aware of the behaviour it is no big deal for us. but I was super confused that clj -Stree as well as -Spath showed a different tools.reader version than what was actually loaded

kommen15:06:00

at least we never encountered it behaving differently than I was saying in various configurations, so I assumed it is deterministic

alexmiller15:06:17

yeah, that's the general reason not to package other deps inside a lib artifact

mfikes22:06:23

@lee One thought I've pondered with respect to the ClojureScript scripts: What if they were all just Clojure programs, driven by top level deps.edn aliases? So clj -A:test instead of script/test I bet that would be a lot of work and the scripts would perhaps end up being more difficult to read, but it is an interesting thought.

lread23:06:30

As I bash my head (pun intended!) against the current scripts that thought occurred to me as well!

lread01:06:17

A pro for the deps.edn would be no more platform dependent classpaths

lread01:06:14

And with the scripts in Clojure, we’d get rid of the Git Bash on Windows approach.

lread01:06:34

I would, though, like to get back to some other distractions, so maybe I’ll just finish up what I’ve started and come back to this idea at some later date.

lread10:06:35

hmmm, did not know PowerShell is now cross platform

lread10:06:41

I guess we’d want to figure out what problems we are trying to solve and weigh the various options

lread20:06:59

honestly @U04VDQDDY, the thing that is slow for this bash script work is verification. The scripts just take a while to run. I’ve scripted the script runs but I am currently manually checking that results are as expected.

lilactown22:06:15

how bad would it be to put

(when (exists? js/Symbol)
  (extend-protocol IPrintWithWriter
    js/Symbol
    (-pr-writer [sym writer _]
      (-write writer (str "\"" (.toString sym) "\"")))))
in a library where users are expected to use React?