This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2017-07-08
Channels
- # beginners (76)
- # boot (13)
- # cider (3)
- # clara (7)
- # cljs-dev (254)
- # cljsrn (5)
- # clojure (20)
- # clojure-austin (2)
- # clojure-chicago (4)
- # clojure-dev (7)
- # clojure-russia (5)
- # clojure-spec (18)
- # clojurescript (68)
- # cursive (8)
- # datascript (3)
- # datomic (8)
- # garden (1)
- # hoplon (3)
- # lambdaisland (4)
- # luminus (20)
- # mount (19)
- # off-topic (30)
- # om (10)
- # onyx (8)
- # parinfer (14)
- # precept (7)
- # reagent (9)
- # unrepl (3)
- # untangled (72)
- # vim (4)
- # yada (1)
I think there’s an issue with {:libs [,,,]}
style JS dependencies
Introduced in 9e5b7ac64dd0388d3403f48381c80ba1e3998da8
but just reverting that one on master
doesn’t fix it
ok, correction, cljsjs/incremental-dom seems to be the sole offender whereas e.g. openlayers works as expected
It is trivial to get pre-built ChakraCore binaries and have our unit tests run over them (they run in about 2 seconds). Willing to contribute patches if there is a desire to make it a thing. Here is a high-level gist: https://gist.github.com/mfikes/b675baa14946e11968a3f191ee49f932
I’ve highlighted the issues that I believe to be blockers for the next release https://dev.clojure.org/jira/secure/IssueNavigator.jspa?reset=true&jqlQuery=project+%3D+CLJS+AND+resolution+%3D+Unresolved+AND+priority+%3D+Blocker+ORDER+BY+key+DESC&mode=hide
I haven't yet had time to properly test if/how string based requires work with non-js-module foreign-libs, but I think it would be important to support those also, to provide way for libraries to depend on js-libs no matter how the js-lib is provided (npm-deps, cljsjs package)
But that can also be added later
But it is great weather today so testing this and writing the blog post will have to wait a bit
@mfikes is this one still an issue https://dev.clojure.org/jira/browse/CLJS-1644
@dnolen Yes, CLJS-1644 still occurs on master. In fact, a duplicate of it was recently written, which I have now closed: https://dev.clojure.org/jira/browse/CLJS-2129
these I consider blockers for the next release - https://dev.clojure.org/jira/secure/IssueNavigator.jspa?reset=true&jqlQuery=project+%3D+CLJS+AND+resolution+%3D+Unresolved+AND+priority+%3D+Blocker+ORDER+BY+key+DESC&mode=hide
these are nice to haves if we can get to them https://dev.clojure.org/jira/secure/IssueNavigator.jspa?reset=true&jqlQuery=project+%3D+CLJS+AND+resolution+%3D+Unresolved+AND+priority+%3D+Critical+ORDER+BY+key+DESC&mode=hide
@dnolen I tested CLJS-1764 the other day and it's still a problem
I think you closed it this morning
(Double warning at the REPL)
IIRC the minimal repro doesn't even contain function (bodies)
@anmonteiro hrm really?
@anmonteiro I’m also having trouble again with :npm-deps
on a different machine https://dev.clojure.org/jira/browse/CLJS-2193
java -cp cljs.jar:esrc clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 49560
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.730"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/anmonteiro/Documents/github/clojurescript/esrc/foo/core.cljs
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/anmonteiro/Documents/github/clojurescript/esrc/foo/core.cljs
true
cljs.user=>
@dnolen ^ using the exact repro in the ticket
@anmonteiro oh and you supplied patch for this (I see now REPL only)
yeah but I remember you didn’t like the patch for some reason
ah yeah
here’s what I remember: the REPL ends up analyzing namespaces twice in cljs.repl/load-namespace
cljs.closure/cljs-dependencies
was the place where the 2nd analysis would be made IIRC
I can have another proper look today to see if we can solve it better
@anmonteiro OK, thanks - I re-opened and left feedback on the patch
@dnolen just saw CLJS-2193 too. I think I know what’s going on
I updated module_deps.js
but didn’t update the packages we install 🙂
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/closure.clj#L2122
I think we should still keep installing these implicitly
just as long as we install them… which we’re not doing
thankfully we have tests for it now
how do I repro?
@dnolen hahaha
that’s pretty hard to do since module-deps
and resolve
are transitive dependencies of NPM!
running npm 5 here
this makes much more sense now
Node 8 comes with NPM5 now
I think there’s one way to not rely on globally installed modules
export NODE_PATH=
definitely
hrm yeah I can’t repro locally
going to install Node 6
reproed with node 6 / npm 4
also submitted an update to the ClojureScript website: https://github.com/clojure/clojurescript-site/pull/108
oh god…
@dnolen is the current directory implicitly added to the classpath in Java (or maybe Lein)?
just spent some time trying to figure out why my fix wasn’t working, I had a compiled CLJS uberjar in my clojurescript directory
hah, maybe that
OK patch attached to CLJS-2193
no, the correct behaviour should be that tools.reader fails w/ EOF but tools.reader.edn doesn't (to mimic clojure)
cljs.user=> (require '[clojure.tools.reader.edn :as e])
cljs.user=> (e/read-string "")
nil
cljs.user=> (e/read-string ";foo")
nil
to be clear, clojure.tools.reader.edn/read-string
returns nil on EOF, while clojure.tools.reader/read-string
throws
this is to parallel clojure.core/read-string
that throws while clojure.edn/read-string
doesn't
if you want clojure.tools.reader/read-string
to return nil, you'll have to be explicit about it
yes I understand that, I'm saying that I can't reproduce your EOF using the .edn
reader, see
[~/src/tools.reader]> lein trampoline cljsbuild repl-rhino
Running Rhino-based ClojureScript REPL.
To quit, type: :cljs/quit
cljs.user=> (require '[cljs.tools.reader.edn :as e])
cljs.user=> (e/read-string ";foo")
nil
if you see something different than this that might be due to different js environments but you'll have to tell me how to repro
tools.reader follows the same API as clojure, so it's an EOF exception for the clojure reader and nil
for the edn reader
(beware that if you use the 2-arity version you'll have to pass :eof nil
explicitly in the opts map)
(if '
is not in the EDN spec, then we should throw instead, but clojure.edn
behaves like that so tools.reader
has at least parity with the official reader behaviour)
while we're talking about reader tests, are any of those dependant from the error message being thrown?
ok cool, context is I'm looking to cut a beta with https://dev.clojure.org/jira/browse/TRDR-44 next week so the error messages will change
@bronsa quick q: for reading input strings at a REPL should I use c.t.r.edn
?
or rather, to make a REPL
Lumo in this case
totally
thanks
clojure.tools.reader.edn> (read-string {:readers {'foo (constantly 1)}
:default (constantly 2)}
"#foo 123")
1
clojure.tools.reader.edn> (read-string {:readers {'foo (constantly 1)}
:default (constantly 2)}
"#bar 123")
2
seems like cljs.reader
is a mix of the clojure reader and clojure.edn
in terms of API?
@dnolen another cljs.util/relative-name
bug.. added tests for this case https://dev.clojure.org/jira/browse/CLJS-2194
found while fixing CLJS-2152
yeah the array-map one was TRDR-40 and is in 1.0 (unless there are other array-map issues i'm not aware of)
probably could use some code review when people have time https://github.com/clojure/clojurescript/pull/69/files
wrapping up the fix to CLJS-2152
oh npm-deps-test fail
so I guess apply 2193 before 🙂
is (.fromArray PersistentHashMap ..)
fine or should I be using (.fromArray cljs.core/PersistentHashMap ..)
instead?
so deferring to tools.reader from cljs.reader
fixes CLJS-2059 which is also on the blockers list 👍
(reading namespaced maps)
just confirmed locally
which means CLJS-1959 is now the only blocker for the next release 🎉
I think David mentioned before EuroClojure
Which is in 2 weeks
Might be this week or the next I believe
@dnolen PR looks fine to me, only minor thing is that the docstrings you copied over refer to clojure.tools.reader.edn
vars, not sure if you want to change that or make the relationship between cljs.reader
and cljs.tools.reader.edn
more explicit
@dnolen the reader PR looks good with the exception of: - 1 failing test in pprint-test - 1 failing test in tagged-literals test
the 2nd one is easily fixed, left an inline comment
the first one fails on (reader/read-string "@@(ref (ref 1))")
@bronsa ^ is this valid under c.t.r.edn
?
right, so that’s probably why the test is failing 🙂
maybe it’s a bad test
doesn’t look like a bug: https://dev.clojure.org/jira/browse/CLJS-1753
(it’s in the nice-to-haves list)
we're currently doing
(if (<= map-count (* 2 (.-HASHMAP-THRESHOLD cljs.core/PersistentArrayMap)))
(.fromArray cljs.core/PersistentArrayMap (to-array the-map) true true)
(.fromArray cljs.core/PersistentHashMap (to-array the-map) true))
@bronsa it’s the only one that will do the checks across PAM & PHM, no-clone parameter is not useful since we have to traverse anyway
it was added 6 months ago - so it’ll be an incompability with older versions of ClojureScript
I don't know how many people are using t.r in cljs but I'd rather not break backward compatibility if it's not strictly necessary
The ChakraCore testing effort perhaps paid off in a sense: While things pass on macOS and Linux, there are three failures on Windows. I don’t know if they are specific to ChakraCore or Windows. If I could figure out how to copy text from the window (grr), I’d share them. Lacking that here is a picture of showing the 3 failures https://twitter.com/mfikes/status/883799875858694144/photo/1
@anmonteiro one weird thing I noticed is that lein-test doesn’t seem idempotent wrt. to npm-deps-test
Yeah, the patches are ready, and can be applied (they don’t cause anything to break in CI or on macOS)
hrm, let me try that
I think there’s ChakraNode
There is a package.json
file or somesuch that is written to the filesystem when running the tests. Perhaps that leads to lack of idempotency?
hah, that could be it actually
maybe we need to wipe that between tests
I’ll try to repro
Yes, there is ChakraNode—I had used it to make a ClojureScript REPL previously. But David is right, the ch
command insists on a file being fed to it rather than it dumping you into a REPL.
merged cljs-1800 branch, will bump tools.reader when you release @bronsa - thanks for the help
FWIW, SpiderMonkey says “no” on Windows with:
TypeError: malformed UTF-8 character sequence at offset 887730
if you try to do js.exe -f core-advanced-tests.js
.
Maybe we can catch this kind of stuff with that Windows-based CI setup António mentioned.@anmonteiro I’m starting to feel somewhat inclined to roll back the changes to boostrap.js for CLJS-1959
I think that would fix it
it’s super convenient to be able to test stuff with Node.js - but perhaps not actually important
@dnolen what would we lose?
do you mean this commit? https://github.com/clojure/clojurescript/commit/59a7f265fac02c931cc3d5615727da861db62e3b
this looks right https://github.com/clojure/clojurescript/blob/367e2fb600c54d3b04a1d85ccf9bc659b7f05151/src/main/cljs/cljs/bootstrap_node.js
@anmonteiro I’d take a patch to take us back to this
gotta run again but that’s what I’m leaning toward - it’s just not worth it to make Node.js users lifes more complicated to change our bootstrap.js scriopt
I’ve isolated the Windows issues I mentioned above to bad JavaScript being generated when compiling on Windows. (The same errors appear when taking the JavaScript generated there and running in the interpreters on macOS, and no errors appear when taking JavaScript generated on macOS and running them in the Windows interpreters.)
The malformed Unicode that SpiderMonkey is not happy with is probably related to low-level Closure. It also occurs in :simple
mode, and the error is right at the á
in this line:
goog.string.FORCE_NON_DOM_HTML_UNESCAPING=!1;goog.string.Unicode={NBSP:"á"};
Good news is, if you develop on macOS or Linux, the code emitted is good. I suspect you run into issues if you try to produce production code on a Windows box.submitted a patch in https://dev.clojure.org/jira/browse/CLJS-2195 making NPM deps tests idempotent