This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2016-09-30
Channels
- # bangalore-clj (1)
- # beginners (9)
- # boot (51)
- # cider (20)
- # cljs-dev (419)
- # clojars (1)
- # clojure (338)
- # clojure-brasil (64)
- # clojure-dev (7)
- # clojure-greece (2)
- # clojure-italy (3)
- # clojure-russia (10)
- # clojure-spec (127)
- # clojure-uk (12)
- # clojurebridge (2)
- # clojurescript (132)
- # core-async (8)
- # cursive (37)
- # datomic (34)
- # dirac (5)
- # events (1)
- # funcool (3)
- # hoplon (39)
- # jobs (3)
- # leiningen (3)
- # off-topic (16)
- # om (44)
- # onyx (7)
- # pedestal (20)
- # protorepl (1)
- # random (1)
- # re-frame (64)
- # reagent (6)
- # specter (4)
- # test-check (9)
- # untangled (17)
- # vim (4)
@dnolen just confirming that the refer-clojure
syntax should be:
(refer-clojure :exclude '[map mapv])
instead of (refer-clojure :exclude [map mapv])
, right?
note the ’
also we can’t have cljs.js/require
anymore if require
is a special form 🙂
@anmonteiro yes all those things need to require quoting
awesome, quoting definitely works. problem would be otherwise
@anmonteiro hrm I wonder if then adding all these special forms is a bad idea
@dnolen I kinda like that idea better
will do
@dnolen @anmonteiro Would it be valuable to create a patch for supporting mingw?
@dnolen I am new to Jira, but in order to follow the patch directions, it looks like it needs to be there. Any suggestions?
I just created an account for the Jira page, I think I need to create an issue, but I don't know how to.
create an Enhancement ticket, set the priority to Minor, filling out the rest of the relevant information
@dnolen just noticed we’re not running all the tests when executing ./script/test
opened a ticket for that, working on a fix now
@anmonteiro yeah I noticed that too, not sure what the deal is - all of them under bootstrapped strangely
I’m thinking about removing the duplicate test_runner.cljs
it’s just a lot of trouble
yea...
that’s the problem
got all of them running now, but the predicates_test are failing
18 of them
expected?
FAIL in (test-preds) (b0@builds/out-adv/core-advanced-test.js:1114:77)
(#object[ig "function ig(b){return null!=b?b.o&8388608||}"] #inst "2016-09-30T12:56:25.080-00:00")
expected: (= ((nth preds i) v) (nth row i))
actual: (not (= true false))
FAIL in (test-int-preds) (b0@builds/out-adv/core-advanced-test.js:1114:77)
(#object[ng "function ng(b){return mg(b)||b instanceof pa||b instanceof Ha}"] 0)
expected: (= ((nth preds i) v) (nth row i))
actual: (not (= true false))
FAIL in (test-int-preds) (b0@builds/out-adv/core-advanced-test.js:1114:77)
(#object[rg "function rg(b){if(mg(b))return!(0>b)||0===b;if(b instanceof pa){var c=Db(b.ta());return c?c:b.ab()}return b instanceof Ha?(c=Db(b.ta()))?c:b.ab():!1}"] 0)
expected: (= ((nth preds i) v) (nth row i))
actual: (not (= true false))
FAIL in (test-int-preds) (b0@builds/out-adv/core-advanced-test.js:1114:77)
(#object[ng "function ng(b){return mg(b)||b instanceof pa||b instanceof Ha}"] 1)
expected: (= ((nth preds i) v) (nth row i))
actual: (not (= true false))
FAIL in (test-int-preds) (b0@builds/out-adv/core-advanced-test.js:1114:77)
(#object[og "function og(b){return mg(b)?0<b:b instanceof pa?Db(b.ta())&&Db(b.ab()):b instanceof Ha?Db(b.ta())&&Db(b.ab()):!1}"] 1)
expected: (= ((nth preds i) v) (nth row i))
actual: (not (= true false))
some of those ^
probably the tests are wrong?
I’ll leave them out for now and look into that in a subsequent patch
@anmonteiro yeah all those numeric tests are busted
so should we just delete them?
gotcha
@dnolen OK every test namespace running now with the patch in http://dev.clojure.org/jira/browse/CLJS-1798
1 or 2 also missing from the self-parity one
@dnolen hah, found the culprit https://github.com/clojure/clojurescript/commit/6ff80dc2f309f961d2e363148b5c0da65ac320f9
when you did this moving around the test-runner got duped
no idea what was happening though
one overriding the other?
Alright, I moved http://dev.clojure.org/jira/browse/CLJS-1797 into resolved and have tested that build works on both Windows 10 and OSX.
@dnolen we can probably uncomment this line too now: https://github.com/clojure/clojurescript/blob/master/src/test/self/self_parity/test.cljs#L309
testing locally to see if it passes
oh it doesn't
@dnolen problem is (Integer.ZERO)
#error {:message "ERROR in file src/test/cljs/cljs/predicates_test.cljs", :data {:tag :cljs/analysis-error}, :cause #object[TypeError TypeError: goog.math.Integer.ZERO.call is not a function]}
trying Integer.ZERO
without the invocation
well actually it also doesn’t pass in ./script/test
so probably can’t be invoked
@dnolen still failing
FAIL in (test-preds) (b0@builds/out-adv/core-advanced-test.js:1114:77)
(#object[ig "function ig(b){return null!=b?b.o&8388608||}"] #inst "2016-09-30T14:20:08.624-00:00")
expected: (= ((nth preds i) v) (nth row i))
actual: (not (= true false))
not very descriptive though
oh it’s not related
it’s in the pred table above
@dnolen it seems that, at least in my machine, (seqable? (js/Date.))
is failing those pred tests?
but I can’t repro it in script/noderepljs
makes no sense to me as well
checking
@dnolen I think this counts, no? https://github.com/clojure/clojurescript/blob/71c93368aedd49ed3ab5156c78a379e14e275ce8/src/test/cljs/cljs/core_test.cljs#L605
cljs.user=> (goog/typeOf (js/Date.))
"object"
that’s right
@dnolen then self-parity tests also pass with the predicates-test
so we can just uncomment that too
great!
@dnolen Thanks for your response on 1195...think I'm finally with you!
@dnolen I think I’ve got everything working except the :toplevel
validation thing
just trying to figure out where/how to hook up that part?
correct me if I’m wrong, but sticking it into cljs.analyzer/analyze
wouldn’t work
because we might call analyze
from a nested call, right?
@jetzajac http://dev.clojure.org/jira/browse/CLJS-1791?focusedCommentId=43998#comment-43998, your patch needs rebasing
@lauri http://dev.clojure.org/jira/browse/CLJS-1786 please respond to my comment when you get a chance
@dnolen Sorry, there is no reason. What could be the correct approach for setting a true value for "print-namespace-maps" value in repl while the default value would stay false otherwise?
@anmonteiro I don’t understand, the default env should have :top true
I was afraid sticking that into empty-env
would break something
@anmonteiro I don’t see how
probably would only break tests, now that I think about it
which I can fix accordingly
preference for :top
/ :toplevel
?
@anmonteiro a few questions about CLJS-1782 if you have a moment
go ahead
the problem is all about Bootstrap
and you might have a file that :require-macros
itself
just like in Om Next now
we wanna make that work for the 3 cases: Clojure, JVM CLJS & self-hosted
but if you follow the :require-macros
in the bootstrap case, you end up looping forever
yeah..
because there’s no “other” macro file
ah right
so the problem then is that we have a single file which we can’t hide under reader conditionals
so we’re analyzing the $macros
part and there’s a macro call
obviously the arity is wrong because it’s not meant to be called from that compilation stage
so we just swallow that warning
we might, the example in om.next
is the invariant
one
we use it in om.next
, but it’s not meant to be used from om.next$macros
and the user knows that
but we can’t hide it under a reader conditional
exactly, we’d never use that in the macro stage
but we only have one file
because we’re not analyzing a different macros file
we’re analyzing the same file that has all the function definitions as well
the 3rd part in the patch is the same problem
but for var resolution
@dnolen should I make any comments more explicit in the patch?
explicit / descriptive, IDK
awesome
yeah I can explain it now that I’ve worked on the problem for a while
but it took me a while to wrap my head around all that
Looking at http://dev.clojure.org/jira/browse/CLJS-1762 now \cc @juhoteperi
I was hoping to work on it today but beer&sauna now
Hopefully I can finish it this weekend
@dnolen if this is at the top-level:
{:a (invoke-some-fn args)}
should analyzing the invocation be top-level?I’d say “yes” because a map doesn’t introduce scope?
@anmonteiro all recursive cases are not :top-level
gotcha
easier to reason about, but more assoc env :top-level false
I have to write 🙂
@juhoteperi playing around with it, tests are passing at least
FWIW there aren’t a lot of tests with JS modules
yeah, it’s awesome, the circle-color project compiles too, as we saw the other day
it compiles with 2 warnings with optimizations for me
but I think I specified externs
the result in the browser wasn’t what I expected though
I think I saw a quarter of a circle instead of the full circle
exactly
@maria did your circle-color repo ever work with advanced optimizations? (I don’t remember)
even though you still need externs being able to use 3rd party libs as proper requires is pretty nice https://github.com/swannodette/circle-color/blob/master/src/circle_color/core.cljs
FWIW I got errors when trying to do advanced compilation with the deps in that repo
so I’d assume no
@anmonteiro right I get warnings, but no hard errors
hrm I think I had a hard error at some point
something about React being assigned twice or something
can’t remember in full
@anmonteiro it’s just a warning over here
WARNING - constant module$resources$public$js$libs$react assigned a value more than once.
ERROR: JSC_CONSTANT_REASSIGNED_VALUE_ERROR. constant module$resources$public$js$libs$react assigned a value more than once.
dnolen: I’m back to my yesterday’s proposal, I tried to fix the issue “goog.require after repl/bootstrap“ @bendlas ran into: https://github.com/clojure/clojurescript/compare/master...darwin:async-bootstrap
weird
I’m perfectly fine to just solve it on my side in dirac if you don’t want something like this to be merged
@anmonteiro yeah not seeing that
this is with CLJS 1.7.145
@anmonteiro oh, heh sorry I’m trying this with master
@anmonteiro but ok, you’re verifying the old behavior
yeah, with @juhoteperi ’s patch I see the same as you!
glad I’m not going insane
bootstrap
is annoying bit of functionality to have written - but it’s not meant to be end-all-be-all
@dnolen: it's easy to dismiss, because when running alongside figwheel, it starts with a blank screen, but starts working on the first code update
in anycase I’m not against seeing it fixed but @darwin I don’t understand your patch at all
the minimal thing = boostrap must be called after document finishes loading, because there may be some code coming after the bootstrap call which expects goog.require to work synchronously, like @bendlas discovered
I’m afraid this is too low-level to avoid dynamic vars without messing with user’s environment, I’m open for better ideas
if course, that can be optional, but people can call bootstrap! directly in current impl
well, from UX perspective I wanted to have there a possibility to pass a callback, so I can warn people when they potentially eval dirac REPL js without being properly bootstrapped, that is the only thing which is added as extra
ok, fair, will isolate the code into a separate ns, so people could grab my code if they run into issues
looking at that browser-repl bootstrap, the load queue strikes me as pretty sketchy: clojure and clojurescript do recursive-descent loading: each require is loaded, before the code in each namespace is executed. The browser-repl - provided writeScriptTag
looks like it would load the full namespaces before starting with its dependencies ...
on a second thought, scratch that, load - order is provided by goog.closure's dependency analysis.
ha, but this lead me to another possible fix: The problem, really is triggered by the content of the main .js
file in dev mode: it require's the :main
namespace synchronously, via document.write
this could already utilize goog.require
, after goog/base.js
has already been required via document.write
I don’t have a repro case here, and I’m not going to test actively for this (rare) issue - too lazy to write a test for it
the only thing I care about ATM is that it will fix your particular issue and won’t break my existing code 🙂
why does that code use document.write and not appendChild() script elements? I've always wondered that
i.e. code executing after document.write will see its effects, code executing after .appendChild, will not
anyways, the potential fix just landed in dirac: https://github.com/binaryage/dirac/commit/e5f42d3343f54990f347165e247e85ab163b300c
I’m pretty fine with this solution and people could grab[1] if they wanted to solve it for themselves: [1] https://github.com/binaryage/dirac/blob/e5f42d3343f54990f347165e247e85ab163b300c/src/runtime/dirac/runtime/bootstrap.cljs
@dnolen just attached a patch to the “require without NS” http://dev.clojure.org/jira/browse/CLJS-1346
let me know if anything is missing or not the way we discussed
@anmonteiro a whopper!
yeah I suppose they were concatenated to the advanced build
I had some in the classpath when running script/test
and yeah, it’s kinda of a big one
lots of test lines though
awesome will be next improvements such as extensible tagged literals 🙂
can’t wait
it’s really satisfying to work on these “big” patches
I actually get to mess around with all the compiler phases
@dnolen this patch is also the solution for http://dev.clojure.org/jira/browse/CLJS-1277
@anmonteiro a question maybe you know the answer to - since you looked at 1762 - I’m assuming these modules get indexed as foreign-deps?
hrm actually I don’t know
why would they?
they are GClosure compatible, so not foreign I’d say?
@anmonteiro they are supplied via the :foreign-libs
compiler option
it makes some sense because they aren’t Closure compatible, but they can be transformed to be
@dnolen sorry which patch are you talking about?
I probably didn’t even understand the first question
so yeah, I was conflating everything in my head 🙂
@dnolen so to actually answer your question, I think they are indexed as foreign libs
i.e. we end up assoc
ing to the ijs
so I’d say it maintains foreign status
@dnolen if you want a pretty diff, I’ve pushed it to GitHub: https://github.com/anmonteiro/clojurescript/commit/6229d5a57ec02edf2515ff2d428ac60f6c7bc89e
@anmonteiro ok there are some weird things with 1346
then I build the project with :none - but the :output-to
main.js doesn’t have anything I expect
does foo.cljs
have a namespace?
k it’s just so I can repro locally
@anmonteiro also I think we should probably amend the patch so that these are proper subnamespaces
yeah that makes sense
k so I have a foo.cljs
file without a namespace
and I’m seeing this in :output-to
:
goog.addDependency("base.js", ['goog'], []);
goog.addDependency("../cljs/core.js", ['cljs.core'], ['goog.string', 'goog.object', 'goog.math.Integer', 'goog.string.StringBuffer', 'goog.array', 'goog.math.Long']);
goog.addDependency("../clojure/string.js", ['clojure.string'], ['goog.string', 'cljs.core', 'goog.string.StringBuffer']);
goog.addDependency("../cljs/user$$gen_ns$$_foo0BEEC7B.js", ['cljs.user$$gen_ns$$_foo0BEEC7B'], ['cljs.core', 'clojure.string']);
@dnolen my foo.cljs
requires clojure.string
, and the dep is there
@anmonteiro ok maybe a legit bug, I can’t start a Node.js REPL without hitting an error
Namespace declarations must be at the top-level
?
seeing that too
@dnolen ohhh this is probably in cljs.repl
I didn’t try a REPL after hooking up the :top-level
stuff
looking into that now
it doesn’t seem to be only that
when you type (ns foo.core)
you’ll also get an error
@anmonteiro oh maybe because the REPL always wrap the eval form
well that’s exactly the problem 🙂
I just noticed that too
it always calls identity
if no wrapper is given
@dnolen we can have a dynamic var with-no-top-level-allowed
or something
that’ll probably go away whenever we fix the REPL requires anyway
kinda ugly though
or that
just like disallowing-recur
@dnolen will that solve the REPL problem though?
if we disallow ns*
in a function body it won’t work for REPLs too
I mean function call, not body
because we always wrap the ns
call hrm
yeah I confused function body with the args to the function
which is what the REPL does
@dnolen any reason there is disallowing-recur
in cljs.analyzer.macros
and cljs.analyzer
?
@anmonteiro for bootstrap
it’s under :clj
though
@anmonteiro there was definitely a reason
well I’m not contesting that 🙂
just curious
@anmonteiro right that’s the reason
since I don’t think it occured to me that might work or cause more issues while I was trying to bootstrap
oh I see it now
I was thinking the other way around
(i.e. that the .macros
ns was for JVM)
@dnolen if we disallow ns*
inside ’def
I think it covers the fn body case too?
or rather, there and in fn*
@anmonteiro yes that should be ok
@dnolen OK attached CLJS-1346-2.patch http://dev.clojure.org/jira/browse/CLJS-1346
(also tested entering the REPL this time)
@anmonteiro amazing
@dnolen exciting! happy to have a look at the subtickets, just link them here
@darwin: here is a test project for the issue and to verify the fix: https://github.com/bendlas/cljs.test-bootstrap-loading