This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2017-08-03
Channels
- # bangalore-clj (1)
- # beginners (104)
- # boot (30)
- # braveandtrue (1)
- # cider (6)
- # cljs-dev (95)
- # cljsjs (16)
- # cljsrn (3)
- # clojure (106)
- # clojure-italy (15)
- # clojure-nl (2)
- # clojure-norway (3)
- # clojure-russia (1)
- # clojure-spec (40)
- # clojure-uk (53)
- # clojure-ukraine (1)
- # clojurescript (200)
- # code-reviews (2)
- # cursive (1)
- # datascript (3)
- # datomic (32)
- # editors (28)
- # gorilla (6)
- # graphql (8)
- # hoplon (1)
- # jobs (8)
- # jobs-discuss (5)
- # jobs-rus (1)
- # keechma (13)
- # leiningen (5)
- # luminus (3)
- # lumo (53)
- # off-topic (5)
- # om (5)
- # om-next (1)
- # onyx (56)
- # parinfer (7)
- # protorepl (22)
- # re-frame (47)
- # reagent (37)
- # remote-jobs (1)
- # ring-swagger (9)
- # specter (7)
- # vim (14)
- # yada (30)
so good news, bad news
good news is I have a patch that adds an option to Closure making the browser
field (or any field, really) work
bad news is that calling .toSource
on converted warning
NPM module results in a Closure error
¯\(ツ)/¯
^ wait this is not true
oh it’s the UMD thing
@juhoteperi was this the error you were getting in the UMD thing?
Caused by: java.lang.IllegalStateException
at com.google.common.base.Preconditions.checkState(Preconditions.java:429)
at com.google.javascript.jscomp.CodeGenerator.add(CodeGenerator.java:657)
at com.google.javascript.jscomp.CodeGenerator.add(CodeGenerator.java:638)
at com.google.javascript.jscomp.CodeGenerator.add(CodeGenerator.java:106)
at com.google.javascript.jscomp.CodeGenerator.addFunction(CodeGenerator.java:1363)
at com.google.javascript.jscomp.CodeGenerator.add(CodeGenerator.java:384)
at com.google.javascript.jscomp.CodeGenerator.addExpr(CodeGenerator.java:1556)
at com.google.javascript.jscomp.CodeGenerator.addStringKey(CodeGenerator.java:1631)
at com.google.javascript.jscomp.CodeGenerator.add(CodeGenerator.java:960)
at com.google.javascript.jscomp.CodeGenerator.add(CodeGenerator.java:106)
at com.google.javascript.jscomp.CodeGenerator.add(CodeGenerator.java:987)
@anmonteiro nice one on the Closure PR
I'm testing if the UMD wrapper changes are enough, because I noticed React and others use similar pattern where module.exports is assigned result of a function call, and somehow that already works
@juhoteperi there’s nothing wrong with that pattern far as I know
Well, Closure fails during .toSource
call for some file currently, with that change, so something isn't working
Trying to debug further which file causes the problem
https://www.youtube.com/watch?v=-JLhwsbMvjQ&list=PLX8CzqL3ArzXJ2EGftrmz4SzS6NRr6p2n&index=7
Oh I forgot React doesn't really have any UMD wrapper when using code from npm package
In React 16 the React core is single file: https://unpkg.com/[email protected]/cjs/ vs https://unpkg.com/[email protected]/lib/
Same with React-dom: https://unpkg.com/[email protected]/cjs/ https://unpkg.com/[email protected]/lib/
I wonder if this will work with Closure: https://unpkg.com/[email protected]/server.js
I don't think the module processing knows about constants
@juhoteperi https://unpkg.com/[email protected]/index.js it does work but :none
optimized code will also load the production code
I think Closure just has to be made aware of that pattern
Based on that looks like they are planning on using this
only real issue is that :none
loads the rewritten react/umd/react.production.min.js
but does not use it
just rebased cljs-2291
@bronsa just saw your comment in cljs-2261
but I don’t understand it given the commit
https://github.com/clojure/tools.reader/commit/18512bf0975495e5f29f243444824429f8d46499
it seems that you’re still passing the symbol without the dot to resolve-symbol
we're now not special casing resolution of foo.
in syntax-case, and just delegating to resolve-symbol
got it
do you think it matters for ClojureScript though?
given the macro problem we discussed the other day
1.0.4 already worked in self-hosted
I can see how this is more correct than the previous version though
yeah but caused a regression in CLJ, this allows the 1.0.4 fix in cljs and fixes the behaviour for clojure
awesome, thanks for clarifying
unfortunately as I was writing it, I realized that transitive requires would still be loaded through c.c/require
that makes sense
@anmonteiro 2291 doesn’t apply cleanly for me, do I need to do something to make sure this works?
let me check again
doesn’t apply for me
rebasing again
@anmonteiro ^ fixes it for me - applies
ah cool
yeah same here
I’ll add a PR to the CLJS site
hopefully we won’t forget where to look for it next time
I’m surprised Slack saved the messages above form July 9th
@anmonteiro thanks
good to go, TR 1.0.5 signed off by Nicola
@mfikes if you register the ClojureScript project in your Appveyor account
you’ll get CI for Windows too 🙂
2291 just went in
@juhoteperi would like some more context on 2294
@juhoteperi https://github.com/google/closure-compiler/pull/2596#issuecomment-320102416
rolled back
FWIW Windows CI is now running when I push to my fork at https://github.com/anmonteiro/clojurescript/commits/master
I’ll be working on more patches to make the remaining test failures pass
goal is to have the Windows CI happy / at parity with Travis
here’s a list off the top of my head of what’s missing in the Windows CI:
- self-host tests
- self-parity tests
- set up more JS engines (only spidermonkey is running atm)
- set up a way to check runtime test output in a similar vein to what @mfikes did with Travis (`grep` for "0 failures, 0 errors"
)
maybe we can turn this into one or several JIRA tickets ^
By adding a little meta (`^:numeric ^{:js-op "max"}`) to the js*
form inside the max
macro, we can recover a little of the intent of ::ana/numeric
:
cljs.user=> (max 1 "a")
WARNING: max, all arguments must be numbers, got [number string number string] instead. at line 1144 <cljs repl>
"a"
But, it is a bit of a hack, IMHO.