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

3.2.0 Global not defined with in isPlainObject #1346

Closed
jaidetree opened this issue Feb 1, 2016 · 16 comments · Fixed by #1349
Closed

3.2.0 Global not defined with in isPlainObject #1346

jaidetree opened this issue Feb 1, 2016 · 16 comments · Fixed by #1349
Labels

Comments

@jaidetree
Copy link

screen shot 2016-02-01 at 4 55 12 pm

I'm building our app files with browserify, babel 6.4, envify, react-redux, react 0.14 and the bundles are throwing this error that global is not defined. I take it envify or loose envify is not factoring out the globals?

For now I have reverted to 3.1.7 of redux and 4.1.2 of react-redux but of course it would be nice to see a real fix.

@gaearon gaearon added the bug label Feb 1, 2016
@gaearon
Copy link
Contributor

gaearon commented Feb 1, 2016

Thank you for reporting. What bundler do you use? We should definitely fix this but a temporary workaround would be to enable global polyfill in your bundler (e.g. Webpack does this by default).

cc @jdalton: this means libraries can't depend on Lodash modules using global without making assumptions about bundlers in browsers.

@gaearon
Copy link
Contributor

gaearon commented Feb 1, 2016

Another option would be to temporarily switch to UMD build (redux/dist/redux.min) which shouldn't have this problem.

@gaearon
Copy link
Contributor

gaearon commented Feb 1, 2016

The way I see it, unless Lodash agrees to fix it on their end reasonably quickly by removing dependency on global in CommonJS modules, we have to do the following:

  1. Release 3.2.1 and 4.2.1 that back out of using Lodash
  2. Either wait for Lodash to fix this or...
  3. Bump the major and tell CommonJS consumers we expect global to be polyfilled by their bundler when they use CommonJS build in the browser

I don't know how to enable global polyfill with Browserify but I assume it shouldn't be difficult.

@phated
Copy link

phated commented Feb 1, 2016

@gaearon browserify is supposed to pick up whether the global variable is reference and place it into the arguments list.

@gaearon
Copy link
Contributor

gaearon commented Feb 1, 2016

Maybe detectGlobals Browserify option isn't true in your config? See lodash/lodash#915 (comment) which is about the same exact issue.

@phated
Copy link

phated commented Feb 1, 2016

^ was just going to post that - the other thing that might be wrong is having noParse set on Redux or all node_modules or something

@phated
Copy link

phated commented Feb 1, 2016

@gaearon in relation to #1346 (comment) - see #1339 (comment) where JD mentions he will be releasing isPlainObject on npm without the global reference in the next version.

@jdalton
Copy link
Contributor

jdalton commented Feb 1, 2016

I can release the update this evening.

@gaearon
Copy link
Contributor

gaearon commented Feb 1, 2016

Awesome, thank you for quick response!
I'll leave this open then, and when you're ready ping us, and I'll update the minimal version.

@jdalton jdalton mentioned this issue Feb 1, 2016
@jdalton
Copy link
Contributor

jdalton commented Feb 1, 2016

Awesome, thank you for quick response!

No problem. Third-party integration, like Redux, is our top priority.

I'll leave this open then, and when you're ready ping us, and I'll update the minimal version.

You'll also be able to drop the global webpack plugin addition too.

@jaidetree
Copy link
Author

Oooh awesome! Glad a fix is on the way. Anyway, that is a good catch I do have detect globals off and while enabling it would fix the immediate error it wasn't needed before so it seemed worth reporting.

@vwochnik
Copy link

I'm still having the issue of the missing global for this dependency. I'm using rollup with rollup-plugin-node-resolve, rollup-plugin-babel as well as rollup-plugin-commonjs.

@gaearon
Copy link
Contributor

gaearon commented Jun 29, 2016

Have you updated Redux? This shouldn't be an issue with newer versions of Lodash.

@vwochnik
Copy link

I have tried the whole thing yesterday and it is having this issue. May I provide a minimal test repository?

@vwochnik
Copy link

vwochnik commented Jun 29, 2016

Well, I have just created a test repository with the issue. Note that after compilation, the dist/main.js file contains code referring to the process global variable.

@gaearon
Copy link
Contributor

gaearon commented Jun 29, 2016

Well, I have just created a test repository with the issue. Note that after compilation, the dist/main.js file contains code referring to the process global variable.

So is it global or process that you’re having issues with?

Redux uses process.env.NODE_ENV under expectation that your bundler will replace it with "production" or "development" during envification. Browserify and Webpack can do this. Is this something Rollup doesn’t do?

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

Successfully merging a pull request may close this issue.

5 participants