doc: add current recommendation for ESM/CommonJS dual packages

[GeoffreyBooth]

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-modules flag 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

• documentation is changed or added
• commit message follows commit guidelines

[Trott]

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3635/

[robpalme]

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 via require (which will load the file as CommonJS) or import (which allows Node's loading rules to decide whether to load the file as either an ES module or as CommonJS).

[jkrems]

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).

[bmeck]

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> .

[GeoffreyBooth]

Does this PR need anything else?

[Trott]

Does this PR need anything else?

As long as there's nothing in @bmeck's comments that are blocking this, it can land.

@bmeck Can you confirm that this is A-OK to land as far as you are cocerned?

[Trott]

Landed in 62ac84b

[GeoffreyBooth]

@Trott Thanks! When does the docs site get updated? On each Node release?

[Trott]

@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.)

[GeoffreyBooth]

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+ --experimental-modules. As far as I understand things we haven’t backported the new implementation to 10 yet, though we hope to at some point.