Bump os-locale (add support for ESM dependencies) #5415
Conversation
| @@ -27,7 +28,7 @@ module.exports = { | |||
| '^.+\\.js$': 'babel-jest', | |||
| '^.+\\.txt$': '<rootDir>/tests/jest-raw-loader.js', | |||
| }, | |||
| transformIgnorePatterns: ['<rootDir>/node_modules/'], | |||
| transformIgnorePatterns: ['<rootDir>/node_modules/(?!os-locale)'], | |||
There was a problem hiding this comment.
Standard config for ESM packages
| // and make the import silently asynchronous. In most cases this breaks the import. | ||
| const dependenciesToBundle = [ | ||
| 'os-locale', // Used exclusively in sync functions | ||
| ]; |
There was a problem hiding this comment.
As expected, there's "no way" to require a ESM package synchronously. This is not a webpack/config issue. The only way for this to work is to turn addons-linter into an type=module package.
So this is the alternative: do not keep os-locale as an external dependency, so that node does not try to require it at runtime.
| module.exports = { | ||
| // Set the webpack4 mode 'none' for compatibility with the behavior of the | ||
| // webpack3 bundling step. | ||
| mode: 'none', | ||
| entry: { | ||
| 'addons-linter': './src/main.js', | ||
| }, | ||
| target: 'node', |
There was a problem hiding this comment.
Webpack defaults to browserlist, if set. We have it already and it's set to node 16. This enables ESM support
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5415 +/- ##
=======================================
Coverage 98.77% 98.77%
=======================================
Files 51 51
Lines 2855 2855
Branches 867 867
=======================================
Hits 2820 2820
Misses 35 35 ☔ View full report in Codecov by Sentry. |
fregante
changed the title
|
Ping |
|
@fregante sorry to let you wait on this. The fact that we're essentially adding ESM support here is a bit scary to me (not necessarily a bad thing but not as straightforward as a simple version bump). We'll be looking into converting addons-linter to ESM before the end of the year (though that might not happen for various reasons...). In the meantime, we should probably look at this PR. |
|
"Adding ESM support" means bundling those ESM dependencies, with an explicit list of dependencies to bundle. No ESM dependency will actually be exposed to npm users. Once you do publish ESM, you can just drop/revert that part of the config and it will continue working. |
|
Yeah, indeed. |
transformIgnorePatternscorrectlyos-localev6 has huge savingsv5
v6