esm: Moving exports['.'] to an "index" field#30239
Closed
guybedford wants to merge 8 commits intonodejs:masterfrom
Closed
esm: Moving exports['.'] to an "index" field#30239guybedford wants to merge 8 commits intonodejs:masterfrom
guybedford wants to merge 8 commits intonodejs:masterfrom
Conversation
Contributor
Author
|
For discussion that is not direct PR review, please use the modules group issue at nodejs/modules#422. |
378a666 to
646cb3f
Compare
Contributor
|
For completeness sake, one potential alternative syntax for this would also be: {
"exports": [{
"require": "./main.cjs",
"default": "./main.mjs"
}]
}This is using the fallback syntax (array) which means that it also supports a short-hand for default: {
"exports": [{
"require": "./main.cjs"
}, "./main.mjs"]
}The downside is that we'd end up with a noisy array wrapper in this case that usually (which using the verbose nested syntax) isn't required around conditionals. |
14 tasks
Contributor
Author
|
As per discussion at today's meeting, we have decided not to implement this proposal for now, and instead will be looking to explore a conditional exports main sugar instead for this case. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR moves the
exports['.']main field to its own field as"index". Modules group discussion issue at nodejs/modules#422.We previously added sugar for the
exports['.']as just settingexports: Stringdirectly. This way"exports"can be thought of as the new main for modules.When setting this main, users get encapsulated packages by default, then when they want to add exports they can upgrade the String to an object with a dot property and other exports, which is a sensible upgrade step to make, and the dot export makes complete sense in this context as another export.
When considering conditional exports though, we have the problem that this same sugaring cannot apply because when exports is an object we cannot know if it is an object of export paths, or whether it is an object of conditions for the main.
The problem here is a user might start out with a package.json like:
{ "exports": "./main.js" }then decide to upgrade their package to support conditional exports with dual-resolution. They cannot simply change exports to an object at this point, but rather have to change it to an object with a dot property as well (a two-step process as opposed to a single step adjustment):
{ "exports": { ".": { "require": "./main.cjs", "default": "./main.mjs" } } }The idea of the
"index"field is to replace the"exports"sugar with its own field such that this simplifies the definition to just:{ "index": { "require": "./main.cjs", "default": "./main.mjs" } }@GeoffreyBooth tested against his data set and there were less than 50 cases of index in the wild I believe, with most of those already pointing to a main string.
We've previously been very cautious to get caught up on the definition of a new package.json field here if we could avoid it - which so far we have been able to by using
"exports"for both. But I believe the above usability scenario is enough of a concern to warrant reconsideration here, especially considering this is the field most users will set first for many Node.js development experiences, and small frictions matter at this scale.This PR is a follow-up to the conditional exports PR at #29978, and is based to that branch. The actual diff is only the last commit.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes