cosmiconfig
cosmiconfig copied to clipboard
davidtheclark
support `mjs` extensions out of box
@evilebottnawi Yeah, I'd be curious what implementation you think will work best. Is there a third-party loader that does this well already?
Looks like maybe the woefully underdocumented import() function might work for .mjs, wrapped in some error handlers for incompatible Node versions?
function importConfig(configSrc){
return (process.versions.node greaterOrEqual '13.2.0') ? import(configSrc).then(x=>x.default) : Promise.resolve(require(configSrc));
}
Why do we need to check the Node version instead of typeof import !== 'undefined'?
Does import() work with CommonJS files (.cjs or often .js)? The docs don't explain that, or the details about x.default. Is this better documented somewhere else? I'm hesitant to rely on precedent from Babel or Webpack or Rollup. But we should have tests verifying that it works, anyway.
Relying on import() is going to cause some problems with the synchronous API. Is there a synchronous alternative?
Only import without then, i will try to find time for implementing PoC and we will continue the discussion :+1: Busy right now, sorry, if somebody want to go ahead, PR welcome
@davidtheclark import is import keyword rather than a function，so we can't use typeof import !== 'undefined'. import cjs will export module {default: cjsModule} , corresponding module.exports = cjsModule in cjs export.
ES Module is async. maybe there has no build-in sync API provided. Unless we use rollup or std-esm or something else to translate esm into cjs.
Just curious, what is the status on this? I see there hasn't been an update in here for a few months now. Also, could we consider splitting cjs and mjs into two separate PRs?
If I'm not mistaken, currently this is preventing the use of the "type": "module" flag in a package.json for more recent versions of node (12.17.0 and higher) and at minimum supporting the extension .cjs with the current code should unblock the use of the "type": "module" property.
https://nodejs.org/api/esm.html#esm_package_json_type_field
Just in case the answer is yes to splitting the PRs, I opened #238 to speed things along with the cjs extension
Just wanted to share that I created get-package-type for use by nyc to determine how to load /path/to/nyc.config.js. I opened a couple issues on the repo to point out potential edge cases but nyc has been using it for about a month without issue. Example usage at https://github.com/istanbuljs/load-nyc-config/blob/2033a007672d90669c48c79e6a2d63a3cd0851a7/index.js#L66-L74. The actual import() is in ./load-esm.js because nyc supports some versions of node.js where import() is a syntax error, so it's only loaded when needed.
Looking forward to seeing support for at least cjs initially because, as @k2snowman69 mentioned, it's a real blocker at the moment for projects using type: module while tooling still uses commonjs syntax. Thanks for working on this!
Thanks for the PR, @k2snowman69. I'm going to try to look into this soon. I'm starting a v7 branch to merge upcoming breaking changes without making the repo's readme confusing. Looks like the first step is to remove support for Node 8, since that's become a problem for CI. Then I'll try updating dependencies. Then I'll look at your .cjs PR. And I'd like to see what it would take to add .mjs support in the same release, also, because people will be clamoring for that — but I won't linger on that too long if it seems tough, so we can get out .cjs support.
See https://github.com/davidtheclark/cosmiconfig/pull/241: default support for .cjs will arrive in v7.
I have a working implementation for a package defining "type": "module", and using a .js configuration file: https://github.com/davidtheclark/cosmiconfig/compare/v7...vvanpo:type-module
Here's an example usage:
@coreyfarrell it's using your get-package-type utility, works flawlessly.
However, I think we're blocked until Jest resolves https://github.com/facebook/jest/issues/9430. It appears that won't happen until at least --experimental-vm-modules is unflagged by Node.js. I've been trying to run tests with that flag, but I'm running into issues: https://github.com/facebook/jest/issues/9430#issuecomment-662832876
Sorry I'm not completely up to date here so what I'm about to suggest might not be relevant, but what about adding .mjs support behind a flag (disabled by default) that only works for async?
One problem I'm foreseeing is that once https://github.com/nodejs/node/issues/31860 lands and we're able to run tests against loadMjs() using node --experimental-vm-modules node_modules/.bin/jest, this will only work on the latest node release. I don't know if there are plans to backport these fixes to node@12, meaning the CI pipeline that runs against node@12 would fail.
Even if those experimental APIs/fixes do get backported, we still have a coverage problem. The conditional hiding the call to loadMjs() is dependent on the node version, so that branch would have no coverage for the CI run against node version 10, and Jest is configured to fail in that scenario.
@davidtheclark how do you think we should address this problem?
I suppose we could pull the Jest configuration out of the package.json and into a jest.config.js file, and then only enforce coverage on node versions greater than 10.
@vvanpo conditional testing sounds right. Test the feature only on the runtime version in which it is available.
@vvanpo conditional testing sounds right. Test the feature only on the runtime version in which it is available.
The type:module feature is available in Node 12. If Jest is a culprit for these tests, is it possible to use a different approach for testing this feature?
@just-boris it's not really Jest's fault, they're waiting on Node.js to sort out module support for their vm API, which is apparently causing them some trouble. So while we could potentially test this feature in a separate non-Jest script, currently all of cosmiconfig's tests are run through Jest and are reporting 100% coverage, so I'm not too keen on adding disjointed tests to the codebase as a temporary band-aid, and I doubt cosmiconfig's maintainer is either. I would prefer this work doesn't block the release of v7.0 (as type: "module" will still be unblocked via .cjs configuration files), and we can just add it as part of v7.1 whenever Node.js adds import() support to their vm API.
v7 has been released with .cjs support, so I'll change the title of this issue to reflect that .mjs support is the outstanding feature request.
What is the plan for ESM support? A lot of packages are moving to pure ESM at the end of April when Node.js 10 is obsolete.
Reviewing the thread above, it looks to me like we still don't have a strong suggestion for how this should work. The closest is the idea to use get-package-type (prototyped here); but as I understand it that approach is not compatible with the intent expressed in https://github.com/sindresorhus/np/issues/599 to support ESM even if there's no "type": "module" in package.json.
So this issue is still open to concrete ideas about how to best satisfy the intended use cases.
I'm going to be away from the computer on vacation for a week, but after that I'll check back for ideas and then see what I can do.
A comment regarding how this is implemented in Mocha.
However, it looks like require.extensions has been deprecated since Node.js v0.10.6, so that reasoning is probably not applicable here.
Regarding the Gulp implementation, the code for supporting Node.js <10 would not be needed here.
In both cases, it seems wasteful to attempt require first, so I think that checking the type in the closest package.json makes the most sense.
To support ESM without the package type being set to module or the file having a .mjs extension, you could maybe consider adding an option like forceEsm=(true|false). I don't know why you'd want to use that though, as the user would most likely still need to set the package type in order for Node.js to successfully import the file.
I started a PR in https://github.com/davidtheclark/cosmiconfig/pull/251, with ideas for how the interface, implementation, and testing could work. If anybody has suggestions for improving on some of the hacks or overcoming the CI testing problems, please share!
I'm not very motivated to work on this feature myself, so unlikely to commit another chunk of weekend time to it. If anybody is really interested in seeing ESM support, contributions will be the best way to make that happen.
Also, as that last paragraph indicates, the package could really use more active maintainers if substantial changes like this continue to be needed. If anybody is interested in playing an active maintainer role and working on features like this, please let me know.
Top level await is only allowed in .mjs files, so this may be one reason .mjs support is useful. But, configs using top-level await may bring about their own complications, because most configs are resolved pretty quickly. The one situation I was trying to use top-level await may be a little bit of a hack, I'm not sure: https://github.com/atkinchris/next-logger/issues/12
Just posted this note at the top of the README:
MAINTAINERS WANTED! If you're interested in taking over and maintaining Cosmiconfig, please let @davidtheclark know (with an issue or email). I'd like to hand over the keys completely, so I'm looking for owners, not people who just want to merge a PR or two! You can make the decisions about what happens in v8 and subsequent versions, how the package balances stability and opinionated features, and so on. Take a look at open issues and PRs to learn about possibilities that have been on people's minds over the years.
If you're interested in taking ownership of the package and moving this feature forward, let me know!
Here workaround - https://github.com/webpack-contrib/postcss-loader/issues/613#issuecomment-1331207803
There's a PR nearly ready to be merged - it just needs unit tests for ESM #283
