Modules: Locking down fragment/query on bare package names

[bmeck]

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 https://github.com/domenic/package-name-maps/issues/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.

1. treat all characters prior to / as part of the package specifier for these bare import specifiers. meaning that package names can contain ? or #.
2. probably enhance errors for when the part of such specifiers preceding the ? or # get a message about if the prefix would have been found.

[jdalton]

cc @iarna for npm since it's package name related.

[guybedford]

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?a directly on the filesystem.

So we're actually already pretty locked down and in agreement with the package map approach here.

[jdalton]

Wouldn't this also impact how folks were planning to handle reloading modules
_(e.g. the cache buster hash trick from the browser)_?

[bmeck]

@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]

I have to admit that this seems counter-intuitive to me. Maybe logical, but counter-intuitive:

If I do

import './foo?x=4'

Then I import the file ./foo.mjs, and the x=4 is part of the cache key, so is used as a "cache buster".

But if I do

import 'foo?x=4'

Then we're importing the package in the folder/file node_modules/foo?x=4? I would expect to load node_modules/foo with 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

import 'foo/bar?x=4'

_is_ treated (somewhat) as a URL and the x=4 suddenly becomes a cache-buster again.

In the interest of the principle of least-surprise, I hope it doesn't land this way.

[bmeck]

@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]

Conclusion should stand and we aren't discussing this anymore.