Fork me on GitHub
#cljs-dev
<
2016-10-15
>
maria01:10:10

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?

maria01:10:57

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

anmonteiro09:10:23

@maria tests pass for me with current master

anmonteiro09:10:55

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

anmonteiro09:10:08

maybe try script/clean and script/bootstrap again first

anmonteiro10:10:01

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

anmonteiro10:10:07

although that might not explain the whole thing

maria10:10:44

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).

maria10:10:33

Thanks for the tip @anmonteiro 🙂

anmonteiro10:10:38

testing with Nashorn now

maria10:10:42

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

anmonteiro10:10:30

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

maria10:10:06

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

anmonteiro10:10:15

could depend on the Java version

maria10:10:49

I’m using 1.8.0_60.

anmonteiro10:10:13

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)

anmonteiro10:10:19

I’m a little behind. updating

anmonteiro10:10:08

@maria OK only 1 failure now with 1.8.0_101

anmonteiro10:10:23

seems like we’re on the same page then

anmonteiro11:10:25

@juhoteperi ever encountered this error?

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

anmonteiro11:10:51

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

anmonteiro11:10:20

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)

anmonteiro11:10:29

I can paste the whole stack trace somewhere if it helps

juhoteperi11:10:57

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

anmonteiro11:10:31

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

juhoteperi11:10:45

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

juhoteperi11:10:00

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

anmonteiro11:10:23

changing the path

anmonteiro11:10:07

that got me through that one

anmonteiro11:10:14

now for the next error

juhoteperi11:10:25

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

anmonteiro11:10:19

compiled and works! 🎉

juhoteperi11:10:19

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

anmonteiro11:10:41

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

anmonteiro11:10:53

I think it’s the UMD module

juhoteperi11:10:05

one file for each react and react-dom?

juhoteperi11:10:31

hmm, that should install npm packages, right?

anmonteiro11:10:31

then the files are in the dist folder

anmonteiro11:10:40

and I copied them over to resources

juhoteperi11:10:49

ah, npm package probably has concatenated files or something

anmonteiro11:10:05

oh right, it’s the huge browserified module

anmonteiro11:10:15

not the actual React sources 😞

anmonteiro11:10:29

still awesome though

anmonteiro11:10:53

trying advanced compilation right now

anmonteiro11:10:05

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

anmonteiro11:10:08

@juhoteperi pretty sure this means DCE, right?

anmonteiro11:10:12

if we can consume React & ReactDOM

juhoteperi11:10:31

Even if we consume browserified modules

juhoteperi11:10:48

Not sure if externs affect that in some way

juhoteperi11:10:02

Besides preventing name mangling

juhoteperi11:10:26

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

anmonteiro11:10:10

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

anmonteiro11:10:28

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)

anmonteiro11:10:26

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

anmonteiro13:10:17

@juhoteperi got it to work under advanced optimizations

anmonteiro13:10:31

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

anmonteiro13:10:40

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

anmonteiro13:10:22

this definitely confuses the Closure Compiler

anmonteiro13:10:43

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

juhoteperi13:10:22

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

juhoteperi13:10:41

or define them as properties under ReactInjection object

juhoteperi13:10:46

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

anmonteiro13:10:03

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

anmonteiro14:10:02

yeah, worked

anmonteiro14:10:34

also working with the minified build of React & ReactDOM

anmonteiro14:10:47

just need to set :language-in :ecmascript5

juhoteperi14:10:15

Closure could maybe convert the modules to es3 while processing them

anmonteiro14:10:33

hrm I think it does

anmonteiro14:10:49

:language-out is ES3 by default I think

juhoteperi14:10:06

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

anmonteiro14:10:20

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

anmonteiro14:10:33

but I agree, probably a better default nowadays

anmonteiro14:10:51

@juhoteperi the circle is not changing color though

anmonteiro14:10:11

must be something related to name mangling as well

anmonteiro14:10:09

right but I changed that

anmonteiro14:10:21

might have forgotten to change something else

anmonteiro14:10:11

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

juhoteperi14:10:05

With module processing, probably not

anmonteiro14:10:58

maybe it should with advanced compilation right?

anmonteiro14:10:03

I’ll open an enhancement ticket

anmonteiro15:10:44

what would be the appropriate replacement?

dnolen15:10:30

Circle/Circle or something like that

anmonteiro15:10:35

I tried Circle/Circle but it didn’t work

dnolen15:10:47

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

anmonteiro15:10:13

cleaning and building again

anmonteiro15:10:41

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

dnolen15:10:42

@anmonteiro the problem is not in ClojureScript

dnolen15:10:57

the JS module doesn’t export anything

anmonteiro15:10:58

yeah I see it

anmonteiro15:10:30

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

anmonteiro15:10:41

consuming both modules with just one minimal change

anmonteiro15:10:02

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

anmonteiro15:10:44

now just one last hurdle to get through

anmonteiro15:10:06

there’s some weirdness going on with the react events

anmonteiro15:10:12

(only in advanced compilation)

anmonteiro15:10:28

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

anmonteiro15:10:03

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

Yehonathan Sharvit16:10:16

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

dnolen16:10:07

@viebel do you mean the standard library?

dnolen16:10:28

there is no trick

Yehonathan Sharvit16:10:29

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

dnolen16:10:33

cljs.core is implicit

Yehonathan Sharvit16:10:46

I made a typo in my previous question

dnolen16:10:32

it’s a special case, no “trick"

Yehonathan Sharvit16:10:51

Where is it handled in the code?

dnolen16:10:17

hrm don’t recall

dnolen16:10:27

but search for cases of $macros

dnolen16:10:11

likely in closure.clj

Yehonathan Sharvit16:10:33

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

Yehonathan Sharvit16:10:51

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

dnolen16:10:11

why do you think that?

Yehonathan Sharvit16:10:27

I followed the code with the debugger

dnolen16:10:28

I would remain skeptical about your conclusion

dnolen16:10:58

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

dnolen17:10:38

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

Yehonathan Sharvit17:10:03

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})

dnolen17:10:54

@viebel that’s just about bootstrapped builds

Yehonathan Sharvit17:10:22

what do u mean by “boostrapped builds?

dnolen17:10:27

that’s what I mean

dnolen17:10:39

builds vs. dev time stuff

dnolen17:10:15

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

dnolen17:10:31

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

dnolen17:10:54

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

dnolen17:10:05

and we need special cases for builds in closure.clj

Yehonathan Sharvit17:10:15

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

dnolen17:10:31

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

Yehonathan Sharvit17:10:33

I mean for me inside KLIPSE

dnolen17:10:51

I don’t know

dnolen17:10:22

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

dnolen17:10:03

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

Yehonathan Sharvit17:10:06

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

anmonteiro17:10:47

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

anmonteiro17:10:55

Planck for example bundles its own AOTed macros

anmonteiro17:10:05

by AOTed I mean pre-compiled to JS

dnolen17:10:50

there is no reason to replicate anything about the core macros

anmonteiro17:10:07

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

dnolen17:10:08

they are a special case which nothing else need participate in

anmonteiro17:10:22

just return the compiled JS

Yehonathan Sharvit17:10:36

ok thx guys. will try it

dnolen17:10:30

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

dnolen17:10:03

you could load namespaces from localStorage if you like

Yehonathan Sharvit17:10:18

Thx mike fot those precious pieces of information

Yehonathan Sharvit17:10:38

@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

dnolen17:10:45

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

Yehonathan Sharvit17:10:14

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

Yehonathan Sharvit17:10:27

And let the reader play with the code

dnolen17:10:32

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

Yehonathan Sharvit17:10:19

I just wanted to share with you my enthusiasm 😃 cljs

Yehonathan Sharvit17:10:13

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

Yehonathan Sharvit17:10:44

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

Yehonathan Sharvit19:10:30

the fact that cljs.core is skipped is understood

Yehonathan Sharvit19:10:05

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

Yehonathan Sharvit19:10:18

Doesn’t it break anything?

anmonteiro19:10:32

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

anmonteiro19:10:50

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

anmonteiro19:10:56

simply because they’re already there

Yehonathan Sharvit19:10:30

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

anmonteiro19:10:49

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

Yehonathan Sharvit19:10:08

Yeah. I used to do a similar trick in KLIPSE

anmonteiro19:10:18

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

Yehonathan Sharvit19:10:50

what about the macros ns?

anmonteiro19:10:52

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

Yehonathan Sharvit19:10:14

Why some macros ns could be skipped?

anmonteiro19:10:32

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

anmonteiro19:10:54

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

anmonteiro19:10:52

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

Yehonathan Sharvit20:10:16

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

anmonteiro20:10:43

that I’m not sure

anmonteiro20:10:02

let’s try it out. give me an example

anmonteiro20:10:16

what about it

Yehonathan Sharvit20:10:21

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

Yehonathan Sharvit20:10:31

This is the list of macro ns that are skipped

Yehonathan Sharvit20:10:58

For instance print-length-loop macro inside pprint

anmonteiro20:10:26

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

Yehonathan Sharvit20:10:13

So why is it skipped?

Yehonathan Sharvit20:10:31

why skipping it doesn’t cause troubles?

anmonteiro20:10:22

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

anmonteiro20:10:33

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

Yehonathan Sharvit20:10:18

I think I see what u mean

anmonteiro20:10:36

but you can’t use the macros directly

anmonteiro20:10:53

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

Yehonathan Sharvit20:10:05

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

Yehonathan Sharvit20:10:21

because the macros used by this ns are already expanded

anmonteiro20:10:46

right, so I think we’re onto something

anmonteiro20:10:05

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

Yehonathan Sharvit20:10:48

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?

Yehonathan Sharvit20:10:10

I mean why would the code try to load them?

anmonteiro20:10:39

because it’ll load the dependencies for the namespace

Yehonathan Sharvit20:10:52

even if we skip my.ns?

Yehonathan Sharvit20:10:19

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

anmonteiro20:10:36

yes, that’s what I mean

anmonteiro20:10:00

and that is by design, and a good thing!

anmonteiro20:10:21

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

anmonteiro20:10:41

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

anmonteiro20:10:54

replace macros with any other dependency that a namespace might have

Yehonathan Sharvit20:10:17

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

Yehonathan Sharvit20:10:41

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

anmonteiro20:10:01

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

anmonteiro20:10:12

deftest, is, are, all are macros

anmonteiro20:10:49

and what you’re saying would break implicit macro loading

Yehonathan Sharvit20:10:37

so it’s only good for implicit macro loading?

Yehonathan Sharvit20:10:47

I agree that it’s a good reason!

anmonteiro20:10:57

it’s not only for implicit macro loading

anmonteiro20:10:33

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

anmonteiro20:10:07

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

anmonteiro20:10:27

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

Yehonathan Sharvit20:10:14

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

Yehonathan Sharvit20:10:22

at least for non-macros ns

Yehonathan Sharvit20:10:29

is it a good idea?

anmonteiro20:10:09

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

anmonteiro20:10:31

Closure namespaces are just JS objects

Yehonathan Sharvit20:10:46

yeah but then you’ll need to convert names

mfikes20:10:52

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.

Yehonathan Sharvit20:10:54

you know “-“ -> “_"

anmonteiro20:10:56

yes, that’s right

anmonteiro20:10:22

so you’d have to call munge

Yehonathan Sharvit20:10:54

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

anmonteiro20:10:01

@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

Yehonathan Sharvit20:10:52

For instance core.match cannot be loaded at all

Yehonathan Sharvit20:10:04

The code contains java code

mfikes20:10:19

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

anmonteiro20:10:46

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

mfikes20:10:06

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.)

Yehonathan Sharvit20:10:16

@mfikes can you send a link to the code?

Yehonathan Sharvit20:10:28

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

mfikes20:10:02

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

anmonteiro20:10:29

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

mfikes20:10:33

There is always a way, if you are determined 🙂

Yehonathan Sharvit20:10:05

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

mfikes20:10:26

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

Yehonathan Sharvit20:10:24

@mfikes back to the skipping of ns

Yehonathan Sharvit20:10:51

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?

Yehonathan Sharvit20:10:07

instead of maintaining a closed list of ns?

mfikes20:10:40

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

Yehonathan Sharvit20:10:02

what could be the consequences - if you are pessimistic?

mfikes20:10:50

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

Yehonathan Sharvit22:10:59

why find-ns is boostrap only?