webpack: ProvidePlugin injection into .mjs files seems to be broken
Do you want to request a feature or report a bug?
A bug, I think.
What is the current behavior?
At runtime, a .mjs
module with injections from ProvidePlugin
will throw an error at runtime, indicating that require
is not defined. The final module code produced by webpack looks like this:
/*! no static exports found */
/***/ (function(module, exports) {
/* WEBPACK VAR INJECTION */(function(foo) {console.log(foo);
/* WEBPACK VAR INJECTION */}.call(this, require("/Users/aparker/src/notes/webpack4upgrade/mjs-provide/foo.js")))
/***/ })
/******/ });
(Note the call to require
.)
If the current behavior is a bug, please provide the steps to reproduce.
You can reproduce the behavior using this repository.
What is the expected behavior?
The module should look more like this:
/*! no static exports found */
/***/ (function(module, exports, __webpack_require__) {
/* WEBPACK VAR INJECTION */(function(foo) {console.log(foo);
/* WEBPACK VAR INJECTION */}.call(this, __webpack_require__(/*! ./foo.js */ "./foo.js")))
/***/ })
/******/ });
If this is a feature request, what is motivation or use case for changing the behavior?
Please mention other relevant information such as the browser version, Node.js version, webpack version, and Operating System.
node -v
:v9.11.1
webpack -v
:4.5.0
- OS: Mac OS 10.13.4
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 13
- Comments: 26 (14 by maintainers)
Commits related to this issue
- fix(build-config): remove mjs support when resolving files due to issues in webpack such as https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by robin-drexler 6 years ago
- fix(build-config): remove mjs support when resolving files due to issues in webpack such as https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by robin-drexler 6 years ago
- fix(build-config): remove mjs support when resolving files due to issues in webpack such as https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by robin-drexler 6 years ago
- fix(build-config): remove mjs support when resolving files due to issues in webpack such as https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by robin-drexler 6 years ago
- Fix build output to prevent webpack consume mjs file incorrectly https://github.com/webpack/webpack/issues/7032 — committed to ecomfe/standard-redux-shape by otakustay 6 years ago
- [PR] Add Buffer property to buffer module Summary: Previously discussed or implemented in #5153, #3495, #2938, #3723, #3724 and maybe elsewhere. This PR refines previous efforts by adding: 1. A cor... — committed to facebook/flow by motiz88 6 years ago
- fix: do not resolve "module" field in package.json https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by ZauberNerd 6 years ago
- fix: do not resolve "module" field in package.json https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by ZauberNerd 6 years ago
- fix: do not resolve "module" field in package.json https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by ZauberNerd 6 years ago
- fix: do not resolve "module" field in package.json https://github.com/webpack/webpack/issues/7032 — committed to xing/hops by ZauberNerd 6 years ago
- fix: disable ProvidePlugin (fixes #7032) — committed to aladdin-add/webpack by aladdin-add 6 years ago
- fix: disable ProvidePlugin (fixes #7032) — committed to aladdin-add/webpack by aladdin-add 6 years ago
- fix: disable ProvidePlugin (fixes #7032) — committed to aladdin-add/webpack by aladdin-add 6 years ago
- Merge pull request #8250 from Aladdin-ADD/patch-3 fix: disable ProvidePlugin for javascript/esm (fixes #7032) — committed to webpack/webpack by sokra 6 years ago
To add some context here, I’m getting an identical error when trying to bundle a module that references
process
with close to no config on my part (i.e. no explicit use ofProvidePlugin
). Would option 1 address the use case of referencing node builtins in.mjs
files?@ooflorent already did some refactoring to fix these injection problem in the
next
branch. You can send a PR adding a few test cases to check if it works correctly.Filed https://github.com/graphql/graphql-js/issues/1536 and https://github.com/graphql/graphiql/issues/721, but to me it seems like this is a webpack bug. I don’t see why a ESM file shouldn’t be allowed to use
process.env.NODE_ENV
like other files can.This is breaking
graphiql
. It depends ongraphql
, which contains a reference toprocess.env
: https://github.com/graphql/graphql-js/blob/dec24f9/src/jsutils/instanceOf.js#L19-L36Now, this file uses the
.js
extension, but it’s Flow, and all files get compiled to an ESM build which uses.mjs
and is referenced in package.json with themodule
field: https://github.com/graphql/graphql-js/blob/dec24f9/package.json#L7So when you want to use
graphiql
, and bundle your project with webpack, you get a.mjs
file with a reference toprocess.env.NODE_ENV
which is not getting replaced and triggers the above error.I changed our approach to polyfilling, which obviated the need to use
ProvidePlugin
for most of our use cases, but we too are usinggraphql
which is publishing.mjs
files and referencing Node APIs. How do you suggest we proceed in this case?I would prefer fixing this to actually perform the variable injection where needed into ES modules (i.e. have these global node APIs available) and am willing to work towards a PR for that. I am not willing to submit a PR that disables this kind of injection on .mjs files and breaks support for these libraries.
Actually option 2 is preferred, but it’s really complex to do. The way these stuff is injected is pretty old (and I hope we could get rid of this soon). So option 1 will disable this feature for now, so it doesn’t break anymore and we can figure out a way to improve the variable injection workflow.
Respectfully, would anyone care to elaborate on why option 2 isn’t the preferred one? I mean, I know next to nothing about the Webpack codebase, but I see
import
injection as the closest thing to “correct” in this scenario. Especially with regards to keeping Node globals working, which otherwise is quite the breaking change for users of libraries that rely on them.