Trailing slashes
domenic opened this issue · 10 comments
@RReverser pointed out to me today that in Node.js, you can do either require("lodash") or require("lodash/"), and both will map to the lodash package's main module.
Similarly, in Node.js you can do either require("./directory") or require("./directory/"), and this will look for the "main" field in ./directory/package.json and require that module.
With import maps, these trailing slash variants are not possible to remap in the same way. We currently require trailing slashes to match, so you can do "lodash/": "/node_modules/lodash/" and "./directory/": "./some/other/directory/". But then, importing those will give URLs like https://example.com/node_modules/lodash/, which is not what you want; you wanted the URL to be https://example.com/node_modules/lodash/main.mjs.
Another aspect to consider is if we introduce import.meta.resolve(). What should the result of import.meta.resolve("lodash/fp/..") be? As currently proposed, it would be whatever URL "lodash/" maps to. Which, per the above, is not so useful.
So the first question is, do we care about this case? I'm inclined to take a wait-and-see approach. In particular, my initial instinct is that even though Node has two ways to require the same module, "lodash" and "lodash/", maybe it's OK that on the web only the first way works. I'd be open to changing this opinion as we see more people running into issues in the wild.
Then, if we did want to address this, how would we? Simply lifting the "slashes must match" restriction is not great, because what if you want both "lodash/" -> </node_modules/lodash/main.mjs> and "lodash/foo.mjs" -> </node_modules/lodash/foo.mjs>? You wouldn't be able to get both.
One route for addressing this, which is attractive to me, might be to try to canonicalize all trailing slashes in import specifiers away. That is, "lodash/" literally means the same thing as "lodash". This is trickier for cases like "/directory/"; it'd be a breaking change (but maybe a web-compatible one?) to always treat that the same as "/directory", i.e. change it to by default request the URL </directory> instead of the URL </directory/>. Maybe we could only impose the normalization when an import map gets involved? But that might be a bit messy.
Once difference to note is that when that package has an "exports" field, the / form no longer works (unless it has "./": "./", which is deprecated).
However, canonicalizing trailing slashes still seems like the proper thing to do.
@RReverser pointed out to me today that in Node.js, you can do either require("lodash") or require("lodash/"), and both will map to the lodash package's main module.
This is no longer the case for the ES modules implementation of Node.js, and a special ERR_UNSUPPORTED_DIR_IMPORT error is thrown instead.
This has been part of the effort of deprecating some of the magic around module resolution by not including support for directory resolution in the ES modules resolver implementation to try create alignment with browser resolution.
There is a lot to this conversation though, happy to flesh out any aspects further as necessary.
@guybedford I think it's slightly different in that ESM in Node.js doesn't support directory imports altogether anymore, regardless of whether it was specified with a trailing slash?
@RReverser yes you are correct this is based on strictly checking if the file is a directory not the trailing slash. So in Node.js a trailing slash that is not a directory would throw MODULE_NOT_FOUND instead.
But you can still resolve a directory import for the main for CJS, eg via import('pkg') where pkg has a "main": "./dir/" field will work fine. This was necessary for backwards compatibility in loading CJS packages.
There is a separate deprecation PR currently in progress to deprecate this behaviour for ES modules though as well, in ensuring "main" always points to a complete path in the page.
Regardless, Node.js fully deprecates the resolution of trailing slashes, which is the important point to note here. They simply can't resolve in Node.js anymore.
This isn’t accurate - node has not in any way deprecated trailing slashes in CJS, for packages that lack an “exports” field. That may not be relevant to import maps, though, which are for ESM.
@ljharb import and import() do not support resolving directories with trailing slashes at all. There is no specifier with a trailing slash that will resolve through these mechanisms without customizing the resolve hook.
@guybedford agreed, but "fully deprecated" implies "with require" as well, so I wanted to clarify.
agreed, but "fully deprecated" implies "with
require" as well, so I wanted to clarify.
Also probably worth mentioning node --es-module-specifier-resolution=node which makes them work as well.
But yeah, to be honest, I didn't know Node chose not to support directory -> main file imports by default in ESM syntax, not even via exports field. This seems unfortunate as it creates inconsistency between what you can do with npm packages vs local dirs.
I think I'm OK not supporting this, but if we have people using import maps in the wild where this is an issue, please let me know (preferably with a link to the site).
@domenic FWIW the deprecation path is moving now in Node.js to fully disable trailing slash imports for ESM.