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

UMD Production builds define global variables #10909

Closed
rauhs opened this issue Sep 28, 2017 · 6 comments
Closed

UMD Production builds define global variables #10909

rauhs opened this issue Sep 28, 2017 · 6 comments

Comments

@rauhs
Copy link

rauhs commented Sep 28, 2017

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

Bug

What is the current behavior?

The UMD production build linked in the announcement ( https://unpkg.com/[email protected]/umd/react.production.min.js ) define global variables y and Nb (react-dom).

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/ebsrpraL/).

Use the build with an app that defined the function or variable y, loaded before react.

What is the expected behavior?

React builds should not define any other global symbols other than React.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.0.0. It worked fine with some betas of 16.0.0.

@gaearon
Copy link
Collaborator

gaearon commented Sep 28, 2017

This is a bug. We should probably always wrap the UMD bundle into an IIFE.
Want to send a PR to fix it?

@rauhs
Copy link
Author

rauhs commented Sep 28, 2017

TBH, I've never built a JS project since I'm only doing Clojurescript so I have not the slightest clue where in the build process a fix would be necessary. :/

@gaearon
Copy link
Collaborator

gaearon commented Sep 28, 2017

I imagine we'll want to change this to do something more like this. And add a case for UMDs here. Or maybe we can just unify and do this for every bundle.

@gaearon
Copy link
Collaborator

gaearon commented Sep 28, 2017

I sent #10933 to fix this but now realize it doesn't help because Rollup UMD wrapper relies on this.

Now that I think of it, I'd like to know which tool is responsible for the top-level variable in the first place. I would expect Rollup to keep everything inside its wrapper. Is it Uglify then? Can we add an option to preserve top-level wrapper?

@gaearon
Copy link
Collaborator

gaearon commented Sep 28, 2017

Apparently it's GCC's doing.

@gaearon
Copy link
Collaborator

gaearon commented Nov 3, 2017

React 16.1.0-beta has been released. Please update react, react-dom, and react-test-renderer (if you use it) to this version and let us know if it solved the issue! We’d appreciate if you could test before Monday when we plan to get 16.1.0 out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants