Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: clarify require/import complement exclusivity #33832

Closed
wants to merge 2 commits into from
Closed

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford 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-bot nodejs-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
Contributor Author

//cc @sokra

Copy link
Contributor

@jkrems jkrems 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

bmeck commented Jun 10, 2020

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
Contributor Author

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.

@guybedford guybedford changed the title doc: clarify require/import mutual exclusivity doc: clarify require/import complement exclusivity Jun 10, 2020
@bmeck
Copy link
Member

bmeck commented Jun 10, 2020

sounds good

Copy link
Contributor

@sokra sokra 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

sokra commented Jun 11, 2020

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

@jkrems
Copy link
Contributor

jkrems commented Jun 11, 2020

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
Contributor Author

@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 <jan.krems@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
@guybedford
Copy link
Contributor Author

Landed in f89530f.

@guybedford guybedford closed this Jun 28, 2020
@guybedford guybedford 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 <jan.krems@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
@MylesBorins MylesBorins 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 <jan.krems@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #33832
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants