Fork me on GitHub
#cljs-dev
<
2015-10-20
>
bensu09:10:08

right, but at least you can add a notice in the release.

maria10:10:09

@dnolen: Tried deraen’s patch on a small personal project, but unfortunately having some problems when requiring cljs.js. Here is a minimal project to reproduce the problem: https://github.com/mneise/cljs-1437-test

maria11:10:44

Tried it with and without the patch and only got those errors once I applied the patch.

juhoteperi11:10:16

Strange. Only change to a cljc file is adding one function. Most of changes on closure.clj.

maria11:10:17

If you compare the dependencies included in cljs_deps.js you can see that there are two missing: cljs.core$macros and clojure.walk.

juhoteperi11:10:50

I think this is related to changing how dependencies are added during build process. Before, files were compiled and the compiled JS was checked for goog.require and goog.provide and then those CLJS ns were compiled.

juhoteperi11:10:11

After the change, only ns form is read from cljs files.

dnolen11:10:17

@maria thanks for the check! I recall that cljs.js needs special deps handling same as node.js

dnolen11:10:43

@juhoteperi: maybe this isn't there any longer? ^

juhoteperi11:10:31

@dnolen: There's different kind of special handling for cljs.js. get-compiled-cljs and cljs-source-for-namespace do special things.

juhoteperi11:10:11

get-compiled-js was used in cljs-dependencies and then dependencies were read from the compiled JS

dnolen11:10:51

@juhoteperi: right but it seems something is getting skipped?

juhoteperi12:10:04

@dnolen: Well the new code doesn't compile namespaces so it doesn't see dependecies which are generated from code using js/goog.require

dnolen12:10:50

Hrm not sure I understand yet. But I'll probably be trying this case out myself later today.

bensu20:10:51

@dnolen: ^ you were right yesterday: "we probably need to setOptions somewhere". It was initOptions simple_smile

dnolen20:10:30

@bensu awesome, thanks!

bensu20:10:59

@dnolen: let me know if the test is what you had in mind

dnolen21:10:13

patch looks great, thanks

dnolen21:10:16

testing now

dnolen21:10:55

worked pushed to master

bensu21:10:55

Then, I only have one other in the pipeline (re: node shim) which needs some peer testing

dnolen21:10:54

it’s the only one other than the dependencies thing I really care about getting into the next release

bensu22:10:29

@dnolen: the patch mentions files that look like this: BCE03FE.js I'm familiar with them but can't find where those names are created, or pin point why.

bensu22:10:33

do you have any pointers?

bensu22:10:14

I'm actually looking at one of your patches