Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event plugins names are mungled in Closure Compiler under advanced optimizations #9417

Closed
roman01la opened this issue Apr 13, 2017 · 7 comments

Comments

@roman01la
Copy link
Contributor

Do you want to request a feature or report a bug?
a bug

What is the current behavior?
When React is compiled by Closure Compiler with advanced optimizations, names of injected event plugins are being mungled.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/84v837e9/).
Compile React with Closure Compiler directly from NPM.

What is the expected behavior?
The names of injected event plugins should not be mungled.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React v15.5.4
It didn't work in previous versions of React

Closure Compiler renames object properties, unless they are strings explicitly.
It happens because of the way how event plugins are injected here

  EventPluginHub.injection.injectEventPluginsByName({
    SimpleEventPlugin: SimpleEventPlugin,
    EnterLeaveEventPlugin: EnterLeaveEventPlugin,
    ChangeEventPlugin: ChangeEventPlugin,
    SelectEventPlugin: SelectEventPlugin,
    BeforeInputEventPlugin: BeforeInputEventPlugin,
  });

And later used here

  for (var pluginName in namesToPlugins) {
    var pluginModule = namesToPlugins[pluginName];
    var pluginIndex = eventPluginOrder.indexOf(pluginName);
@gaearon
Copy link
Collaborator

gaearon commented Apr 13, 2017

Yea. We're not really compatible with GCC right now and there are also a few places where we went for more clear code rather than GCC compat. I don't think it makes sense to fix GCC compat with one-offs because we'll just keep breaking it. Instead we should probably investigate enabling it by default in our build process now that we're switching to flat bundles. @trueadm is interested in this.

@roman01la
Copy link
Contributor Author

@gaearon Makes sense. Just want to note that since ClojureScript now allows to build JS deps directly from NPM it means that potentially this issue blocks transition to a new build pipeline.

@trueadm If I can help somehow with this, please let me know.

Thanks!

@anmonteiro
Copy link
Contributor

While it is an issue that React injects those properties dynamically, I've actually been down this road before and you're probably just using stale externs. Here's the commit where I fixed what you're seeing: cljsjs/packages@149363f

@roman01la
Copy link
Contributor Author

@anmonteiro I'm compiling directly from NPM, so no externs. But I guess this would help, thanks!

@HiroAgustin
Copy link

@roman01la were you able to find a work around for it? I'm running into the same problem using webpack-closure-compiler.

@roman01la
Copy link
Contributor Author

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

I'll close this as we don't have a goal of being GCC compatible yet. Rather, we'd like to GCC-compile ourselves: #11092. We'd need to fix up all those issues as part of that work, but I'd prefer to track them there in that issue.

@gaearon gaearon closed this as completed Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants