This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
- # arachne (2)
- # aws (5)
- # aws-lambda (5)
- # beginners (4)
- # boot (25)
- # cljs-dev (270)
- # cljsjs (1)
- # cljsrn (72)
- # clojars (5)
- # clojure (201)
- # clojure-belgium (5)
- # clojure-brasil (4)
- # clojure-italy (2)
- # clojure-korea (2)
- # clojure-russia (24)
- # clojure-spec (24)
- # clojure-uk (22)
- # clojurebridge (1)
- # clojurescript (125)
- # cloverage (3)
- # cursive (41)
- # datomic (37)
- # dirac (4)
- # emacs (2)
- # hoplon (421)
- # lein-figwheel (1)
- # leiningen (5)
- # luminus (2)
- # mount (1)
- # off-topic (18)
- # om (44)
- # om-next (4)
- # onyx (44)
- # pedestal (3)
- # proton (9)
- # re-frame (21)
- # reagent (21)
- # ring-swagger (12)
- # specter (9)
- # sql (2)
- # untangled (62)
- # vim (16)
@juhoteperi nice work on the GCC module stuff. I’m looking over the patch. looks “easier” than initially thought
although “easier” is relative, I imagine how much work you put into this 🙂
at least the diff is small if that’s some measure
diff is relatively small, but time used per line is quite high 😄
I’ve got a few questions:
js-transforms has been changed to do a little less work. is this because all modules now need to be processed in 1 go?
2. did you build the compiler and test it with the circle-color project? does it work with advanced compilation too?
1. Yes it is related to that. Previously each file would have been transformed just before running convert just for that file.
3. in the case of 2., probably externs are not needed anymore for the consumed libraries, or is this not true?
Because all files are converted in one step, all the files need to be transformed before Closure compiler tries to parse them.,
I did build the compiler, but did not try with circle-color yet
gotcha, interested in seeing if that works out ^
1. and js-transforms has not in fact been changed,
@juhoteperi hah, I didn’t expand the diff 😄
Yep, I was confused myself at first
^ I think this is the only point where you have some doubts?
;; FIXME: Should this contain other module types also? ;; And both from foreign-libs and ups-foreign-libs (cljs-1682)? ;; But probably other module-types can't be parsed by this compiler?
Yes, and the change of convert-js-modules API
But I guess the module-types are mostly internal detail anyway as they depend on Closure compiler
So people probably won't define their own convert-js-modules methods (and it is in implementatio ns)
I don’t understand that last part
what change in the API are you talking about?
doesn’t it look the same for CLJS-Compiler consumers?
I guess calling it API is wrong
Just implementation change, convert-js-module -> convert-js-modules and different parameters
ah I see it now
I didn’t notice the pluralization
It used to be called for each foreign-lib with module-type, now it is called for each used module-type with list of all files with of that type
yeah, I got that
@juhoteperi so if I understand the problem correctly, my feedback for the
FIXME I pasted earlier would be to include
ups-foreign-libs too, filter by the ones that actually have
:module-type and convert them
the problem might be dependency order, maybe?
I don't think we need to care about dep order here
Oh, but this is in the context of
I get the problem about other module types now
Ah right, there is problem with amdjs
I think I added comment about that somewhere
amdjs and commonjs should be processed together
so they see each other
what module-types are you testing with?
I’ll build the compiler and test the Circle-color project
at least that’s what I was planning to do
it might be nice to have a script that downloads the deps for that (so we don’t need to include it) and builds it?
@dnolen the circle-color stuff?
I think @juhoteperi emulated that in the tests, not sure if we actually need it in full
Yeah in tests I just included "minimal" files, I manually selected small parts of the files
I could change it to download the files
The JS code probably doesn't really work, but it can be transformed and checked that transformation creates proper Closure module defs
@juhoteperi what’s blocking it from working? (maybe I missed that you mentioned this earlier)
The mininal React file contains just some function stubs, no real implementation
Yes that should work
right so I think that’s preferable as a test since it’s the most comprehensive thing we have
@juhoteperi the circle-color project works with your patch 🎉
testing advanced compilation now
generates those 2 warnings, and doesn’t really work in the browser
probably not even related to the module stuff?
I actually get a hard error using advanced compilation with
@juhoteperi actually it was an externs problem… It kinda works with advanced compilation using your patch
I thought externs wouldn’t be necessary though?
Probably depends on if module does something dirty, like dynamically create methods
@dnolen there are now a bunch of new closure warning categories that we should probably consider adding to
should this be a different ticket or included in @juhoteperi’s patch?
Could be probably done easy in different patch?
maybe, after yours has been committed
@anmonteiro different patch is going to be better
@juhoteperi @anmonteiro yes if the JS code is generating methods and then statically using them later - there will be problems
@anmonteiro out of curiosity how big is the gzipped circle example now?
the compiled artifact, right?
@anmonteiro also interested in whether there’s actually any improvements to advanced compile time? (i.e. building the tests)
I don’t really know how to serve gzip compression locally
should I just gzip the artifact?
@dnolen wow this can’t be right
the whole thing is 351kb, 85kb gzipped
well, I thought it would be a lot more
@anmonteiro it ain’t shabby! 🙂
extern inference will reduce the damage as the infered extern would only include really used properties
@juhoteperi yes for CLJS code
Well, maybe that can eventually be fixed on Closure or React side
(use without exterrns)
CLJS master without <@U061V0GG2>’s patch: Compile sources, elapsed time: 12941.43335 msecs Compile sources, elapsed time: 0.135049 msecs Applying optimizations :advanced to 71 sources Optimizing 71 sources, elapsed time: 39783.06221 msecs CLJS master with the patch: Compile sources, elapsed time: 13317.732659 msecs Compile sources, elapsed time: 0.194513 msecs Applying optimizations :advanced to 71 sources Optimizing 71 sources, elapsed time: 40123.820472 msecs
so not really any improvement there
@anmonteiro heh, little change - that’s ok
@anmonteiro oh what memory settings are you using though?
did you run it multiple times? looks small enough to be just error
@dnolen any easy way to do that in the test script?
@juhoteperi the compilation time is long enough for HotSpot to take effect
java -server -Xms1g -Xmx2g -cp "$CLJSC_CP" clojure.main "$CLOJURESCRIPT_HOME/bin/cljsc.clj" "[email protected]"
no change is better than a change for worse 🙂
oh one more thing
I’m going to check the bundle sizes
between different Closure versions
Btw. once interesting change in 20160911 was that input sources (e.g. foreign-libs) can now include source-map
ah makes sense
for JSX stuff for example
@juhoteperi yes I saw that, I also saw some chatter about source map merging
but I didn’t investigate further - so annoying that we had to roll our own thing for that
@dnolen the ClojuTRE talk portion of source mapping was compelling haha 🙂
seemed like a really hard thing
38920.114401 msecs compiling the compiler tests
and the bundle size is unchanged
@anmonteiro huh, I’ll give the patch a shot too later and see what the work machine says
ah right, you might get better times in that machine
this is a 2 year-old 15” Macbook Pro
this is awesome work @juhoteperi, thanks! 🙂
I second that ^
We should probably check if it is really Closure-compiler compile which takes 40 sec, or if our optimize function is doing something unncessarily slow
@juhoteperi we should, though I’m pretty sure the advanced compile time thing only measures Closure time - if it doesn’t we should log that
@dnolen it seems it’s measuring more than that
optimize function call time, which initializes compiler, builds input file lists etc, and creates the source map
Most of the stuff it is doing should be very fast, but I don't know about source map generation
@anmonteiro ah, we should fix that
meaning we should also measure Closure time only in addition to our whole optimization pass
@dnolen should be a simple change, I’ll open a ticket for that as well
(eval-str "42") .... I removed the
:statement check from
(emit* :constant ..., it works and tests pass. Is it that simple?
first I would come up with a real goal see if co-aligns with issues we already know about and then ask questions on how to accomplish that goal
my impression is you want to be able to do simple scripting a la Clojure with ClojureScript
however if you really want to understand what all needs to be done, I’m more than happy to walk you through it
however we would like to get more portable semantics with Clojure - this is the real and only goal
deps.cljs, data literal files, simple scripts all share common need for ns-less files
so we would like for this to work but there’s a serious problem which needs to be addressed first
and we also need to impose constraints on what is permissible so that the work and user expecations are both reasnoable
@pat there’s some context in this issue http://dev.clojure.org/jira/browse/CLJS-1346
@dnolen just remembered, still awaiting feedback on the comment there ^ (I had forgotten about it, sorry)
@anmonteiro responded I think the thing that I’m getting stuck on is in my new comment
@pat as you can tell we’re in the very deep end - which is why no one has worked on it 🙂
@dnolen I think a
:toplevel key in the AST would probably be easier
as per your comment, as opposed to my proposal
@anmonteiro that sounds reasonable too - feel free to leave that comment
wrt. your question, don’t we have something we use for REPLS that’s
wouldn’t that work for files without NSes to see each other?
maybe it serves a completely different purpose and I didn’t get it right 🙂
@dnolen when is
cljs.user created now?
so proposal: we create it upon seeing the first file without a
if we never see one, things work as before (currently)
and we can have a custom resolve for
cljs.user to look in these sub namespaces to resolve a var
oh you might have mentioned this before
@dnolen ah no you mentioned “gen a random NS"
yeah, if we have the file name it could work
probably also need to take path into account and munge it somehow
yeah but they’ll probably be at different paths in the classpath, right?
so there would be no way of mapping errors to those files?
maybe that’s wrong, we probably know which file "we’re compiling right now"
@dnolen I’ll capture all this in a comment to the issue, but there’s something I’m not getting still
if we’re analyzing a file with an
NS statement, do we use the
the name might be misleading to me
did you mean it only in the context of files without NS or every top-level declaration??
@dnolen just to make sure I understand correctly:
assoc :toplevel true in the analyzer entry points (e.g.
analyze-file) but remove it from the env in the recursive calls?
right, no nested
so then we could relax the assumption that every file needs
this first part doesn’t seem hard to implement at all at first sight
ah! that’s what I was missing
@dnolen added a comment to the issue, let me know if anything doesn’t look quite right; I might have misunderstood something http://dev.clojure.org/jira/browse/CLJS-1346?focusedCommentId=43974&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-43974
this looks promising! I’ll only be partially happy when we have support for
@anmonteiro summary looks good to me
looks like a fun weekend project 😉
@dnolen: now that I think about it, what did you mean by
:top? Top-level expressions or top of the file?
I might have misunderstood what you meant by top
Yeah, but what do we want to do with toplevel expressions beyond disallowing
require and such?
Or rather, only allowing them at the toplevel
So I suppose top of the file is not really needed since it'll just be the first form we find
Like it's done now
it’s just the hack we need in the compiler for emitting the first form, but even that should be ok
Are you talking about something that's already in place or?