@anmonteiro @dnolen Did you test circle-color advanced build with older versions of Cljs? I'm seeing Closure error + empty output with 1.7.145 and 1.9.229

@juhoteperi definitely Closure error with 1.7.145

only warnings with your patch

@juhoteperi I get output with your patch applied, but it doesn’t work

Yes, Closure has changed "constant module$resources$public$js$libs$react assigned a value more than once." from error to warning

exactly

but the app doesn’t really work even with externs

also need to make sure you’re using externs

Oops, I had typo on externs option

Yep, seeing quarter of circle now

@juhoteperi that and the input doesn’t work

it’s what we both had

Problem with input change is very clear, and is caused by the cljs source

(. this -handleColorChange)

yeah

this handleColorChange custom method is introduced by cljs code, and not included in externs so calls get rewriten

right that makes sense

but that doesn’t explain the other issues

cx and cy properties on circle svg component get renamed to ak and bk (or something like that)

other properties work as they are included in some externs

Processed Circle.js: React$$module$resources$public$js$libs$Circle.createElement("circle", {cx:"100px", cy:"100px", r:"100px", fill:this.props.color})

Optimized: ta.createElement("circle", {ak:"100px", bk:"100px", r:"100px", fill:this.props.color})

Is there anything that can be done about this? Other than to add those properties to extern file?

hrm

@juhoteperi: oh because those are props and JSX doesn’t compile to object literals with string properties

Yes

oh but those are legitimate SVG properties no?

Yes

I checked and looks like JSX transform tool doesn't have option to output string properties

sorry what I mean is - isn’t there an externs for SVG?

(though JSX having a knob for output would also be useful)

Hmm, I don't see specific svg extern file at Closure repo

https://github.com/Hunga1/svg-externs/blob/master/svg.js

in general I guess I don’t see how this would be a problem actually

props would get renamed consistently

so JSX behavior is a good thing as long there aren’t string shenanigans somewhere

in user code

Works with that extern file

wow

React probably passes those properties as is to DOM

wow, great news

so now I wonder if #js needs something else 😛

hmm? #js emits string properties

right but that’s not what you want

since if everything is going through Closure, it will optimize all these properties

will think about it some more ...

But optimizing #js wouldn't work with anything that is outside of Closure, like browser built-in JS

@juhoteperi I’m not suggesting breaking #js

I’m just pointing out that this means for React stuff we just want to emit literals w/o strings

I've hurt for this a few times actually

yeah there’s other cases

We use #js for struct-like things but google closure tooling really wants struct to be literals and dict to be strings

easy thing is #jso, uses set! for properties instead of aset

or something similar

yes something like that, but should think about the name

but reader macro isn't essential, just nice sugar

ok - @juhoteperi anyways fantastic stuff - I poked around your patch and I couldn’t really think of anything specific

wrt. things that needed changing

it’s all implementation details as far as I’m concerned

Great. I'll work through FIXME notes and maybe tune the test.

@juhoteperi is there something about your patch that you feel doesn’t fall into this assessment?

@favila agreed

actually now that I think about it, to emit a literal js record (instead of a={};a.prop1=...) from cljs side, need a compiler primitive or at least a js*. Also need new JSValue-like wrapper on the clojure side if clojure macros want to emit the same.

yes

probably can just make JSValue have a flag

also I wonder if this should be a per file configuration thing

so we don’t have to add more syntax

that might get confusing. I run into it when I need to pass js value to other adv-compiled js from cljs

true

In that case I need to remember to either use my special anonymous-deftype macro on the calling side, or to use array-access on the callee side

this needs a ticket and a enumeration of the options

likely will need to stew on it for a while