Commit
- Loading branch information
There are no files selected for viewing
4 comments
on commit 0c709ba
There was a problem hiding this comment.
@ichernev Hey! I’m the author of moment-locales-webpack-plugin, coming here from iamakulov/moment-locales-webpack-plugin#33.
I’ve noticed Moment v2.25.2 added the module field which this commit then removed. Just curious: what problems did it cause with webpack? I’m happy to help to resolve them.
The problem with this commit is that it effectively reverts the library to v2.25.1 state. Webpack doesn’t recognize the jsnext:main field, and neither does Rollup, plus the field is generally deprecated. So publishing ES files in dist effectively does nothing: neither Node.js nor bundlers will pick it up.
There was a problem hiding this comment.
I’ve noticed Moment v2.25.2 added the
modulefield which this commit then removed. Just curious: what problems did it cause with webpack? I’m happy to help to resolve them.
#5484
also, thx for the elaboration in the commit, I'll look into it tomorrow.
There was a problem hiding this comment.
Aha, so the specific issue is #5484 (comment).
What exactly is happening there is that, with moment@2.25.2, when someone writes code like this:
import moment from "moment"
import "moment/locale/pl"
moment.locale("pl")they’re effectively importing two Moment.js versions into the bundle: the CommonJS one (moment/moment.js) and the ES one (moment/dist/moment.js). Because of that, apparently, when the code calls moment.locale(), it sets the global locale in the wrong instance of Moment, and the pl locale is never applied.
Why does that code import two versions of Moment?
- The ES version is imported because
import moment from "moment"looks at moment’spackage.json, notices themodulefield, and importsmoment/dist/moment.js - The CommonJS version is imported because
moment/locale/pl.jsdoesrequire('../moment')which, in turn, requiresmoment/moment.js
You can confirm this by looking at the webpack-bundle-analyzer report. Here’s the report for the code from #5484 (comment) and moment@2.25.3:
And here’s the report for the same code and moment@2.25.2:
How can this be solved?
The obvious solution is to replace import "moment/locale/pl" with import "moment/dist/locale/pl". But it’s impossible: we can’t teach everyone to change their imports when they’re upgrading to a newer Moment.js.
Instead, the following workaround seems possible. In all locale files, instead of requiring the ../moment.js file, require the package root:
--- a/moment/locale/pl.js
+++ b/moment/locale/pl.js
@@ -5,6 +5,6 @@
;(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined'
- && typeof require === 'function' ? factory(require('../moment')) :
+ && typeof require === 'function' ? factory(require('../')) :
- typeof define === 'function' && define.amd ? define(['../moment'], factory) :
+ typeof define === 'function' && define.amd ? define(['../'], factory) :
factory(global.moment)
}This would make webpack/Node.js/whatever resolve the import based on the package.json. The loader will look at package.json, pick the right file based on "module" or "main" fields, and will import that file. So if, with moment@2.25.2, the snippet from #5484 (comment) behaves as follows:
import moment from "moment" // → looks at package.json → follows the "module" field → imports "moment/dist/moment.js"
import "moment/locale/pl" // → does `require('../moment.js')` → imports "moment/moment.js"
// → Two moment versions are bundled– with this change, it would behave as follows:
import moment from "moment" // → looks at package.json → follows the "module" field → resolves to "moment/dist/moment.js"
import "moment/locale/pl" // → does `require('../')` → looks at package.json → follows the "module" field → resolves to "moment/dist/moment.js"
// → Only a single moment version is bundledNuance: If you apply this trick to moment/dist/locales/ as well, you’ll need to tune it slightly.
In moment/dist/locales/*.js, this code:
import moment from '../'
would resolve to moment/dist/index.js, not moment/package.json – and will fail. To fix that, you can maybe try
a) rewriting paths inside moment/dist/locales/ to do import moment from '../..' or
b) adding moment/dist/index.js with the code like
export * from './moment.js'Option a) is slightly better since it makes all code follow the same resolution algorithm (so it will have fewer weird edge cases).
There was a problem hiding this comment.
@iamakulov does the latest version fixed this issue.
I still see the error ./locale not found using version 2.27.0
Hey,
I was trying to get a clue as to why this "module" line was the culprit but couldn't find any information on it in the official nodejs documentation.
While I welcome the fact that webpack funtionality is fixed now, can you give me some hint or a link about this package.json property?