2.2.0-rc.4's es6 modules' "module" usage detection seems to be broken

[SEAPUNK]

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

Bug

What is the current behavior?

External modules (symbol-observable, moment) no longer compile with this release.

If the current behavior is a bug, please provide the steps to reproduce.

1. Use webpack 2.2.0-rc.4
2. Compile a script that requires moment or symbol-observable (or another module that triggers this problem)

What is the expected behavior?

Bundle compiles fine

---

It doesn't appear that moment and symbol-observable modules actually use module.exports at the locations Webpack says there is an error.

For example, symbol-observable/es/index.js:

/* global window */
import ponyfill from './ponyfill';

var root;

if (typeof self !== 'undefined') {
  root = self;
} else if (typeof window !== 'undefined') {
  root = window;
} else if (typeof global !== 'undefined') {
  root = global;
} else if (typeof module !== 'undefined') {
  root = module;
} else {
  root = Function('return this')();
}

var result = ponyfill(root);
export default result;

Gives the error:

./~/symbol-observable/es/index.js
13:9-15 module is not allowed in EcmaScript module: This module was detected as EcmaScript module (import or export syntax was used). In such a module using 'module' is not allowed.

[terinjokes]

Adding to symbol-observable, it looks like lodash-es falls into this same bug.

import root from './_root.js';

/** Detect free variable `exports`. */
var freeExports = typeof exports == 'object' && exports && !exports.nodeType && exports;

[TheLarkInn]

By exporting And importing this is considering it an esm module. Esm has no access to local module or exports values even if only used as some sort of UMD wrap, which it appears to be here.

[jdalton]

Checking for the existence of a variable should not trigger the warning. It's valid JavaScript regardless of context. If possible, it would be better to detect assigning to the free variables in question:

exports.foo = 1;
// or
module.exports = {};

[TheLarkInn]

Was just asking about this. Thanks for the follow-up

[jdalton]

RE:me

If possible, it would be better to detect assigning to the free variables in question:

Ah, that might still falsely trigger it for UMD juggling which is safe. Maybe instead of a hard error treat it as a warning? Or get fancy and only throw the warning if the exports or module.exports assignments are done outside of an if block.

[SEAPUNK]

Or get fancy and only throw the warning if the exports or module.exports assignments are done outside of an if block.

I don't think that would catch all the ways this could happen, may it be in a try/catch, or even indirect assignment a-la exportMethods(module), which looks like what symbol-observable is doing. Not to mention that module and exports IIRC could still be validly declared and used as a plain variable, for whatever weird reason someone wants to use it.

I think it's better as a warning, instead of a hard error, with the possibility of disabling the warning on certain locations in the config. It'd prevent a vast majority of edge case bugs related to this.

[TheLarkInn]

@jdalton @SEAPUNK Is the vast majority of this UMD handling or interop checks you are guessing?

[SEAPUNK]

I think it is. Moment's code where the error appears looks slightly different, but I'd bet that it's the case for it too.

[jdalton]

Is the vast majority of this UMD handling or interop checks you are guessing?

For the falsely flagged cases, yes. For common UMD patterns see the umdjs repo. You may find most UMD cases check for the existence of free variables before use, so tripping a typeof check. While non-UMD cases just assign at will without existence checks.

[TheLarkInn]

🤔 We do have support based on the module and main field. I'm not saying we shouldn't fix this, but I feel like working together with lib authors to publish according to @defunctzombie 's https://github.com/defunctzombie/package-browser-field-spec

webpack's module resolution pattern respects this resolution spec. Just as workaround?

[TheLarkInn]

You may find most UMD cases check for the existence of the free variables before use so tripping a typeof check. While non-UMD cases just assign at will without existence checks.

I think thats a good start.

[dkrutsko]

It looks like it's also breaking on module.hot. Are we suppose to use something else now?

if (module.hot)
    module.hot.accept (...);

[jdalton]

In that case I think the warning (so instead of a straight error a warning) should be triggered since you're assuming the existence of module (so not checking it exists before using it).

[dkrutsko]

@jdalton: You're right. Although shouldn't there be a different way of implementing HMR now? Like through importing the module.hot function directly. Similar to how react-hot-loader does it with AppContainer?

[TheLarkInn]

@SEAPUNK Thank you for submitting your first issue btw!!! 🙇

[SEAPUNK]

Haha, no problem. I've been using Webpack for a while; it's a great project! Someday I hope to contribute to it.

[TheLarkInn]

Never too late to start :-D :-D

The suspect plugin starts from here is: CommonJSInHarmonyDependencyParserPlugin.js

I believe either in the individual DependencyBlocks or here, we need to conditionally hook into only certain types of ast aware logic. Like "Was this a free global access or assignment?".

[Jessidhia]

It looks like it needs "typeof module" and "typeof exports" plugins to replace them with the constant "undefined". I am not sure, but I think webpack should then detect the rest of the expression is unreachable and not parse it.

[Jessidhia]

@dkrutsko the module.hot expression, specifically, is supposed to work, at least until a syntactic replacement is available (e.g. a potential import.hot).

One thing that won't work is, say, passing the module object to a function that then checks for the hot property. Would need to use something like { get hot () { return module.hot } } in that case.

[griffinmichl]

redux and redux-form both use lodash-es, which means they produce this same bug

[jdalton]

redux and redux-form both use lodash-es, which means they produce this same bug

That's a maybe. redux uses the isPlainObject module of Lodash which may or may not (I haven't checked) access the freeExports or freeModule code path.

[griffinmichl]

One or the other is broken by this issue. They're the only libraries using lodash-es in my failing build.

[todd-richmond]

rc5 fails for me at runtime now instead of compile time with the same "exports is not defined" error as above. I'm using ES6 stage2 - not TS, but with redux-form and its lodash-es dependency

[heypoom]

Tested @TheLarkInn's commit, and it perfectly resolves the problem. Thanks a lot!

Before applying #3971: ("exports is not defined" error)

After applying #3971: (Works perfectly)

[aitboudad]

@TheLarkInn seems working only if I target commonjs "module": "commonjs", but with "module": "es2015" I get the following issue:

[aitboudad]

I think it's another issue, see for more details #3972

[jods4]

I can confirm that the example given by @niieani above (Aurelia loader detecting NodeJS) is fixed by rc5.

[jouni-kantola]

@TheLarkInn: I tried the #3971 fix, and now get the runtime error: Uncaught TypeError: Cannot set property exports of #<Object> which has only a getter

As I assume that is the expected error, couldn't a more helpful error message be displayed?
I'm thinking something like:
module.exports is readonly when using ES2015 modules. Do not mix import with module.exports.

[cesarandreu]

I was still getting some errors, but applying #3971 fixed them.

If you don't want to revert but some third-party dependencies have errors, removing 'module' from your main field might help:

{
  resolve: {
    mainFields: ['browser', 'main']
  }
}

[jods4]

I don't know if it's related or not at all but I have a parser plugin for call A.B. It worked fine but now that ES modules work, if my code looks like

import { A } from "x";
A.B();

Then my plugin code is not called anymore. Whereas this would:

// Assume A is global
A.B();

Is that expected or an issue?

[sokra]

@jods4 this is expected, as the parser (and plugins) only look for "free vars". If A is defined it's not a free var.

[mutsys]

I tried webpack@2.2.0-rc.7 last night with typescript@2.2.0-dev.20170117 and all was well. Issue seems to be resolved for me.

[mmahalwy]

@todd-richmond was this fixed for you?

I found this to be the culprit for babel-loader:

plugins: [ 'add-module-exports' ]

[todd-richmond]

@mmahalwy RC7 and GA both resolved my problems - thx

[stenvdb]

@phyllisstein I am getting the same error under the exact same conditions. Were you able to solve this? When I try to use export like so export * from 'foo'; I am getting the exports is not defined error. Using Webpack 2.2.1.

This seems to be related to the babel transform-runtime plugin, but not sure how to fix this. If I remove the plugin, my code works just fine. Any help would be appreciated!

[phyllisstein]

@stenvdb IIRC my diagnosis was similar to yours and I wound up removing that plugin. That said, I spent a few minutes trying to get back into that bad state and could not, with either babel-plugin-transform-runtime@6.23.0 or babel-plugin-transform-runtime@7.0.0-alpha.1.

If I'm reading @sokra's comment and the release notes for v2.2.0-rc.5 correctly, I should think it's actually no longer possible to hit the exact error I was seeing at the time. But unfortunately I've lost all state on this and can't be as useful as I might like. 😕

[stenvdb]

@phyllisstein Thanks for the response, I'm also on 6.23.0, in fact did a global update for all packages. But I'm definitely seeing the same error everybody here is talking about, and it's caused by that export * from statement.

If it helps, I'm quite new to this and I'm working with a tutorial for which you can find the source files here: https://github.com/morkro/tetrys
You see in this file the exact stament you and me had that caused this error: https://github.com/morkro/tetrys/blob/master/src/scripts/store/index.js
That can be of help perhaps to reproduce the error (so, when you use that code but with the transform-runtime plugin).

[phyllisstein]

@stenvdb Yowza, aside from wanting to gently and lovingly discourage you from spending even another second in that repo, it's hard to know what else to say without seeing the code you're working with. If you want to push that to GitHub I'll happily find a moment to take a look, but I don't have enough to go on from here.

[stenvdb]

@phyllisstein Wow thanks. I've uploaded my source files: https://github.com/Stenvdb/redux-test
The babel plugin transform-runtime is in comment in config/webpack.config.js. If you uncomment that and run again you'll see the error.

The repo mentioned above (tetrys) is just some source files for this tutorial btw https://www.sitepoint.com/redux-without-react-state-management-vanilla-javascript/

[phyllisstein]

@stenvdb Didn't want to keep polluting this issue; see my PR at https://github.com/Stenvdb/redux-test/pull/1. ✨

[Apidcloud]

Any news on this? I'm also getting the same error when using babel's transform-runtime.

[marlonmarcello]

I think I have the most simple set up and I'm still getting this error.

//package.json
"devDependencies": {
    "babel-core": "^6.26.0",
    "babel-loader": "^7.1.2",
    "babel-preset-env": "^1.6.1",
    "webpack": "^3.10.0"
  }

//webpack.config.js
module.exports = {
  target: 'web',
  entry: "./src/es6/wtc-swearjar.js",
  output: {
    path: path.resolve(__dirname, "dist"),
    filename: 'bundle.js
  },
  module: {
    rules: [
      {
        test: /\.js$/,
        exclude: /(node_modules|bower_components)/,
        loader: "babel-loader",
        options: {
          presets: ["env"]
        }
      }
    ]
  }
}