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

Async chunk loading breaks complex expressions in globalObject option #12924

Closed
noppa opened this issue Mar 18, 2021 · 5 comments · Fixed by #14806
Closed

Async chunk loading breaks complex expressions in globalObject option #12924

noppa opened this issue Mar 18, 2021 · 5 comments · Fixed by #14806

Comments

@noppa
Copy link

noppa commented Mar 18, 2021

Bug report

What is the current behavior?

// webpack.config.js
  output: {
    library: 'myLib',
    libraryTarget: 'umd',
    globalObject: 'typeof self !== \'undefined\' ? self : this',
  },
  optimization: {
    splitChunks: {
      chunks: 'all',
      minSize: 1,
    }
  }

// index.js
  await import('./chunk')

generates code

var chunkLoadingGlobal = typeof self !== 'undefined' ? self : this["webpackChunkmyLib"] = typeof self !== 'undefined' ? self : this["webpackChunkmyLib"] || [];
/******/ 		chunkLoadingGlobal.forEach(webpackJsonpCallback.bind(null, 0));

The ternary expression for globalObject isn't protected as an expression, but gets mixed with the surrounding code.
In case the condition is true (typeof self !== 'undefined') and consequent path is taken, the result of this expression ends up being the equivalent of

var chunkLoadingGlobal = self

which then throws in the next line

Uncaught TypeError: chunkLoadingGlobal.forEach is not a function

Similar code also gets generated to the async chunk

typeof self !== 'undefined' ? self : this["webpackChunkmyLib"] = 

which would also fail if the execution ever got that far.

If the current behavior is a bug, please provide the steps to reproduce.
Full minimal reproducible example in noppa/webpack-global-object-issue-repro.

What is the expected behavior?
The code in globalObject option should be treated as an arbitrary expression and either lifted to a variable or wrapped in parenthesis

var root = typeof self !== 'undefined' ? self : this
var chunkLoadingGlobal = root["webpackChunkmyLib"] = root["webpackChunkmyLib"] || [];

The problem is easily enough fixed on the user's side:

-     globalObject: 'typeof self !== \'undefined\' ? self : this',
+     globalObject: '(typeof self !== \'undefined\' ? self : this)',

and I get that this issue could end up closed as "working as intended". However, I think that this limitation should then at least be mentioned in the docs and/or some kind of warning should be given. The nasty thing about the current behavior is that the code only breaks under very specific conditions:

  1. The app must contain async chunks. Without them, globalObject is only used in expression
    })(typeof self !== 'undefined' ? self : this, function() {
    which doesn't break the way the chunkLoadingGlobal code does. Simply adding /* webpackMode: "eager" */ to the import will "fix" the issue.
  2. The condition in ternary must be true, as the RHS of the ternary actually works fine. In practice this means that the app crashes in browser (where self is defined) but not in Node.

Thus, the broken globalObject code is already widely used in the wild: grep.app search for unparenthesized ternaries in globalObject option.

I believe that at least these issues could be caused by this:

Other relevant information:
webpack version: 5.26.3 and 4.46.0
Node.js version: 15.6.0
Operating System:
Additional tools:

noppa added a commit to noppa/ng-hot-reload that referenced this issue Mar 18, 2021
Without the parenthesis this resulted in invalid code
```
var chunkLoadingGlobal = typeof self !== 'undefined' ? self : this["webpackChunkngHotReloadCore"] =
```

See webpack/webpack#12924
@alexander-akait
Copy link
Member

To be honestly using () is always good, because it is always protect your code, but yes we can fix it

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@webpack-bot
Copy link
Contributor

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

@alexander-akait
Copy link
Member

Still valid

@vankop
Copy link
Member

vankop commented Nov 23, 2021

@alexander-akait I think better to add notes to docs. I think from option name is expected to get an identifier or member expression at least (not conditional expression) at runtime ok, I rethink.. lets fix it.

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