Prevent DOM tag name renaming by closure compiler in advanced mode

[piranha]

Without this change tag names will be changed by closure compiler: https://monosnap.com/image/1uZUvJewUyMLRvXm6nHXNL02z.png

form and textarea are ok by default (you can see form in screenshot above).

I'm not sure this is the best way to handle renaming, but I couldn't invent anything better - I wonder if it's possible to get renamed equivalent in runtime...

[SanderSpies]

Alright, that's unfortunate.

And how about using Google Closure Compiler externs? https://developers.google.com/closure/compiler/docs/api-tutorial3?hl=nl&csw=1#externs <- seems like the best solution at first glance.

Note: not a Google Closure expert here.

[piranha]

Externs are for the case when you're compiling library for external consumer, but when you're compiling it together with application you want to live without them. Plus they didn't work for me when I compiled them together, haha. :)

[sophiebits]

Perhaps we should write 'a': false and change the transform to output React.DOM['a'] instead of React.DOM.a…

[piranha]

But then you'll have to require that people which are not using JSX to write in the same way... I was planning to use React from ClojureScript and import functions just as functions. I could have a simple wrappers for them, of course, not a big deal, but I think that having method name and tag name separately is not bad. :) Plus it's not making code that much bigger it seems to me...

Though yeah, I shivered a bit because of non-DRYness of code, but managed to convince myself it's ok by using method name/tag name argument. :)

[zpao]

I'm going to be inclined to say no to anything that forces React.DOM['a'] though I might be able to get on board if that's optional in the transform for GCC compat.

This doesn't add that much to the build, but if we take it, it would be 10% of the increase over 0.4. I'm inclined to leave this be until after we ship 0.5. I'd like to get @jordwalke to take a look and see if he has any other crazy ideas here.

   raw     gz Compared to last run
 +1259   +413 build/react.js
 +1108   +405 build/react.min.js

[steida]

Externs for React https://github.com/steida/este-library/blob/master/externs/react.js

[piranha]

Thanks, I know about them and used them, but that is something I would love to live without. :)

[jordwalke]

Wasn't there an argument to the GCC command line that allows you to add additional whitelist/blacklists? That adds no overhead to the actual code.

[piranha]

Well yeah, it's called 'externs', I guess it could be used, I'm just not sure if that is a good solution to a problem.

[swannodette]

+1. Would love to just use React from ClojureScript instead of writing something like it from scratch - and I'm sure other folks using Google Closure Compiler in their pipeline would be happy to benefit too.

[sophiebits]

Why aren't externs appropriate for this case?

[swannodette]

Actually sorry I didn't quite understand what this pull request was about - I came across it in a fairly roundabout fashion. It does in fact seem to me now that externs are the correct solution.

[piranha]

Well biggest problem with externs is that they will have to be specified in every project which depends on React. I wonder if it's possible to annotate code somehow so that Closure Compiler won't rename keys of this particular object...

[steida]

I don't quite understand too. Which object? I am using React with Compiler in advanced mode with no problems.

[piranha]

Do you use externs?

[steida]

Sure.

[piranha]

That's exactly what I would like to not do. Externs serve for libraries included from elsewhere and I would like to compile React together with my application so that Closure Compiler can perform proper dead code elimination.

[steida]

I compiled it without externs. It wasn't easy, common js stuff etc., and result is: React code produces plethora warnings. I don't want to mess my code with that so I decided to use React as is. Check: https://github.com/steida/este-library/blob/master/Gruntfile.coffee#L108.

[steida]

Btw, React has 25KB gzipped roughly. That's fine. Non trivial app will leverage gzip even more.

[piranha]

It surely emits quite a bit of warnings, but nothing serious and it works, if compiled like that:

https://github.com/piranha/pump/blob/master/Makefile#L32

It's not only about size for me, but also a bit about convenience...

[piranha]

Anyway, this pull request is not relevant anymore, plus pump needs to be changed to leverage advanced-compiled React (and focus shifts to om anyway, which works with React a bit differently).