Dubious statement regarding reserved specifiers

[DerekNonGeneric]

https://nodejs.org/api/esm.html (permalink) states the following.

Specifiers may not begin with / or //. These are reserved for potential future use. The root of the current volume may be referenced via file:///.

However, at the very least, module specifiers beginning with / as arguments to the node CLI are working just fine for me on Debian, but not on Windows, which is evident in https://github.com/nodejs/node/issues/32952#issuecomment-616803680.

Rather than throwing an error stating the above quote, I am experiencing ERR_REQUIRE_ESM, which isn't consistent with the documentation. Perhaps a new error should be made to support this reservation.

[DerekNonGeneric]

Worth noting is that this is stated in the Terminology section of import Specifiers and may not necessarily apply to the CLI, so I wonder if I should close this issue or we really do need a new error here. Debugging this was quite challenging, so please bear with me. :)

[ljharb]

It shouldn’t work on any platform - perhaps we just have a bug?

[DerekNonGeneric]

Not working would imply throwing an error if I'm not mistaken.

[SimenB]

Related to nodejs/node#31710, no? Where, after some back and forth, I finally understood / is interpreted as the pathname in the url (so should be relative to cwd). PR clarifying the docs are here: nodejs/node#32237. I might be misunderstanding, though 😀

[DerekNonGeneric]

Related to nodejs/node#31710, no?

No, it's not. This issue is about why there are no guards in place to enforce restrictions on specifiers starting with / or //.

This should fail to resolve.

[guybedford]

Agreed there is inconsistency here between the docs and implementation, so we should change one or the other.

[jonerer]

@guybedford agreed. IMO the implementation should change, otherwise we will have inconsistency between platforms. But I'll try to revive my documentation PR as a stopgap measure

@SimenB not quite. Starting with a / on MacOS is interpreted as "relative to root", which is to say "absolute". On node v13.11.0 and v14.0.0

[coreyfarrell]

Regarding nodejs/help#2642 (comment) I do not think the import specifier requirements should apply to the CLI execution script so I view this as a bug in node.js for Windows. I point to the web where <script type="module" src="script.js"></script> is valid but import 'script.js' is forbidden. So in the web the script source is valid because it uses HTML resolution algorithm. Similarly for node script.mjs CLI path resolution should apply to the main script being executed.

I do think the import specifier rules should apply to --experimental-loader, this is needed to as the experimental loader supports bare import specifiers.

[bmeck]

We should be wary with our verbiage and comparisons to HTML vs JS (web and/or node). In JS of the web spec the import specifiers are not relative URL strings, they only consist of a non-base relative URL strings on purpose. That is not true for the HTML attribute (in part due to historical reasons). I think equating the two is untrue and likely will lead to problems. That said, treating the CLI as having historical need to support something is fine.

[DerekNonGeneric]

IMO the implementation should change, otherwise we will have inconsistency between platforms.

I also agree that the implementation should change to re-align w/ what's stated in the documentation.

Perhaps a new error should be made to support this reservation.

The ERR_INVALID_MODULE_SPECIFIER code seems appropriate here, but I'd appreciate a more suitable message displayed if the implementation were to use this code.

[DerekNonGeneric]

I'm mainly interested in keeping / reserved so that we can eventually use that to mean package root instead of FS root. By doing this, we'd effectively sidestep any potential debate about the sigil character since there would be no need for one.

[ljharb]

(Note that there’s other problems there, namely that it will be unlikely to get consensus given that “/“ universally means system root - but that’s also a reason to keep it reserved)

[DerekNonGeneric]

I've gone ahead and implemented this specifier restriction in a loader that modifies the default resolve hook to throw an ERR_INVALID_SPECIFIER error upon failure. This is what I imagine the expected behavior would be if the implementation were to match what's stated in the docs.

https://gist.github.com/DerekNonGeneric/9cadd7bea251dd684e78a273445fd193

This loader can be tested in v14.x using …

node --experimental-loader=./reservedResolve_userland.mjs ./foo.mjs

// foo.mjs
import * as ns from '/bar.mjs';

Notice I'm intentionally using a restricted specifier (/bar.mjs), which will trigger the error. By the way, I'm not suggesting the error message in this loader be the exact error message used by Node core if it is decided that the implementation should change, it's simply derived from what's written in the docs.

It might be nice to come to a conclusion on this relatively soon before folks start to get accustomed to the current behavior and that ends up not being how the final implementation turns out.

[DerekNonGeneric]

If one of the goals is to have browser interop, we should probably remove this restriction.

Refs: https://html.spec.whatwg.org/multipage/webappapis.html#resolve-a-module-specifier

Disclaimer: I don't know what the initial motivation was for making this reservation.

[bmeck]

Do we need to do anything here? Perhaps we could just add some docs to https://nodejs.org/dist/latest-v14.x/docs/api/cli.html#cli_1 about the argument being resolved as a file and not through a CJS or ESM loader?