doc: add current recommendation for ESM/CommonJS dual packages #27957
Conversation
Trott
added
the
author ready
| > it will very likely entail changes to the behavior of `"main"` as defined | ||
| > here. | ||
| The `"main"` field can point to exactly one file, regardless of whether the | ||
| package is referenced via `require` (in a CommonJS context) or `import` (in an |
There was a problem hiding this comment.
The bracketed clarifications seem a bit ambiguous about whether it's the context of the caller or the callee module. I think we're trying to make a statement about the callee module. And it's not true that import implies either the caller or the callee (if we consider dynamic import()) will be in ESM context. I am finding this hard to word accurately and unambiguously. Maybe something like:
The
"main"field can point to exactly one file, regardless of whether the package is referenced viarequire(which will load the file as CommonJS) orimport(which allows Node's loading rules to decide whether to load the file as either an ES module or as CommonJS).
There was a problem hiding this comment.
I think the original is more correct. It is about which loader / module graph the module may be part of, not about the interpretation of the file contents. E.g. in a require context, it may be any number of file formats supported via require._extensions and in an import context it may be any content type supported by the import loader system.
The main point here is that it can only point to exactly one file on disk and it may only be instantiated/interpreted at most once: Either as part of the require module graph or as part of the import module graph (with the additional note that the import graph may point to nodes in the require graph).
Trott
added
author ready
|
I'd note that you can load it multiple times in both, if you `import`
(meaning using the ESM system loading mechanism) it would go through the
ESM system and `require` (meaning using the CJS system loading mechanism)
would go through the CJS system. That would have them be evaluated an in
both graphs, the intent at least is to explain that it *should* not be used
in both, not that it *cannot*. It would be hard to place non-race based
guards on this behavior.
```js
// foo => `console.log(globalThis.id); globalThis.id++;`
globalThis.id = 1;
require('foo');
import('foo');
```
Would log:
```
1
2
```
I do not think preventing modules from being loaded in both systems is a
useful pursuit as the legacy behavior of CJS allows it to load virtually
any file (even `.wasm`) without concern for new systems like ESM. We should
just keep the loading separated, not try to form some logic that adds
complexity to try and tread the 2 distinct systems as the same (which would
likely mean all code must match previous CJS semantics, which is odd).
Overall I think this PR is finely worded and thing using "context" is fine,
if another wording choice is desirable I think "system" or "mechanism"
would work.
…On Fri, May 31, 2019 at 2:17 PM Jan Olaf Krems ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In doc/api/esm.md
<#27957 (comment)>:
> @@ -206,16 +206,15 @@ a full path including extension: `"./index.mjs"`, not `"./index"`.
If the `package.json` `"type"` field is omitted, a `.js` file in `"main"` will
be interpreted as CommonJS.
-> Currently a package can define _either_ a CommonJS entry point **or** an ES
-> module entry point; there is no way to specify separate entry points for
-> CommonJS and ES module usage. This means that a package entry point can be
-> included via `require` or via `import` but not both.
->
-> Such a limitation makes it difficult for packages to support both new versions
-> of Node.js that understand ES modules and older versions of Node.js that
-> understand only CommonJS. There is work ongoing to remove this limitation, and
-> it will very likely entail changes to the behavior of `"main"` as defined
-> here.
+The `"main"` field can point to exactly one file, regardless of whether the
+package is referenced via `require` (in a CommonJS context) or `import` (in an
I think the original is more correct. It is about which loader / module
graph the module may be part of, not about the interpretation of the file
contents. E.g. in a require context, it may be any number of file formats
supported via require._extensions and in an import context it may be any
content type supported by the import loader system.
The main point here is that it can only point to exactly one file on disk
and it may only be instantiated/interpreted at most once: Either as part of
the require module graph or as part of the import module graph (with the
additional note that the import graph may point to nodes in the require
graph).
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
<#27957?email_source=notifications&email_token=AABZJI5QOW33BQB5ZMNPGZDPYF2WHA5CNFSM4HQVDEEKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2JG2SI#discussion_r289519477>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABZJIZTUHWZ6AZXRXRZDT3PYF2WHANCNFSM4HQVDEEA>
.
|
|
Does this PR need anything else? |
|
Landed in 62ac84b |
PR-URL: nodejs#27957 Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
|
@Trott Thanks! When does the docs site get updated? On each Node release? |
Yes. Of the currently-supported versions of Node.js (8, 10, and 12), which should get this update? (The update will only happen if this commit is pulled over to the correct release line branches.) |
Only 12, at the moment, as this is only applicable to the Node 12+ |
PR-URL: #27957 Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
This PR updates the ECMAScript Modules docs to include a recommendation from the @nodejs/modules team regarding dual CommonJS/ESM packages. Per our current roadmap, there is a good chance that the current support for dual CommonJS/ESM packages is what will be the final support when the
--experimental-modulesflag is dropped; the package organization method suggested in this PR would then become the recommended way for public package authors to support both CommonJS and ESM consumers of their package, should the author desire to publish both types of sources within the same package. If either of the other dual package-related proposals still under consideration end up shipping, this recommendation would still be valid.Checklist