oh I know why

because of how the CommonJS stuff handles export

var foo = function() {}; modules.export = foo; pattern

will get compiled to

goog.provide(”module$foo”); module$foo$foo = function() {}; var module$foo = module$foo$foo;

busted

put that in a closure and it won’t work

yep

so maybe what we’re doing is wrong

and we should try to fix it in Google Closure

dang it's gonna start snowing soon here in NC

@dnolen btw about to release 0.5.9 with support for hot loading the foreign lib dirs

its pretty slick

nice!

@bhauman that is a feature that we probably would like to have in boot-figreload as well I will peek at your code 😀

(again lol)

🙂

heh yeah Google Closure actually has a test for that code gen - I’ll probably ask around whether that’s necessary - it’s not clear to me what semantic it’s preserving by doing things that way

hrm actually I do understand why it was done this way

because in compiled mode goog.provide gets erased

so it seems the output of CommonJS preprocessed by Google Closure really is incompatible with Node.js here

woohoo

just got Circle Color working with NPM deps

+ advanced compilation

@anmonteiro rock!

surprisingly the problem is with optimizations :none 😄

I don't know how to work around the process.env.NODE_ENV stuff

@roman01la's trick worked for advanced

@anmonteiro just make a process.env ClojureScript namespace with (goog-define NODE_ENV “development”)

hrm, maybe I just need to set! process in window or something

I did that, but it doesn't get picked up with :none

I suppose it needs to be a global

or I'm just doing something wrong

might be dep order.. hrm

that’s right

you need to load that ns first - fortunately dep order is guaranteed now 🙂

:preload, maybe?

you could do that

or just have your main ns order the deps

oh!

right, I put it as the last require

let's see if I move it to first place

hrm, no dice

@bhauman I don’t have a good answer for the __filename, __dirname problem. If it turns out to be a huge issue I may just wire in special support for that under :target :nodejs

want to hear more feedback first and whether there aren’t other subtler problems with global require for everything first though

@anmonteiro hrm ok

gotcha!

ohhh

the Circle Color project is 33.8KB gzipped

huh wow really? even with the ClojureScript bits?

or maybe Circle Color isn’t really using anything

if it’s not, that makes sense

it doesn't use anything CLJS, I believe

ok then that’s expected

52KB gzipped if I add:

(enable-console-print!)

(println "hi")

right

which is in line with your results I believe

yes it is

it’s not shabby that’s for sure 🙂

preloads seems to work for setting process.env

nice

weird thing is I get this now: Uncaught TypeError: this._instantiateReactComponent is not a function

at ReactCompositeComponentWrapper$$module$$Users$anmonteiro$Documents$github$circle_color$node_modules$react_dom$lib$instantiateReactComponent.performInitialMount

etc..

hrm

a solution is also to exploit the :preamble option

to set process.env.NODE_ENV

but then again, only works for :simple or :advanced

Hey guys. I'm trying out the new externs inference and I'm having trouble with this:

How would I typehint this correctly?

I tried this, but it didn't seem to work:

@mrg https://clojurescript.org/guides/externs#return-types

I read that @bhauman - but I don't know how that would apply in this case?

I haven't actually used it myself.

but from that tutorial you definitely can't hint an expression

ah I see what you mean

jquery is going to be rough in this regard

May come in the future. I just wasn't sure if I was doing something wrong.

Thanks, Bruce

@mrg to be safe you should always stay away from js* (defn jquery [sel] (js/jQuery sel))

that should make the whole inference more reliable .. but jquery is going to be tough regardless

Thanks @thheller

@mrg externs inference is designed to work for your case

but the guide probably needs to clarify the singleton pattern

(def jQuery js/$) is all you need

you don’t need any other hints if you ever use jQuery

I guess currently it may warn about adding externs to Object but you can ignore these if you like

it probably wouldn’t be hard to suppress those warnings in the case of this pattern - a top level tainted value

@dnolen chaining won't work without tags right? $('.thing').css().click().attr().etc()?

translated to cljs -> obviously

@thheller it could with what I’m suggesting

treating singleton pattern as a special case

since it’s quite common

what I mean is - it already works 🙂

you’re just going to get warnings about Object externs

but we could suppress those for this pattern

but you'll get externs for Object not jQuery?

yes

but I don’t think that matters much

well it matters in the sense that nothing named click will ever get renamed

but that doesn't matter much yeah

I'll give it a try. Thanks @dnolen

@dnolen with (def jquery js/$) I am getting the warning WARNING: Cannot infer target type in expression (. (jquery ".slider") slider (clj->js {"full_width" false}))

@mrg hrm ok will have to think about that

@dnolen there's a problem in cljs.build.api/add-package-jsons where it won't find the package.json for packages which :main entry is not at the root of the package

i.e. if :main is lib/Foo.js, it'll try to look for package.json in lib and bail when not found

@anmonteiro yeah, I didn’t know how common that was - so I didn’t bother handling that case

seems common enough for these 4 packages 🙂

@dnolen patch welcome?

yes, but what will the patch do?

I didn’t see anything obvious here

still trying to figure that out

just checking one level up seems too naive

it’s hard to say what the best approach is

would have been better of module-deps just returned that for us

that's what I'm looking into now

any reason the module_deps.js file has to be a single line?

would clojure.string/replace fail otherwise?

@anmonteiro no reason other than I didn’t test what happens if it isn’t?

if it works fine that way, go for it

I think I have something that works

another benefit is also dropping the JSONStream dep 🙂

sweet!

Foreign-deps from local deps.cljs files don't currently work with Boot-cljs because relative-name presumes that the files are under user.dir: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/util.cljc#L152

hrm that's unfortunate

@juhoteperi where are they placed in Boot-cljs?

It would work if the function tried to resolve file from classpath directories if it is not under user.dir

in temp directories, e.g. resources/foo.js -> home/juho/.boot/cache/tmp/home/juho/Source/saapas/mv8/uanrg/foo.js

And home/juho/.boot/cache/tmp/home/juho/Source/saapas/mv8/uanrg/ is in classpath

but how can you compute anything relative here?

that’s the actual problem

it doesn’t matter where you put anything

just whether you can effectively prevent clashes

The foreign library contents are resolved from classpath, so file path relative to classpath directory it is inside should be unique enough

Or well, they are first resolved from classpath, and then from working directory (using reader)

@juhoteperi ok, how about a patch that uses classpath if the file isn’t a child of user.dir

Working on it

@dnolen OK if I delete add-package-jsons entirely?

@anmonteiro go for it

it'll be in Git if we need to bring it back..

if it’s not needed anymore

👍

putting a patch together now

I'll let you know

cool

@dnolen just attached a patch to http://dev.clojure.org/jira/browse/CLJS-1920

great thanks, will take a look later

figured out why consuming React from NPM doesn't work with :none

https://github.com/facebook/react/issues/8894

just moving the assignment to after the function declaration fixes it: https://github.com/facebook/react/pull/8895/files

@anmonteiro got here to ask about exact same issue, we gonna fix React! 😄

🙂

@roman01la first the PRs need to be accepted 🙂

I'm confident they will, the changes looks fairly simple

yeah, also they do want to bundle React using Closure, so it makes sense to fix all these hacky JS things

right. I saw that, and it's a good thing that actually helps our side 🙂