When running the compiler tests I get errors when running ./script/test, ./script/test-self-host and ./script/test-self-parity (https://gist.github.com/mneise/aec8fe7acfb353ec9424a6136fb12344). Does anyone else get these errors as well?

I’m running the tests on the current master branch.

@maria tests pass for me with current master

that’s just weird, is that from a fresh clone?

maybe try script/clean and script/bootstrap again first

if I had to guess by the output, I would say that the Closure Library dep is behind

although that might not explain the whole thing

Doing script/clean and script/bootstrap helped a lot. The tests are now passing with v8 and spidermonkey. Still can’t get the tests to run with JavaScriptCore though, probably because something is wrong with my setup. But I am getting a test failure with Nashorn (https://gist.github.com/mneise/811d36fa1a565b17c5170ae875fb62d7).

Thanks for the tip @anmonteiro 🙂

testing with Nashorn now

And ./script/test-self-parity and ./script/test-self-host also work now 🙂

great!

@maria also getting some failures in Nashorn. relevant output (commented on your gist)

Mhh, I’m not getting as many failures as you though. For me, only one test fails.

could depend on the Java version

I’m using 1.8.0_60.

right.

java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)

I’m a little behind. updating

@maria OK only 1 failure now with 1.8.0_101

seems like we’re on the same page then

@juhoteperi ever encountered this error?

Exception in thread "main" java.lang.IllegalStateException: Cannot build without root node being specified

I’m trying out your patch with the latest React 15.4.0-rc.4

getting that error

ERROR: JSC_COMMONJS_MODULE_LOAD_ERROR. Failed to load module "react" at resources/public/js/libs/react-dom.js line 8 : 6
Exception in thread "main" java.lang.IllegalStateException: Cannot build without root node being specified, compiling:(/Users/anmonteiro/Documents/github/circle-color/scripts/build.clj:15:1)

I can paste the whole stack trace somewhere if it helps

Probably caused by some problems in processing modules and not properly handling errors

trying it with React 15.4.0-rc4 because I thought it would be properly consumed. They changed react-dom to be its own package, properly separating it from the “react” package

Given the path of react-dom.js file, it would probably have to require "resources/public/js/libs/react" instead of "react"

I don't think module processing can yet handle with npm packages

oh right

hrm

changing the path

that got me through that one

🙂

now for the next error

report-failure just prints the errors but later the code tries to read resulting code which probably will fail if there were errors

@juhoteperi oh boy.

compiled and works! 🎉

What kind of react and react-dom sources are you using?

just needed to change require('react)` to require('./react')

aha!

I think it’s the UMD module

one file for each react and react-dom?

yarn add [email protected] [email protected]

hmm, that should install npm packages, right?

then the files are in the dist folder

and I copied them over to resources

ah, npm package probably has concatenated files or something

oh right, it’s the huge browserified module

not the actual React sources 😞

still awesome though

Yeah

trying advanced compilation right now

can you point me to the externs for the SVG thing?

Not sure if this is the one I used: https://github.com/Hunga1/svg-externs

@juhoteperi pretty sure this means DCE, right?

if we can consume React & ReactDOM

Hmm yeah

Even if we consume browserified modules

Not sure if externs affect that in some way

Besides preventing name mangling

If this works, I can package cljsjs react packages with special identifier or something which will use module processing

not working with advanced compilation but I might be doing something wrong

Uncaught Error: EventPluginRegistry: Cannot inject event plugins that do not exist in the plugin ordering, `$SimpleEventPlugin$`.
    at $module$$181$$.$exports$ (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:2017:96)
    at $_dereq_$$47$$ (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1438:412)
    at Object.$injectEventPluginsByName$ (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1446:479)
    at Object.$inject$ (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1713:121)
    at Object.$r$$45$$.31.108 (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1549:111)
    at $t$$29$$ (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1345:42)
    at file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1345:145
    at Object.$r$$45$$.48.157 (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1682:155)
    at $t$$29$$ (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1345:42)
    at $e$$216$$ (file:///Users/anmonteiro/Documents/github/circle-color/resources/public/js/out/circle_color.js:1345:400)

@juhoteperi happy to push these changes if you want to try them out locally

@juhoteperi got it to work under advanced optimizations

but it was tricky. I had to change these properties to strings so that the Closure Compiler wouldn’t mangle them: https://github.com/facebook/react/blob/59ff7749eda0cd858d5ee568315bcba1be75a1ca/src/renderers/dom/shared/ReactDefaultInjection.js#L61-L66

I wonder if there’s a way to write externs for that

the problem being that they check for the properties on the object via strings: https://github.com/facebook/react/blob/c78464f8ea9a5b00ec80252d20a71a1482210e57/src/renderers/dom/client/eventPlugins/DefaultEventPluginOrder.js

this definitely confuses the Closure Compiler

@juhoteperi anyway, my experiments above only further validate your patch, so again, great job!

just add something like "var SimpleEventPlugin; var EnterLeaveEventPlugin..." to externs

or define them as properties under ReactInjection object

Or anywhere

As long as name "SimpleEventPlugin" is mentioned by extern, it will work

@juhoteperi: didn't know about that, I'll try in a bit

yeah, worked

awesome

also working with the minified build of React & ReactDOM

just need to set :language-in :ecmascript5

Closure could maybe convert the modules to es3 while processing them

hrm I think it does

Hmm

:language-out is ES3 by default I think

Aha. Perhaps lagunage-in for processing should be set to es5.

it’s only needed for the minified build in the case of React

but I agree, probably a better default nowadays

@juhoteperi the circle is not changing color though

must be something related to name mangling as well

Yep: https://github.com/mneise/circle-color/blob/master/src/circle_color/core.cljs#L25 the problem is handleColorChange

right but I changed that

might have forgotten to change something else

hrm doesn’t seem that ClojureScript uses :file-min for anything

With module processing, probably not

ah right

maybe it should with advanced compilation right?

I’ll open an enhancement ticket

http://dev.clojure.org/jira/browse/CLJS-1822

hrm I also don’t get why this needs to be js/Circle: https://github.com/mneise/circle-color/blob/master/src/circle_color/core.cljs#L26

@anmonteiro it doesn't

what would be the appropriate replacement?

Circle/Circle or something like that

I tried Circle/Circle but it didn’t work

I made some modifications and it was working for me, but I don’t have that on this machine

cleaning and building again

https://github.com/swannodette/circle-color/commit/b3d1d64af6727d782ade098a5f241ea5c351ede4

hrm Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined

@anmonteiro the problem is not in ClojureScript

oh.

the JS module doesn’t export anything

yeah I see it

@dnolen I’m making similar changes, but building with react and react-dom 15.4.0-rc.4

consuming both modules with just one minimal change

i.e. require(‘react’) -> require(‘./react')

now just one last hurdle to get through

there’s some weirdness going on with the react events

(only in advanced compilation)

I pushed my current attempt if anyone wants to take a look: https://github.com/anmonteiro/circle-color

specifically build (with advanced compilation) and check the console React warning when trying to write in the input field

@dnolen what is the trick that makes cljs.core available in bootstrapped cljs?

@viebel do you mean the standard library?

yes

there is no trick

Sorry I meant what is the trick that makes cljs.core macros available in bootstrapped cljs?

cljs.core is implicit

I mean macros

I made a typo in my previous question

it’s a special case, no “trick"

Where is it handled in the code?

hrm don’t recall

but search for cases of $macros

Searching...

likely in closure.clj

I see (js/goog.require "cljs.core$macros”) in js.cljs

But at run time it does nothing - it is already loaded

why do you think that?

I followed the code with the debugger

I would remain skeptical about your conclusion

goog.require from within a file is not what triggers a load

Closure deps files are what are used to load things in order

ok

I see cljs-source-for-namespace in closure.clj: There is a special treatment for cljs.core$macros

(if (= "cljs.core$macros" (str ns))
    (let [relpath "cljs/core.cljc"]
      {:relative-path relpath :uri (io/resource relpath) :ext :cljc})

@viebel that’s just about bootstrapped builds

what do u mean by “boostrapped builds?

that’s what I mean

builds vs. dev time stuff

bootstrapped isn’t really any different from any other kind of ClojureScript thing

the only difference is these special cases for cljs.core$macros

cljs.js has to manually require (it does do something)

and we need special cases for builds in closure.clj

Would it be possible to do the same for other macro ns e.g spec macros?

no

at least not without stating the point of doing such a thing

I mean for me inside KLIPSE

I don’t know

but I don’t know why you think need this solution (since I don’t have enough context)

that is - you’ve yet to state the problem you are trying to solve

I’d like to preload spec macro namespace and to bundle it inside KLIPSE - so when one does require spec.clj macro ns it doesn’t have to wait

makes sense?

@viebel I told you the other day you don’t need any special case in the compiler for that

Planck for example bundles its own AOTed macros

by AOTed I mean pre-compiled to JS

@viebel @anmonteiro is correct

there is no reason to replicate anything about the core macros

you just need to compile the macro namespaces you want to bundle separately and make a special case in your application when it encounters the need to load those namespaces

they are a special case which nothing else need participate in

just return the compiled JS

ok thx guys. will try it

@viebel the whole point of *load-fn* is to allow you to do whatever you need in your particular bootstrapped application

you could load namespaces from localStorage if you like

@viebel For background on what Planck is doing: http://blog.fikesfarm.com/posts/2016-02-03-planck-macros-aot.html And check out how Planck does it specifically here: https://github.com/mfikes/planck/blob/master/script/build#L53

Thx mike fot those precious pieces of information

@dnolen something else - related to clojure.spec: did you have time to take a look at the parsing with derivatives live snippets, running in the browser with KLIPSE http://blog.klipse.tech/clojure/2016/10/02/parsing-with-derivatives-regular.html

@viebel more specifically, this is where Planck writes the compiled JS out: https://github.com/mfikes/planck/blob/master/planck-cljs/src/planck/repl.cljs#L740-L743

@viebel don’t have time to look at it right now, sorry

😞

I thought I was so cool to be able to see this quite amazing algo running in the browser...

And let the reader play with the code

I have no doubt about that 🙂 just busy with some other things

I understand

I just wanted to share with you my enthusiasm 😃

It’s so amazing to see how you guys at Cognitect are able to take theoretical papers and make something very useful with it

Parsing with derivatives => clojure.spec Map tries => Clojure Immutable data structures

@anmonteiro advised me to look at the list of namespaces that could be skipped in self-host https://github.com/clojure/clojurescript/blob/master/src/test/self/self_parity/test.cljs#L166

the fact that cljs.core is skipped is understood

but how comes we could skip cljs.env or cljs.tools.reader ?

Doesn’t it break anything?

@viebel you can skip whatever library is already in the build

if you’re compiling with :simple optimizations, for example, there are certainly a lot more libraries that you can skip loading

simply because they’re already there

makes sense!

see this for example: https://github.com/mfikes/planck/blob/master/planck-cljs/src/planck/bundle.cljs

Is there a way to check - at runtime -a lib is already loaded?

that namespace doesn’t do anything at all, besides causing the libraries to be included in the build

Yeah. I used to do a similar trick in KLIPSE

if you then compile with simple optimizations, every library in there can be skipped

what about the macros ns?

(except the cases where there might exist macro files for them)

Why some macros ns could be skipped?

macros are an exception for this reason: if you require them during the JVM build they are expanded in Clojure

which means you won’t get the $macros namespaces that you need in bootstrap

@viebel AFAICT some macros namespaces can be skipped because their macros are already expanded in the build

You mean that their code will be avail at runtime in bootstrapped cljs?

that I’m not sure

let’s try it out. give me an example

pprint

what about it

cljs.pprint 'cljs.env.macros 'cljs.analyzer.macros 'cljs.compiler.macros

This is the list of macro ns that are skipped

For instance print-length-loop macro inside pprint

@viebel hrm so I think e.g. the pprint case can’t be loaded in bootstrapped ClojureScript

So why is it skipped?

Or more exactly

why skipping it doesn’t cause troubles?

Right, I think skipping it doesn’t cause trouble because the macros are already expanded

I think @mfikes will have more insight into this particular case

I think I see what u mean

but you can’t use the macros directly

like require-macros ‘cljs.env.macros wouldn’t be able to expand them perhaps

I can require a non-macro ns that is already loaded even if it requires a macro ns

yeah!

because the macros used by this ns are already expanded

exactly

right, so I think we’re onto something

the exception is: if you wanted to use those macros directly, you’d have to not skip loading those namespaces

But if the ns my.ns is already loaded and we are skipping it - why would the macros ns required by my.ns be loaded?

I mean why would the code try to load them?

because it’ll load the dependencies for the namespace

even if we skip my.ns?

yes

https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/js.cljs#L364

you mean: skipping a ns doesn’t skip its dependencies?

yes, that’s what I mean

and that is by design, and a good thing!

you can have cljs.test in your bundle but not its macros for example

so you skip loading cljs.test but cljs.js will still load the macros that you need

replace macros with any other dependency that a namespace might have

why is it a good thing? if cljs.test is in your bundle then you can use it without loading the macros

and if you want to use the macro directly, you’ll have to load the macros ns separately anyway

!

@viebel right but in cljs.test you want to use the macros directly 🙂

deftest, is, are, all are macros

and what you’re saying would break implicit macro loading

so it’s only good for implicit macro loading?

I agree that it’s a good reason!

it’s not only for implicit macro loading

if a namespace has dependencies, they should be loaded when you ask for the namespace to be loaded

if you skip loading a namespace, doesn’t mean you have its dependencies in your bundle

or that they are current (think live-reloading, it’s a LISP!)

Makes sense.

I was thinking of using find-ns to detect if a namespace could be skipped

at least for non-macros ns

is it a good idea?

@viebel I don’t know. something like this would also probably work, but it’s a hack: (exists? js/my.ns)

Closure namespaces are just JS objects

yeah but then you’ll need to convert names

Originally, I chose to skip loading certain namespaces simply because I wanted to make progress without being blocked. Ideally, the underlying problems could be addressed and there would be no need to skip loading namespaces.

you know “-“ -> “_"

yes, that’s right

so you’d have to call munge

@mfikes in KLIPSE I want to skip ns loading to save time

@viebel right, but 2 things needs to be distinguished here: skip loading a namespace because you have it vs skip loading it because you can’t

Yeah

For instance core.match cannot be loaded at all

The code contains java code

@viebel FWIW, I found that it is possible to defer loading portions of the analysis cache by simply making it lazy.

@mfikes are you referring to the lazy-map thing you have in Planck?

Yes

gotcha

Also, my guess is that, JavaScript itself is handled in a lazy manner by engines. (It can choose to defer parsing, and modern engines certainly defer compilation.)

@mfikes can you send a link to the code?

https://github.com/mfikes/planck/blob/master/planck-cljs/src/planck/repl.cljs#L174

In cljs core.match macros use clojure.core.match: https://github.com/clojure/core.match/blob/master/src/main/clojure/cljs/core/match.clj#L3

Elaboration on it: http://blog.fikesfarm.com/posts/2016-01-04-planck-lazy-analysis-cache-loading.html

So there is no way to bring core.match into planck or klipse 😞

Yes. It is probably a few hours of porting effort. Much much easier than core.async if I had to guess.

what would u use instead of (:import [ Writer]) ? https://github.com/clojure/core.match/blob/master/src/main/clojure/clojure/core/match.clj

@mfikes hah! thanks to that blog post I found the V8 snapshot feature you were talking about the other day

http://v8project.blogspot.de/2015/09/custom-startup-snapshots.html

There is always a way, if you are determined 🙂

I know...

I was asking if you already found the way - for this special case?

@viebel I’d change that file to *.cljc and then make it conditional, and re-implment the pieces as needed in ClojureScript.

@mfikes back to the skipping of ns

do you think it’s a good idea for me - in KLIPSE - to use find-ns in order to detect if a ns is already bundled?

instead of maintaining a closed list of ns?

I don’t know enough about the consequences to have an opinion

what could be the consequences - if you are pessimistic?

I don’t know. (For something like that I would try it, gain an understanding of how it behaves, etc. I haven’t.)

why find-ns is boostrap only?

https://github.com/clojure/clojurescript/blob/e0242833188a0dca6ab5ec909f54e7e73239cebf/src/main/cljs/cljs/core.cljs#L10595