[pkg] exports: use module.require for nodejs - #3344

Ok the change looks good. Wow, we are causing a lot of trouble for webpack :)

👍

Merged in bb6a30e

um, this will break lots of tools - require is not the same as module.require.

Indeed, this PR caused a breaking change in a minor version (see #3431). Please revert it ASAP.

if use require, webpack will load all locale data into browser(which is fine for nodejs but too much for web app).

This is a breaking change, but any serious web app should not depend on it.

If webpack breaks on the standard and normal require, that's a webpack bug, not one individual packages should solve.

Also, if it's a breaking change, it should be reverted for a v2.15.1, and published as v3.0.0 (at which point, Airbnb will no longer be able to use moment.js, so hopefully that doesn't happen)

@yiminghe can you give more details about your environment so we can work out a fix that works on both phantomjs/karma and your env (webpack?).

@ichernev you can check the links in my pr. using require will cause browser to load all locales(too much for client) for webpack.

the idea usage(load on demand):

import 'moment/locale/zh-cn'; // import specified locale module
moment.locale('zh-cn');

@yiminghe should I flip the checks order? Can you suggest something that works for the existing use case? So 'require' works for webpack but requires all of it, and if you use module.require it doesn't?

The whole idea of this code is to ONLY work on node.js. In other envs it's intended to be used by first requiring only the locales you want (in code) and then switching the locale -- the snippet you provided.

Ok guys now it's reverted, so what's the plan now?

I use

import 'moment/locale/hr' # or require('moment/locale/hr'); the result is the same
moment.locale('hr')

and on version 2.15.1 it makes 150Kb heavier file (uglified) than on 2.5.0. So I had to revert to 2.5.0. 150K of dead weight is significant for web.

150k is a small-to-medium sized image.

It's definitely something that can be improved, but it sounds like a webpack bug. When I install moment locally and browserify a file that requires moment/locale/hr, the resulting file is 143K total with 2.15.1, and the identical 143K total with 2.15.0.

@ljharb agree, 150KB could not be a problem in many site, but in high performance ones you fight for every KB. And there is additional 150KB of code browser needs to parse as well.

When it's considered as a webpack bug is it known for them?

This isn't a bug in Webpack. Webpack's noticing that Moment (potentially) depends on all of the locales, and bundles them. This is the correct and expected behavior, albeit an undesirable one for most Webpack users.

I'd propose queuing this change back up for the next release that will include breaking changes. It's a breaking change, but a sensible one.

In what way is it sensible? It breaks require for every tool that's not Webpack.

@schmod Wasn't there a way to tell webpack to bundle only manually specified files? I'm pretty sure a solution floated around.

If there is a way to trigger that behavior from whithin moment, that is great, or at least we can document it on momentjs.com.

Lets see : webpack/webpack#3128