node-semver
node-semver copied to clipboard
add support for node.js esm auto exports
This is an alternative to https://github.com/npm/node-semver/pull/325/
Node.js now supports ESM named exports via auto-detection, but the way node-semver
was creating module.exports
was breaking that algorithm. The advantage of this approach is that there is no need for a wrapper file or for package.exports
. It should be completely Semver Minor
This is a minor tweak to the layout of index.js
that allows for ESM named exports support in all modern Node.js ESM implementations.
We could borrow the tests from #325, but it it a bit obtuse to add a single test to the testing framework as there is a 1:1 file to test expectation.
Coverage remained the same at 100.0% when pulling b035c1d0905b4927f47df190544f68e6739fce2d on MylesBorins:alternative-esm into e79ac3a450e8bb504e78b8159e3efc70895699b8 on npm:master.
Any thoughts on merging this in?
I wonder if perhaps cjs-module-lexer could instead be updated to understand this pattern?
Is there anything preventing this PR from being merged?
@rxliuli merging this PR would mean that multiple packages in the ecosystem would need to follow it, and old versions never would - whereas https://github.com/npm/node-semver/pull/383#issuecomment-883865501 would mean that every version of any package following this package's pattern would Just Work - i don't think one-off changes to packages are a sustainable approach.
I still think we should land this @darcyclarke thoughts?
@ljharb I get where you are coming from regarding a philosophical approach here... but this change will in fact fix a usability issue with the package and is ready to land, not landing it on principal seems like a mistake. That doesn't mean we should try and fix this in the lexer as well. /cc @guybedford
@MylesBorins because of backporting constraints, I don't think lexer features should be changed. Object detection is just patchy unfortunately. I think we should only stick to updates for parser bug fixes that break new language syntax or fail on valid language syntax.
@guybedford what would be the backport constraint here? wouldn't this just add new named exports when backported?
The problem is we cannot expect users to track fine-grained exports detection rules as being compatible or not. For example if we made the lexer support these named exports in Node.js 18 only, it would be highly suprising when it breaks on older versions of Node.js, as opposed to other types of new features which can be clearly communicated as experimental.
Understood - although that seems true of virtually any feature, whether labelled experimental or not, and the solution to all of them is to run CI on supported node versions.
Yes, although the difference here is parser internals are difficult to communicate as features, in the same way that versioning parsers or transformers is hard.
Fixing this here fixes auto esm exports for the most highly downloaded package on the entire npm registry. Fixing the lexer is outside the scope of what node-semver needs to worry about today, and folks can go discuss that in the appropriate repo.