jashkenas / underscore

JavaScript's utility _ belt
https://underscorejs.org
MIT License
27.35k stars 5.53k forks source link

WebPack emulates require('underscore') incorrectly #2852

Closed dmaicher closed 4 years ago

dmaicher commented 4 years ago

I updated from 1.9.2 to 1.10.2 today and for me mixins are not working anymore when using require('underscore');

This fails with TypeError: _.camelcase is not a function:

const _ = require('underscore');
const underscoreString = require('underscore.string');

_.mixin(underscoreString.exports());

console.log(_.camelcase('foo-bar'));

but this works:

import _ from 'underscore';
const underscoreString = require('underscore.string');

_.mixin(underscoreString.exports());

console.log(_.camelcase('foo-bar'));
jgonggrijp commented 4 years ago

Sorry to hear that this is biting you, @dmaicher. However I cannot reproduce this with Underscore 1.10.2 and underscore.string 3.3.5. Is there anything special about your setup?

dmaicher commented 4 years ago

Thanks for your fast reply :blush:

I'm using webpack@4.43.0 and the issue happens with code bundled for the browser. When I run the code with node on the CLI then it appears to be fine :confused:

dmaicher commented 4 years ago

Here is a minimal reproducer: https://github.com/dmaicher/underscore_issue_2852

jgonggrijp commented 4 years ago

Here is your problem:

https://github.com/dmaicher/underscore_issue_2852/blob/348eef226ec7392b2ced2b92ddf2fcba2517ab5c/public/build/app.js#L13

That import path, underscore/modules/index-all.js, must WebPack have obtained from the module field in our package.json:

https://github.com/jashkenas/underscore/blob/5d8ab5e37c9724f6f1181c5f95d0020815e4cb77/package.json#L17-L18

whereas the main field is what it should have used, given that you are using CommonJS imports. module is for ES imports in tools that support them, main is for everybody else. This explains why your code works with import but not with require().

Apparently, WebPack nowadays assumes ES modules by default but has a fallback for CommonJS imports. I guess you'll have to configure it somehow to not assume ES modules or to ignore the module field. Or maybe you want to switch to ES modules.

I'm closing this now because this is a WebPack issue rather than an Underscore issue. Feel free to continue commenting, though.

dmaicher commented 4 years ago

@jgonggrijp thanks for checking :+1:

I was able to fix it by adding an alias for resolving in the webpack config: 'underscore': 'underscore/underscore.js',

But this seems a bit odd to me. Many other packages I use specify "main" and "module" inside their package.json and for them it works fine.

Webpack takes the first field it finds in the order ['browser', 'module', 'main'] See https://webpack.js.org/configuration/resolve/#resolvemainfields

I'm curious why it only fails for underscore but seems to be working for other packages :blush:

jgonggrijp commented 4 years ago

If you name those other packages, I'm happy to take a look. Anyway, module and main are not meant to be interchangeable, so WebPack's treating them as such is unwarranted:

https://github.com/rollup/rollup/wiki/pkg.module

The documentation you linked to hints at a perhaps slightly less ugly solution, which is to set resolve.mainFields to ['main']. Again, I believe WebPack shouldn't be considering module in the first place, since you are using CommonJS imports.

dmaicher commented 4 years ago

Just in case here are some packages I use that have both module and main:

https://www.npmjs.com/package/vuejs-datepicker https://www.npmjs.com/package/bootstrap-vue https://www.npmjs.com/package/acorn https://www.npmjs.com/package/perfect-scrollbar

dmaicher commented 4 years ago

Related webpack issue: https://github.com/webpack/webpack/issues/5756

jgonggrijp commented 4 years ago

Yes, that is your issue. Although there also seems to be something off with the way WebPack is emulating CommonJS when resolving to the module. It appears that it is doing something like this:

// converting modules/index-all.js to ES3
var underscoreESModule = {
    'default': _,
    map: _.map,
    filter: _.filter,
    // ...
};
// in your code that imports underscore
var _ = underscoreESModule;

The latter line would make sense if you were doing this:

import * as _ from 'underscore';

but what you're actually doing is equivalent to this:

import _ from 'underscore';

and the right way to translate that to ES3 in WebPack would be as follows:

var _ = underscoreESModule['default'];

This is also exactly the problem that other people are running into in webpack/webpack#5756.

Other build tools such as Browserify and Rollup handle this more intelligently. Just saying...

codeninja commented 4 months ago

Here is your problem:

https://github.com/dmaicher/underscore_issue_2852/blob/348eef226ec7392b2ced2b92ddf2fcba2517ab5c/public/build/app.js#L13

That import path, underscore/modules/index-all.js, must WebPack have obtained from the module field in our package.json:

https://github.com/jashkenas/underscore/blob/5d8ab5e37c9724f6f1181c5f95d0020815e4cb77/package.json#L17-L18

whereas the main field is what it should have used, given that you are using CommonJS imports. module is for ES imports in tools that support them, main is for everybody else. This explains why your code works with import but not with require().

Apparently, WebPack nowadays assumes ES modules by default but has a fallback for CommonJS imports. I guess you'll have to configure it somehow to not assume ES modules or to ignore the module field. Or maybe you want to switch to ES modules.

I'm closing this now because this is a WebPack issue rather than an Underscore issue. Feel free to continue commenting, though.

This reply just solved an issue I've been battling for 12 freaking days in our webpack build.

THANK YOU!

codeninja commented 4 months ago

@jgonggrijp thanks for checking 👍

I was able to fix it by adding an alias for resolving in the webpack config: 'underscore': 'underscore/underscore.js',

But this seems a bit odd to me. Many other packages I use specify "main" and "module" inside their package.json and for them it works fine.

Webpack takes the first field it finds in the order ['browser', 'module', 'main'] See https://webpack.js.org/configuration/resolve/#resolvemainfields

I'm curious why it only fails for underscore but seems to be working for other packages 😊

This reply just solved an issue I've been battling for 12 freaking days in our webpack build.

THANK YOU!