This repository was archived by the owner on Sep 2, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 2, 2023. It is now read-only.
Locking down fragment/query on bare package names #83
Description
Right now hash/search fragments are preserved when importing new URLs:
import 'foo?a';
import 'foo?b';will load 2 different modules and use these fragments as being passed to the "main" entrypoint of foo. This goes against how package name maps are seeking to work WICG/import-maps#38 . We should do a couple things to get in line with the package name map behavior if that is what we are seeking to use for web compatibility.
- treat all characters prior to
/as part of the package specifier for these bare import specifiers. meaning that package names can contain?or#. - probably enhance errors for when the part of such specifiers preceding the
?or#get a message about if the prefix would have been found.
Activity
jdalton commentedon May 16, 2018
\cc @iarna for npm since it's package name related.
guybedford commentedon May 16, 2018
In the current implementation the above falls into the ResolveModule path at https://github.com/nodejs/node/blob/master/src/module_wrap.cc#L594. Which then concatenates the specifier into the node_modules lookup. Once found, the path is taken to be the part after the package name length.
So the above example would check the directory
node_modules/foo?adirectly on the filesystem.So we're actually already pretty locked down and in agreement with the package map approach here.
jdalton commentedon May 16, 2018
Wouldn't this also impact how folks were planning to handle reloading modules
(e.g. the cache buster hash trick from the browser)?
bmeck commentedon May 16, 2018
@jdalton to an extent?
@guybedford that is a good catch and would have been considered a bug if I saw it before this issue. Since it was not my intention when writing that to have that specific behavior it seems we should still talk about it though.
giltayar commentedon May 17, 2018
I have to admit that this seems counter-intuitive to me. Maybe logical, but counter-intuitive:
If I do
Then I import the file
./foo.mjs, and thex=4is part of the cache key, so is used as a "cache buster".But if I do
Then we're importing the package in the folder/file
node_modules/foo?x=4? I would expect to loadnode_modules/foowith a cache-busting x=4.While I understand the logic—bare imports are not URLs, and should not be treated as such—I believe most people wouldn't understand this and would be surprised by this behavior, especially if
is treated (somewhat) as a URL and the
x=4suddenly becomes a cache-buster again.In the interest of the principle of least-surprise, I hope it doesn't land this way.
bmeck commentedon May 17, 2018
@giltayar you can post the issue I created in package name maps, but I did not see a way to argue about it if prohibiting the characters is seen as a bad idea. Any way it goes, we should match package name maps.
bmeck commentedon Dec 30, 2018
Conclusion should stand and we aren't discussing this anymore.