Spec draft

[guybedford]

I've put some spec work together here implementing the proposal - nodejs/ecmascript-modules@modules-lkgr...guybedford:irp.

Feedback welcome.

[GeoffreyBooth]

Some notes:

• Do we want to perhaps rename ESM_RESOLVE to ESM_LOCATE? Since the overall algorithm is resolution.
• There’s some text farther up about ESM only supporting .mjs which needs updating.
• The “starts with “/”, “./” or “../”” section shouldn’t include “/”.
• I find it confusing that under HAS_ESM_PROPERTIES true, it talks about a mainURL. Since "main" is going to only apply to CommonJS, can we call this entrypointURL or similar?
• Re Note: ESM main yet to be implemented here., why not define it? Or at least say that it will follow the package exports proposal?
• Under ESM_FORMAT(url) step 2, we can’t short circuit to finding .mjs and returning ESM without traversing up to find the package.json file. A deep import inside a CommonJS package, e.g. import 'cjs-package/foo.mjs', needs to throw; otherwise the package scope (defined by the ESM-iness of package.json) would only apply to .js files within the scope and that would cause issues.

[guybedford]

Do we want to perhaps rename ESM_RESOLVE to ESM_LOCATE? Since the overall algorithm is resolution.

I think new terms should only be brought in when absolutely necessary to describe concepts. I see the conceptual distinction you're making, but I don't think this is a necessary conceptual distinction to make. While the Node and v8 loaders have resolve providing interpretation, resolving a specifier is clearly a well-defined location operation, and to call it locate is to bring in new confusions I think. To avoid this terminology problem entirely I've gone ahead and just combined the resolve and locate algorithms into a single algorithm while leaving the subroutine for ESM_FORMAT.

There’s some text farther up about ESM only supporting .mjs which needs updating.

Thanks, I've done this.

The “starts with “/”, “./” or “../”” section shouldn’t include “/”.

I updated this with your earlier feedback

I find it confusing that under HAS_ESM_PROPERTIES true, it talks about a mainURL. Since "main" is going to only apply to CommonJS, can we call this entrypointURL or similar?

The mainURL here is exactly and only the CommonJS main.

Re Note: ESM main yet to be implemented here., why not define it? Or at least say that it will follow the package exports proposal?

As mentioned the goal is to have separate specs that can be merged independently. To define terms is to lock down those property names, and thus lock the discussion of those property names to this PR, which was what I was trying to avoid. I exactly do link to the package exports proposal in the notes under HAS_ESM_PROPERTIES though.

Note that I can do a PR for the exports proposal and add everything all together, as I know you'd prefer. The point though was to break it down into reviewable subfeatures in the name of consensus. The more there is to discuss the more there is to block on.

Under ESM_FORMAT(url) step 2, we can’t short circuit to finding .mjs and returning ESM without traversing up to find the package.json file. A deep import inside a CommonJS package, e.g. import 'cjs-package/foo.mjs', needs to throw; otherwise the package scope (defined by the ESM-iness of package.json) would only apply to .js files within the scope and that would cause issues.

So an existing version of something like GraphQL that already ships ".mjs" files can never be loaded? That seems sub-optimal.

[GeoffreyBooth]

The mainURL here is exactly and only the CommonJS main.

But it's in the section where ESM is true?

So an existing version of something like GraphQL that already ships ".mjs" files can never be loaded? That seems sub-optimal.

It needs to define exports or mode in its package.json to do so. We can't have the package scope concept only apply to .js files. If random .mjs files inside a CommonJS package get loaded as ESM, then suddenly that package is dual-mode but we can't tell that from just reading package.json, and we need to do so to potentially throw errors or stop dual-loading of the same asset etc.

[guybedford]

But it's in the section where ESM is true?

Ahh, that's a bug then! Thanks it should be false not true. I've pushed that change.

We can't have the package scope concept only apply to .js files

The package scope was always considered a MIME lookup, changing the ".js" interpretation, but which can still allow ".mjs" to correspond to the ESM format.

It's definitely worth discussing further. To be honest I don't personally mind, but I get the feeling users will get very confused.

[GeoffreyBooth]

Well it's in the proposal:

If the above example’s cookies.js was renamed cookies.mjs, the theoretical import cookies from 'request/lib/cookies.mjs' would throw.

I think it needs to be this way to avoid some of the issues @bmeck was bringing up and that were being discussed in #1. If that's not the case then sure, let it work. But I think it's better to throw an error for now and potentially enable it later.

[guybedford]

".mjs" is perfectly deterministic - it provides a unique way to indicate the format.

CommonJS packages are already published that contain ".mjs" files.

To say that none of those ".mjs" can ever run is very prescriptive, for what gain? There is no determinism issue here.

I'm not sure what more I can say on this. I've pushed the change anyway.

[GeoffreyBooth]

Not the deterministic issue, the “loading the same asset in two modes” issue. Though that would seem to not be a possibility for .mjs, so maybe that’s not an issue here either. To be honest I can’t remember exactly why that language is in the proposal, though I know it was added for a reason that felt important 😄

Regardless, if you want to suggest a change in the proposal I think you should open an issue and we can discuss it there, then the proposal gets updated and the spec gets updated. We shouldn’t change things in the spec document (or even later, in the implementation).

One thing that does occur to me, and maybe this is just a UX concern and not a technical one, is that allowing importing .mjs within a package that lacks an ESM signifier in its package.json means that people can publish ESM or dual-mode packages without needing to include "exports" or "mode" in the package.json file. They could be like “just pull in my package via import graphql from 'graphql/index.mjs';“ and it would work. Then Node would have no way of knowing if a package is intended to be CommonJS-only, ESM-only or dual-mode other than scanning all the files inside the package. I feel like that would be a problem.

[guybedford]

The important file mode property is that of having a convention that allows all tools to know with 100% certainty whether a given file on the file system is CommonJS or ESM.

[GeoffreyBooth]

Closing as this has been merged into the upstream repo now.