Specify import file specifier resolution proposal

[guybedford]

This PR provides a specification for the node-import-file-specifier-resolution-proposal.

Further to last meeting where some of the properties of this proposal were discussed, hopefully this can allow for a more concrete discussion about the interactions at work.

I think what would help is perhaps using this spec to try to understand some concrete use cases, and then fleshing out those discussions further.

This PR does not yet implement ES modules mains or subpaths as this is currently delegated to the package exports proposal.

Hopefully we can continue to discuss that proposal too in order to work towards a refinement of a complete specification here.

[guybedford]

I'll start with some of my own concerns (and suggestions welcome for the best way to have these discussions too):

1. Having import '/specifier' throw seems inconsistent with browsers to me.
2. Having import 'cjs/subpath.mjs' not work seems overly restrictive to me, where users that have already been writing mjs might expect it to work within CommonJS packages.

[ljharb]

I agree with both of those points.

[weswigham]

I get why people wanna flag these since the behavior on node isn't what one may expect coming from a browser, but.... paths are paths? And while the unix root (/) feels somewhat ambiguous thanks to webservers, a windows root c:/ is completely unambiguous (and usually gets lumped into having the same behavior as a unixy root). Paths simply require context to be resolved correctly - that's just how paths work, IMO. (Also: I know the cjs modules pages doesn't do this, but talking about rooted and unrooted would be better than using the exact string fragments here, to make the algorithm description read more cross-plat - as written, c:/foo.js is technically a c: package specifier, which I don't think is desirable or intended)

I don't think blanket-forbidding absolute module paths is necessarily something we should do - not that I've ever once used an absolute path to a module in any real scenario, it just feels strange to forbid them, since cjs resolution handles them fine.

[SMotaal]

@weswigham I am fairly certain that it is just a stepping stone, ironing out the more critical aspects first since "/" is obviously a resolvable matter but the extra work to get it right for everyone needs bandwidth which is currently spent elsewhere 👍

[GeoffreyBooth]

The idea behind prohibiting leading / and import 'cjs/file.mjs' is that these features can be tackled later. The fact that the proposal doesn’t support them isn’t meant to imply that they should never be supported.

• In the case of leading /, we thought it was best to punt on this for now until there’s a consensus on potentially providing a shorthand way to refer to the root of the package, as opposed to the root of the file system. One always can refer to the file system root via file:///, so we don’t necessarily need leading / to do the same; and there’s an opportunity here to either use it to refer to the package root or to something else that better aligns with browsers. Or we can decide to just follow CommonJS and let it refer to file system root. Regardless, it’s a discussion that can be had later, by reserving leading / for now.

• In the case of import 'cjs/file.mjs', my concern is that if this is allowed, there’s no certain way for Node to determine the parse goal(s) of a package. In the proposal as written, a package needs a package.json with an "exports" key (or some other ESM-signifying key we may later support like "mode") to be treated as ESM; otherwise it’s CommonJS, or if it has both "exports" and "main" it’s dual-mode. Node can therefore determine the parse goal(s) of the package overall from just the package.json. If we allow deep imports of .mjs files within a package where the package.json lacks any of these fields, then we have the odd result of a package that Node treats as CommonJS supplying ESM files. I feel like there’s a strong likelihood that this can cause issues. Again, however, the proposal doesn’t prohibit this in the future; by erroring now, we preserve the possibility of supporting import 'cjs/file.mjs' down the road.

[MylesBorins]

modules-lkgr had a conflict against master 😅. I've gone ahead and rebased this

[ljharb]

The extension is how you determine the parse goal of a package. It would make no sense to have a CJS module in an .mjs file, so maybe I’m unclear on the issue.

[devsnek]

the only way .mjs should point to anything except esm is with very explicit overrides by the application. the default behaviour (even if the package.json doesn't have "esm properties") should be to resolve .mjs as esm.

[GeoffreyBooth]

Rather than discuss the proposal here, with all concerns intermixed, can we use the issues on the proposal’s repo? https://github.com/GeoffreyBooth/node-import-file-specifier-resolution-proposal/issues (I know, I started it, but I’m trying to rectify my mistake.)

I migrated the “.mjs files in CommonJS packages” issue to GeoffreyBooth/node-import-file-specifier-resolution-proposal#5 @guybedford @ljharb @devsnek

[GeoffreyBooth]

the only way .mjs should point to anything except esm is with very explicit overrides by the application. the default behaviour (even if the package.json doesn’t have “esm properties”) should be to resolve .mjs as esm.

Let’s discuss in GeoffreyBooth/node-import-file-specifier-resolution-proposal#5 please. I’m going to dismiss the review because this proposal always interprets .mjs as ESM.

#19 (comment)

[guybedford]

I’m going to dismiss the review because this proposal always interprets .mjs as ESM.

This PR throws when loading ".mjs" from a package that does not have "ESM properties" in its package.json. That is, a CommonJS boundary package with an ".mjs" file.

[weswigham]

...merge conflict marker? 😄

[guybedford]

Replaced by #28.