This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2017-08-16
Channels
- # aleph (1)
- # architecture (5)
- # beginners (43)
- # boot (23)
- # cider (5)
- # cljs-dev (143)
- # clojure (42)
- # clojure-austin (4)
- # clojure-dusseldorf (14)
- # clojure-italy (15)
- # clojure-norway (1)
- # clojure-russia (10)
- # clojure-spec (41)
- # clojure-uk (70)
- # clojurescript (262)
- # cursive (3)
- # data-science (18)
- # datomic (66)
- # figwheel (1)
- # fulcro (39)
- # hoplon (21)
- # jobs-rus (1)
- # juxt (4)
- # lein-figwheel (2)
- # leiningen (4)
- # lumo (26)
- # off-topic (4)
- # om (6)
- # onyx (19)
- # parinfer (50)
- # pedestal (9)
- # portkey (10)
- # re-frame (41)
- # schema (5)
- # spacemacs (2)
- # yada (35)
@anmonteiro I think that’s a refactor for later, gonna close CLJS-2309 for now
it’s an optimization
CLJS-2309 is now resolved on my side too when using master. @anmonteiro fwiw, I was able to reproduce the issue using the standalone 1.9.854 cljs.jar , it can be triggered by specifying a long file path to a packaged foreign lib on the classpath, I have added a script to the repo https://github.com/symfrog/cljs-modules-react-order/blob/master/standalone.sh that will produce ./release/fail_moduleb.js
and ./release/pass_moduleb.js
simply by varying the file path to the jars
Btw. I guess Tsickle also allows converting whole TS projects to Closure
> We already use tsickle within Google to minify our apps (including those using Angular), but we have less experience using tsickle with the various JavaScript builds that are seen outside of Google.
Just an update: https://github.com/honzabrecka/ts-to-goog is now rewritten to clojure and repository contains externs (~3.5k) generated from DefinitelyTyped. Take a look and let me know.
it is very useful thanks!
super cool! was chatting about this with a friend some time ago, excited to see it being done 🙂
After executing this command: java -jar gen.jar <npm-package> , it deletes the npm-modules folder and package.jason file and throws npm-packages not found error. Has anyone ran into this problem, any solution?
@honzabrecka wow awesome
@anmonteiro hey I’m removing the konan
dep and just putting it inline into module_deps.js
I’m also reformatting this file to 4 space indent to match our other JS sources, and making it consistently ES6 - I don’t see any problems with this at the moment - but maybe you know something
Consistently es6?
Like using const
and things?
I've refrained from doing that because of older Node.js versions
It doesn't. I updated from Node 4 to 6/8 some weeks ago.
Hah missed that
@anmonteiro I’m actually fine with lowering to ES5
Ok so we require Node 6, which is the LTS anyway
@dnolen OK cool. Do you wanna address this for today's release?
I don't think anyone really uses Node 4 on work machines
Some servers might run that but nothing really works with that anyway
My thinking is we put that code in our module-deps fork
Not in the cljs codebase
@anmonteiro we could, but I’m actually more interested in just getting export ... from
working
@dnolen sure, but if it's as easy as you make it sound I can do it before lunch
And still be in the release
@anmonteiro ok, then first let me get my fix in, and you can provide a refactor patch
Whatever you prefer
@anmonteiro this was the only change konan needed
Does that handle exporting default declarations?
Dunno if the AST node is the same type
But if it's not it should be included
@dnolen i.e. I think you're handling export {x} from 'y';
but not export default from 'y'
@anmonteiro I pushed my work, far as I can, old stuff is still working fine - tests are passing
for the time being I’d actually like to keep this work in ClojureScript, it’s just simpler to make changes - it also gives a better picture how we might fold this into ClojureScript itself by rewriting this stuff in Clojure
oh material is still not working, but it’s due to some IlllegalStateException now I’ll have to look at when I get back
sounds good
@dnolen IllegalStateException
s are a pain, but I can try and look if you point me to a stacktrace
normally it’s a call to Guava’s checkState
that doesn’t provide any other information
hm, konan
's author already resolved the issue: https://github.com/egoist/konan/issues/4
@dnolen one other thing. Do we now require more deps to be installed?
we should list those in @cljs-oss/module-deps
@anmonteiro already did that
awesome
@dnolen fwiw the konan author also didn’t handle default exports
I’m not even sure if that is part of the import spec yet
it’s only at stage 1
I don’t think we need to handle
@anmonteiro if we can get something else non-trivial like Material to work that would be another big step 🙂
IllegalStateException:
com.google.common.base.Preconditions.checkState (Preconditions.java:429)
com.google.javascript.jscomp.CodeGenerator.add (CodeGenerator.java:986)
com.google.javascript.jscomp.CodeGenerator.addExpr (CodeGenerator.java:1556)
com.google.javascript.jscomp.CodeGenerator.add (CodeGenerator.java:274)
it doesn’t seem to like node_modules/@material/drawer/temporary/constants.js
with the export that only has the key
I use this to resolve the deps https://gist.github.com/thheller/d5761ee8adbe6f857bf6e05a71efda06 which already handles the export from ...
stuff
it works when I change FOCUSABLE_ELEMENTS,
to FOCUSABLE_ELEMENTS:FOCUSABLE_ELEMENTS,
probably a closure bug?
it also fails for me in ‘./persistent’
oh it also has constants
setting language-in to ecmascript-next
doesn’t have any effect
this is where it fails btw: https://github.com/google/closure-compiler/blob/e4ec82ec15f8968f4ed19b7ce49bfd89f7cb34da/src/com/google/javascript/jscomp/CodeGenerator.java?utf8=%E2%9C%93#L986
looks related
perhaps low hanging fruit in Closure that one of us here can help solve
@anmonteiro does that appear in Material?
yes, let me grab the link
@dnolen here https://github.com/material-components/material-components-web/blob/master/packages/mdc-drawer/temporary/constants.js#L29
this is what Thomas was referring to
that’s what Closure is choking on
@anmonteiro huh, what is going on there?
shorthand for var a = 1; {a: a}
you can just specify {a}
I guess it’ll be unsupported in Closure until they add parser support for the new stuff
the issue is fairly new, like 1 month old
maybe it’s being worked on
or maybe a CLJS person needs to work on it for them 😛
@anmonteiro how were you able to determine that the issue was constants.js
?
@dnolen if I change the line to read FOCUSABLE_ELEMENTS: FOCUSABLE_ELEMENTS
it passes
like actually modify node_modules
both in persistent/constants
and temporary/constants
trying something else
by us you mean Closure?
right
might be that
well anyways this isn’t going to get solved right now … I’d say the the change I made is for the better and will allow more things to be consumed
@dnolen agreed it won’t change soon. I can look into it on the Closure side, maybe later
I’m 👍 for a release
ok I’m probably going to lay low for the rest of the week and will be out most of next, technically already on vacation 🙂
@dnolen just reproed the material bug in a Closure compiler test
definitely in Es6RewriteModules
I’ll try to come up with a fix and PR tonight
exciting to help move things along in Closure!