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

Lodash breaks when transpiled twice #1916

Closed
gaearon opened this issue Jan 31, 2016 · 10 comments
Closed

Lodash breaks when transpiled twice #1916

gaearon opened this issue Jan 31, 2016 · 10 comments
Labels

Comments

@gaearon
Copy link

gaearon commented Jan 31, 2016

Original issue: reduxjs/redux#611 (comment)

Basically we started using Lodash as a dependency of Redux, and I noticed that UMD builds can no longer be tested from our Mocha tests. After more investigation I realized that they break if they are transpiled twice because Babel inserts use strict in the code, and then Lodash function () {return this;} global detection returns undefined.

I'm not sure whether this can be considered a Lodash issue. Definitely transpiling twice is wrong but some people do that due to bad configurations. We used to support transpiling twice, but now due to our new dependency on Lodash, transpiling twice is broken.

What do you think?

@jdalton
Copy link
Member

jdalton commented Jan 31, 2016

The Function('return this') is our Hail Mary attempt.

That means it's failing

freeGlobal and ((freeWindow !== (thisGlobal && thisGlobal.window)) && freeWindow) and freeSelf and thisGlobal

Is there a global it can attach to?

@gaearon
Copy link
Author

gaearon commented Jan 31, 2016

Oh, I see. I tried these steps and, if I recall correctly, I used require() in a node shell to verify that it breaks. It also definitely was breaking in Mocha tests when I made them require() that double-transpiled UMD build.

Or, to be more precise, it broke in Mocha tests with a normal UMD build because our Mocha configuration was transpiling it with Babel. That's how I discovered the issue in the first place.

Please see https://github.com/jdalton/redux/pull/1 for steps to reproduce

gaearon added a commit to reduxjs/redux that referenced this issue Jan 31, 2016
@gaearon
Copy link
Author

gaearon commented Jan 31, 2016

Interesting. Apparently return this is not even your code. It's what Webpack inserts:

    module.exports = isPlainObject;
    /* WEBPACK VAR INJECTION */}.call(exports, (function() { return this; }())))

Could this be an attempt to polyfill a global reference?

@jdalton
Copy link
Member

jdalton commented Jan 31, 2016

Yes, that's it. Ours, using Function('return this') is immune to the strict mode rules, but theirs, because it uses a function literal, will inherit the strict mode rules.

Our Function(...) use is a last attempt because it can cause issues in CSP restricted environments.

@gaearon
Copy link
Author

gaearon commented Jan 31, 2016

  node: {
    global: false
  }

in the Webpack config solved the issue.
I'll report this to Webpack.

@gaearon gaearon closed this as completed Jan 31, 2016
@jdalton
Copy link
Member

jdalton commented Jan 31, 2016

Without that though won't global be an undefined variable reference?

@gaearon
Copy link
Author

gaearon commented Jan 31, 2016

Oh right, it would in browser.

@jdalton
Copy link
Member

jdalton commented Jan 31, 2016

See webpack.config.base.js.

I added this as the plugins in the webpack.config.base.js

  plugins: [
    new webpack.optimize.OccurenceOrderPlugin(),
    {
      'apply': function(compiler) {
        compiler.parser.plugin('expression global', function() {
          this.state.module.addVariable('global', "(function() { return this; }()) || Function('return this')()");
          return true;
        });
      }
    }
  ]

@jdalton
Copy link
Member

jdalton commented Jan 31, 2016

Closing as invalid as it's not a Lodash specific issue (and we've resolved it).

I updated the global replacement with

"(function() { return this; }()) || Function('return this')()"

so that under normal cases it avoids Function(...).

Related to webpack/webpack#1963 and reduxjs/redux#1339.

@lock
Copy link

lock bot commented Jan 19, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants