Values with trailing slashes not in path part

[hiroshige-g]

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#/"
  }
}

• https://foo.com/bar/baz is mapped to https://nyaxtstep.com/fox?grapes=/bar/baz
• https://foo.com/bar?baz is mapped to https://nyaxtstep.com/fox?grapes=/bar?baz
• https://bar.com/baz is mapped to https://nyaxtstep.com/fox#/baz

(The behavior before d5add26 is different though)

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

[domenic]

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]

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]

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]

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

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: #134 (comment).

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

[domenic]

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]

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

• https://foo.com/bar/baz is mapped to https://nyaxtstep.com/bar/baz
• https://foo.com/bar?baz is mapped to https://nyaxtstep.com/bar?baz
• https://bar.com/baz is mapped to https://nyaxtstep.com/baz
• moment/foo is mapped to /foo