search

WICG / import-maps

How to control the behavior of JavaScript imports
https://html.spec.whatwg.org/multipage/webappapis.html#import-maps
Other
2.68k stars 69 forks source link

Values with trailing slashes not in path part #173

Closed hiroshige-g closed 4 years ago

hiroshige-g commented 5 years ago

Originally from @nyaxt:

For keys ending with slashes, we require values to end with slashes, but any slashes (not necessarily in the path part) are OK. So for example

{
  "imports": {
    "https://foo.com/": "https://nyaxtstep.com/fox?grapes=/",
    "https://bar.com/": "https://nyaxtstep.com/fox#/"
  }
}

(The behavior before https://github.com/WICG/import-maps/commit/d5add267b960de09d2ef69a765e3ce0614cdad97 is different though)

I don't have strong opinions here; WDYT? Does this looks good?

domenic commented 5 years ago

Hmm. This seems OK-ish; the intention of the rule is mostly to catch potential errors. However it seems like it would be pretty easy to add one additional check, that the trailing slash is in the path.

I guess it would become harder to insert such a check after #167, because then the right hand side is no longer necessarily a URL. /cc @bakkot @michaelficarra.

bakkot commented 5 years ago

I think it'd still be pretty easy to add this check after #167; URL-based specifiers on the right-hand side already need to be normalized, and there could be an additional check at that point forbidding query parameters or fragments when the left hand side is a package prefix. This would involve adding an additional "isValidPackagePrefixRHS" field to the result of tryURLLikeSpecifierParse in the URL case, to go along with the existing isBuiltin, or something like it, but I don't think it would introduce much complexity.

My opinion on the issue:

The https://nyaxtstep.com/fox?grapes=/ case, in particular, seems like it ought not be forbidden: for example, you might have a single node_resolver.php resource responsible for serving all JavaScript which lives in node_modules, and want to do

{
  "imports": {
    "moment/": "/node_resolver.php?path=moment/"
  }
}

(Of course one could rewrite their server in such a way that the resource ended up in the actual path of the URL, but why should they have to?)

So adding this check, at least for the query parameter case, seems like it would rule out useful cases, and I'm not convinced it rules out any commensurately large class of error. The # case might be worth forbidding, since the part after the # won't even be visible to the server (I assume); on the other hand, it's not obviously desirable to carve out an exception for it, since it makes the model more complicated.

bakkot commented 5 years ago

Also, re: the # case, it doesn't seem like it's obviously any more of an error to have a # in a package-prefix URL than any other URL on the RHS. That is, it seems like both of the mappings in the following should be an error, or neither should be:

{
  "imports": {
    "a/": "/a#foo/",
    "b": "/b#bar"
  }
}
guybedford commented 5 years ago

@bakkot on the last point, there can be value in the definition of:

{
  "imports": {
    "b": "/b#bar"
  }
}

as the #bar effectively causes reinstancing like a sort of cache busting just at the module registry level (without triggering refetch) of the module, such that import '/b' would be a different execution instance of the same module.

dcleao commented 4 years ago

In a related issue, I propose a syntax that would allow the excess segments "matching" the / on the LHS to be inserted in a place other than the end of the RHS: https://github.com/WICG/import-maps/issues/134#issuecomment-529594023.

The addition of this feature would dedramatize the choice of where the excess segments are placed by default.

domenic commented 4 years ago

It seems like we're OK with the consequences of the current spec, so I'll close this. Happy to reopen if there's something we missed.

hiroshige-g commented 4 years ago

176 reverts the trailing-slash behavior back to pre-#164 (still I'm neutral):

TomasHubelbauer commented 7 months ago

As of current, does the spec allow using import maps to route imported modules through a single endpoint passing their path via a fragment or a search param like shown in this snippet by @bakkot?

{
  "imports": {
    "moment/": "/node_resolver.php?path=moment/"
  }
}

I am hoping to find a way to achieve this:

<script type="importmap">
  {
    "imports": {
      "./index.js": "./index.js",
      "./": "./launder.js?/"
    }
  }
</script>
<script type="module" src="index.js"></script>

index.js:

import test from './test.js';

console.log(import.meta.url, test);

launder.js:

console.log(import.meta.url);

export default import.meta.url;

In this scenario, in my preferred case, I would like to see ./test.js resolve to ./launder.js?/test.js and launder.js should then print …/launder.js?/test.js followed by index.js printing …/index.js …/launder.js?/test.js.