Fork me on GitHub
#cljs-dev
<
2016-06-03
>
mfikes01:06:08

@dnolen: multi-arity returning vars looks cut-n-dry to fix. Attaching a patch to CLJS-1611.

niwinz09:06:54

I'm experimenting a little bit with commonjs -> google closure modules transformations (documented in https://github.com/clojure/clojurescript/wiki/Compiler-Options#foreign-libs) and it works correctly from cljs.build.api/build but when I put the same in the deps.cljs it raises a internal compiler error. Is that expected?

niwinz10:06:12

Absolutly the same :foreign-libs in the build compiler api works as expected and if I put it into deps.cljs raises a internal compiler exception...

mfikes13:06:34

@niwinz: I would expect that transformations would work with upstream foreign deps (I’ve never heard of such a limitation).

niwinz13:06:15

I'm also surprised, but I can't use the deps.cljs approach for expose a commonjs module using :module-type :commonjs

niwinz13:06:48

it only works if I put the :foreign-libs into my final project (not in the library)

mfikes13:06:52

@niwinz: Perhaps you’ve discovered a use case that just hasn’t been exercised yet.

niwinz13:06:08

Sure, how I can help? I can make a simple project that reproduces that...

mfikes13:06:51

@niwinz: Important bit is to not make a project based on lein or boot but instead with the shipping JAR.

niwinz13:06:09

the minimum necessary 😉

niwinz13:06:58

@mfikes: thanks, I'll try to do it asap

mfikes16:06:16

Sweet! Transit for compiler metadata cache is great. (And it’s definitely reliable for this use—Replete’s been using it for 11 months, Planck 6 months, zero issues, extremely fast.)

dnolen17:06:35

@mfikes: yeah looking forward to wrapping this up, it should be very big boost to REPL start up time

dnolen17:06:56

there’s some weird bug at the moment, hopefully will get sorted out soon.

dnolen17:06:13

something about AOT

rohit18:06:12

@dnolen: Based on your suggestion yesterday about using a js array in cljs.reader for creating the collections instead of using apply, I’ve created a Jira ticket for it. It includes a patch and some benchmarks - http://dev.clojure.org/jira/browse/CLJS-1665

dnolen19:06:14

@rohit looks like a nice win overall

rohit19:06:44

@dnolen: Thanks. one change i’d like to make is to create an .fromArray function for a List like all other collection types. I haven’t done that as I wanted to keep this patch just to cljs.reader.

dnolen19:06:03

Clojure doesn't do that so I'm not sure worth pursuing

dnolen19:06:24

The array adopting stuff is modeled after Clojure

rohit19:06:36

@dnolen: ah ok. makes sense then. cheers!

rohit19:06:39

@dnolen: looking at the core library, I noticed that there a lot of functions exposed to the user which are not in the official API and are not documented per se. Should they be marked as something like “internal use only”?

dnolen19:06:11

would need to see a list of these

rohit19:06:22

cool. i’ll do that and ping you once I have that.

dnolen19:06:10

with transit analysis caches REPLs are indeed significantly faster to start

dnolen19:06:35

for my om.next devcards stuff Figwheel starts in half the reported time

bhauman19:06:49

@dnolen: did you mention a while back that we might add a :signature like key in here

bhauman19:06:11

I'm about to start working on stamping builds with dependencies

dnolen19:06:35

yeah we should look into that

richiardiandrea20:06:07

David I get a weird warning while upgrading to 1.9.0-alpha4 + 1.9.14:

WARNING: Use of undeclared Var cljs.analyzer/_ at line 2366 /.../git/cljs-repl-web/irc/-5y5vor/main.out/cljs/analyzer.cljc
I don't have a minimal repro repo at the moment, if you need it I can produce it

anmonteiro20:06:42

@richiardiandrea: that’s fixed in master

richiardiandrea20:06:01

thanks @anmonteiro sorry for the noise then

mfikes20:06:40

@dnolen: 👍 I tried master on our Figwheel-based setup and it works, with .cache.json files appearing. No noticeable speedup (it was already compiling in about 3 seconds, so perhaps not enough volume of code to matter.)

dnolen20:06:55

@mfikes: interesting even on the second launch?

mfikes20:06:27

Yes, the first compile is on the order of 20–30 seconds. Subsequent launches involve a compile of about 3 seconds.

mfikes20:06:06

(I’m thinking that perhaps this just isn’t a large enough project to benefit from the Transit conversion.)

dnolen20:06:31

yeah I think load / compile time will dominate

dnolen20:06:46

I’m thinking this should only shave off 1/2 second or something

richiardiandrea20:06:16

in my case is the same, the compile time is so high that I cannot see the difference in launching the repl (but good that it is there!)

mfikes20:06:31

(I’m not actively using Figwheel all of the time, so I’m probably not the best to offer feedback other than a simple “smoke test” that it didn’t horribly break. 🙂 )

dnolen20:06:12

yeah probably more noticeable to Quick Start folks

dnolen20:06:41

one thing we haven’t done is enable :parallel-build for REPLs

dnolen20:06:04

which would also give us parallel analysis loading

richiardiandrea20:06:56

confirm that the warning went away with 1.9.36, going back to the performance topic, http://clojurescript.io compiles in:

real	1m57.961s
user	2m49.489s
sys	0m2.619s
with :parallel-build on

dnolen20:06:51

@richiardiandrea: thanks for the confirm

jr21:06:29

Has this been reported yet?

ERROR: Not supported: class clojure.lang.Var at file src/path/to/file.cljc
    data: {:file “src/path/to/file.cljc", :from :boot-cljs}
       adzerk.boot_cljs.util.proxy$java.lang.Throwable$ff19274a: java.lang.Exception: Not supported: class clojure.lang.Var
       adzerk.boot_cljs.util.proxy$java.lang.Throwable$ff19274a: Not supported: class clojure.lang.Var

jr21:06:06

This is using transit-clj and latest cljs. I can reproduce with the following code in a cljc file

(def foo "foo")
#?(:clj (defmacro bar [] (:name (meta (var foo)))))
(def bazz (bar))

juhoteperi21:06:28

@jr: Can you reproduce this without boot-cljs?

jr21:06:44

I’ll set up a new project and test

jr21:06:04

seems like a transit serialization issue and not boot fwiw

juhoteperi21:06:50

Yeah, doesn't look like something boot-cljs would cause, but as it mangles the exceptions it is a bit harder to debug the problem

juhoteperi21:06:33

Did you change the file path? 😄

jr21:06:17

proprietary project 😛

dnolen21:06:27

@jr going to need something minimal

dnolen21:06:39

but nothing about the error so far seems to have anything to do with Transit

richiardiandrea22:06:26

so in replumb the only failing test is with the form: (find-doc "[^(]newline[^s*]"), I need to investigate

richiardiandrea22:06:47

it does not return the doc we were expecting

richiardiandrea22:06:44

maybe the new 1.9 version got rid of some of the -newline functions

jr22:06:48

I’ll come up with a minimal project to reproduce. In the mean time here is the full stack trace, which indicates (to me) that something about emitting a serialized var is broken https://gist.github.com/JacobNinja/30a42ee9d4517190acfe95e2d21ab4a9

richiardiandrea22:06:00

false alarm on my side, silly difference in the output

dnolen22:06:31

@jr or that just looks like bad code that never should have worked and you just got lucky

jr22:06:44

could be

dnolen22:06:50

somehow a var is leaking into the analysis cache

dnolen22:06:05

probably due to a macro incorrectly splicing it in somewhere

dnolen22:06:17

there’s no transit handler for it so it blows up

dnolen22:06:27

@jr still I agree that there is a problem here - you need to be able to disable transit encoding if necessary

jr22:06:45

fwiw, the breaking code is similar to this, which imports vars from another namespace https://github.com/ztellman/potemkin/blob/master/src/potemkin/namespaces.clj#L11

dnolen22:06:40

yeah I don’t really want to know why it happens

dnolen22:06:10

the problem is we didn’t catch it before so people got away with it

dnolen22:06:27

and now we need a way to turn off faster encoding/decoding

jr23:06:05

Is transit supposed to emit EDN for analysis? Seems like it is emitting JSON based on the stacktrace

dnolen23:06:28

you’re misinterpreting what I said above - probably wasn’t clear enough

dnolen23:06:36

we used to dump EDN for the analysis caches

dnolen23:06:41

and we still do

dnolen23:06:50

we dump JSON now if you have transit-clj as a dep

richiardiandrea23:06:14

just forgot to say that all the tests now pass in replumb with cljs 1.9.36, thanks David!

jr23:06:31

dnolen: Ah that makes sense now. I read the ticket description and thought EDN is the emitted format for transit. Thanks for the clarification

jr23:06:48

Is JSON faster than EDN in transit? what’s the reason for changing formats?