Skip to content

Conversation

@guybedford
Copy link
Contributor

@guybedfordguybedford commented Jun 10, 2020

This PR adds a small doc change to clearly define that "import" and "require" are always mutually exclusive in exports (if something was not required then it definitely was imported) when loading an ES module.

Currently there isn't clear guidance that when defining in "exports":

{"exports": {"import": "./main.mjs","require": "./main.cjs","default": "./main.js"}}

that it should never be possible to resolve to ./main.js.

For example, other tools and bundlers could interpret new Worker('pkg') to not be an import or require path at all. Or similarly for other mechanisms of loading in future. When we should always define that loading an ES module goes through the import path.

Otherwise, this leaves a semantic gap where users might find default matched by some tools in the above example. By narrowing this gap we ensure we can continue to provide predictability between tools.

//cc @nodejs/modules-active-members

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation. labels Jun 10, 2020
@guybedford
Copy link
ContributorAuthor

//cc @sokra

Copy link
Contributor

@hybristhybrist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically there may be "classic-script" as a 3rd option but I'm fine with pretending that it doesn't exists, as far as exports is concerned. Classic scripts don't have a notion of "import specifiers", really.

@bmeck
Copy link
Member

I'd just note that "mutual exclusivity" doesn't mean one or the other must be true, just that they both cannot be true, so it seems the example in the original comment about not being able to hit default still isn't clear from the doc change.

@guybedford
Copy link
ContributorAuthor

Thanks @bmeck for the logical assistance here... ok I've changed the wording to be explicit they that are fully complementary in matching. Let me know if that sounds better.

@guybedfordguybedford changed the title doc: clarify require/import mutual exclusivitydoc: clarify require/import complement exclusivityJun 10, 2020
@bmeck
Copy link
Member

sounds good

Copy link
Contributor

@sokrasokra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with that. That might be true for node in the current state, but I don't think that should be a general precondition.

Other conditions should be allowed for future extensions or alternative module formats.

Esm might be superseded by a new module format in a long distant future. For this you might want to use a new condition zap to point to the zap modules.

Non JavaScript module systems also want to use a different condition. For a css module using @import "package" you may want to use the @import condition name.

@sokra
Copy link
Contributor

Mutually exclusive seems fine to me. Meaning when import is defined, require is never defined, and vise versa.

@hybrist
Copy link
Contributor

To clarify here: the condition is called “import” because it refers to a module graph, not to a module file format. So it may point to a web assembly module or a JSON module, assuming those can be linked into an import-style graph of modules. Just like “require” doesn’t mean CommonJS file format. It could also point to .node or .json files which are also supported in require-style module graphs (potentially).

@guybedford
Copy link
ContributorAuthor

@sokra would it help if we specifically adjust the wording to distinguish between module and asset where asset can imply things like CSS references that are not represented by modules in the module graph? As @jkrems mentions, WebAssembly et al should still be considered as modules under this interpretation so they would be loadable under import.

guybedford added a commit that referenced this pull request Jun 28, 2020
PR-URL: #33832 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
@guybedford
Copy link
ContributorAuthor

Landed in f89530f.

@guybedfordguybedford deleted the doc-mutual branch July 11, 2020 06:30
MylesBorins pushed a commit that referenced this pull request Jul 14, 2020
PR-URL: #33832 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 14, 2020
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
PR-URL: #33832 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #33832 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.esmIssues and PRs related to the ECMAScript Modules implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@guybedford@bmeck@sokra@hybrist@GeoffreyBooth@nodejs-github-bot