@dnolen: the macro question in #C03S1L9DN originated from the stuff above. with @bensu’s help I figured out that using a ` wasn’t the right thing here:

(defmacro define
  [sym default]
  (let [defname (-> (str *ns* "." (name (symbol sym)))
                    (clojure.string/replace "-" "_"))
        goog-define (str "/** @define {boolean} */"
                         "goog.define('" defname "', " default")")]
    (list 'do
          (list 'declare (symbol sym))
          (list 'js* goog-define))))

@martinklepsch: re, CLJS-1389 before spending any more time on a particular solution I would first explore why just emitting CLOSURE_DEFINES under :none is not satisfactory

@martinklepsch: adding any new api should always be considered as a last resort

@dnolen: agree.

@dnolen: the problem is that overriding defines with CLOSURE_DEFINES only works for defines done with goog.define. So far so good. Now they still need the @define annotation. Plus when using goog.define you don’t actually declare a var causing undeclared var warnings down the line.

@martinklepsch: how did you come to this conclusion

the ticket has no links, nothing backing up any claims

ticket has a link

http://google.github.io/closure-library/api/source/closure/goog/base.js.src.html#l147

based on all threads about this issue previously what you are saying is simply not true

that link doesn’t prove anything

if you read the source it’s obvious you don’t need goog.define

I see that there’s a part missing. let me check.

all you need to supply is CLOSURE_DEFINES

goog.define is just sugar.

@martinklepsch: I’ve commented on the ticket what I think should be done.

it would require users to do nothing new

@dnolen: I think I did exactly the change you suggest and ran into problems but let me check again first

@martinklepsch: again all threads I have ever read say what I suggested work

if you can prove that it can’t the ticket needs to include documentation demonstrating this

Will do.

Thanks!

np

@martinklepsch: looks like someone forgot to elevate your privileges in JIRA

you can do pretty much anything to tickets I can outside of real admin stuff

I’ve assigned the ticket to you, assign it back to me whenever you like

great. thank you

@martinklepsch: rationale is much better now, in general tickets should proceed with that before proceeding further.

@martinklepsch: so it seems like we’ll want the define helper macro + support to construct CLOSURE_DEFINES from the compiler options under :none

yeah? The macro was just an experiment really. You think it’s the best option?

not sure how you can supply the JSDoc bit otherwise

The CLOSURE_defines stuff is trivial really.

yeah

there’s no benefit to this being a function

goog.define is a special thing like goog.provide or goog.require

yeah, wasn’t thinking about function but rather having something like (def ^:goog-define thing true) or so. That said I have no clue if that would be a good or terrible idea.

@martinklepsch: that’s a more invasive change not less

@dnolen: ok. will try to smoothen that macro then.

@dnolen: is there some fn that converts a cljs my.thing/sym? to my.thing.sym_?

cljs.compiler/munge

for this safe to use since it’s an internal thing

cool. Thanks

@dnolen: The right place to add this would be core.cljc is that correct? What’s the “flow” of making changes in there? evaluating the ns form in a repl throws a bunch of stuff at me.

@martinklepsch: never had any issues loading that file in a REPL

ok, must be something with inf-clojure then

@dnolen: can you point me to a macro that is similar to the define one? Don’t quite understand what to scope in :cljs conditionals and what not

@martinklepsch: don’t worry about :cljs for now

making anything new work in bootstrapped is super low priority

ah, k. This is what I have right now: https://gist.github.com/martinklepsch/2e4a20e98ead0ed6953d but it gives be Unable to resolve symbol: js* in this context, — mostly took inspiration from the other macros and they seem to do it the same way?

@martinklepsch: that macro seems unnecessarily complicated to me

tbh, my macro skills are non-existent — very happy to be educated

@martinklepsch: the less you do with js* the better

(goog/require …) preferred

(goog/define …) sorry

oh, right!

the only thing you want js* for is the JSDoc bit

yeah makes sense.

@martinklepsch: as far js* not working, see the arithmetic macros like +

@dnolen: my problems with js* not working weren’t caused by the macro stuff but rather by things like calling str instead of core/str.

this also causes exceptions with js* being unresovable which confused me

ah right, yeah cljs/core.cljc is a bit tricky now because of the circularity

I have something that works now: https://gist.github.com/martinklepsch/2e4a20e98ead0ed6953d — feedback appreciated

@martinklepsch: looking good

@dnolen: Cool!

so that + the necessary changes to cljs.closure is patch-worthy

cool. will upload something to JIRA in a bit then

@dnolen: is main injected during :whitespace?

no only for :none

because in theory CLOSURE_DEFINES would need to be present for whitespace as well

making this work with :whitespace is a nice to have not essential

at least as far as I understood — :whitespace is still uncompiled in Closure lingo right?

:none, :simple, :advanced are the modes we really care about

@martinklepsch: I believe so but would need to verify

I never use :whitespace

yeah agree, just making sure things are clear

hello all

I am hoping to contribute some effort to cljs development. what is the best way to get oriented as a newb?

http://dev.clojure.org/jira/secure/IssueNavigator.jspa?mode=hide&requestId=10616

this low hanging fruit query, not much there at the moment

@mikepence: make sure you have sent in your CA. Then scan through the tickets and JIRA and pick something that tickles your fancy

if you have questions ask here or on ClojureScript IRC

I and others that work on the compiler can help

how do I get a CA?

nevermind, found it

https://github.com/clojure/clojurescript/wiki

@dnolen: can’t get it to work in node. Does the node_bootstrap.js somehow create a new context where the previously set vars aren’t available anymore?

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

@martinklepsch: remember Node.js has modules

you need to be careful how you create CLOSURE_DEFINES so everything can actually read it

goog.global holds the global context in platform specific way

so make sure you set a property on that

so var goog.global.CLOSURE_DEFINES vs. just var CLOSURE_DEFINES basically?

well sans var

ah right yeah.

Dunno much about node tbh

if you really want something to global write to goog.global

in browser js/window of course, but in Node won’t work

goog.global gets you out of caring

@dnolen: thanks, that helped

@dnolen: I added a patch. Can add a second one with the sample thing if wanted.

@martinklepsch: thanks!

@dnolen: has been fun! thanks for all your guidance

@martinklepsch: patch looks OK, just needs to be rebased to master.

@dnolen: done. not sure if how I can remove the old patch

@martinklepsch: I tweaked the docstring, we actually munge :closure-defines for the user in cljs.closure/add-implicit-options

if some tool is not passing build options through cljs.closure/add-implicit-options that’s a bug

ah, great. I somehow assumed there is no processing being done there.

should have checked the claim that it doesn’t work

@martinklepsch: thanks for the patch, nice enhancement

Happy to get it in. Parameterized builds without custom macros and env vars across all optimization modes is really handy

it sure is, low hanging fruit like this is definitely some of the most impactful contribution

@martinklepsch: going to rename define to goog-define to make it more clear. cljs.core/define is just a bit too weird semantically

yeah, I was considering suggesting that but you seemed fine with it so 👍

define indeeed seems weird given that def is basically saying the same

anyways, go for it.