Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting bare addresses #94

Open
guybedford opened this issue Jan 11, 2019 · 11 comments
Open

Supporting bare addresses #94

guybedford opened this issue Jan 11, 2019 · 11 comments

Comments

@guybedford
Copy link
Collaborator

I'm glad to see that recursive resolution is no longer being provided.

Given this, should we consider supporting bare addresses? This would allow:

{
  "imports": {
    "x": "./x.js"
  }
}

to be equivalent to:

{
  "imports": {
    "x": "x.js"
  }
}

I'd be glad to put a PR together as well if this would be useful.

@domenic
Copy link
Collaborator

domenic commented Jan 11, 2019

Hmm. I think you're right this is possible.

I lean toward the conservativism of requiring the ./ prefix, and vaguely it feels like there might be a rule of "when using URLs in module-ey contexts, use the ./ prefix". But my reasoning here isn't very strong.

I'd be OK with changing this, but I wouldn't rush into it. Does anyone else have an opinion?

@zenparsing
Copy link

Do you think some users would expect recursive application to work if they see a bare import as the map target?

@domenic
Copy link
Collaborator

domenic commented Jan 11, 2019

That's a good question. My initial instinct is it would depend on what else was nearby in the import map. If it were a bunch of URL-ey things they might see a "bare" relative URL as what it is (i.e. a URL). Similarly, the OP has a .js extension, which helps indicate that it'd be a URL-ey thing. But if it were a small import map without obvious extensions, then it might be more confusing...

@eliasvasylenko
Copy link

I think these arguments #80 apply here too; soft failures should be preferred over permissive reinterpretation, just as they should be preferred over hard failures.

The probability that people will want to prescribe new meaning to bare addresses in the future may seem low, but it's surely non-zero, and I don't think supporting them in this way actually buys anything in the meantime.

@rictic
Copy link
Contributor

rictic commented Feb 25, 2019

Given the goal to allow a specifier to resolve to a built-in module, it would be better to require ./. Taking the example from the readme:

{
  "imports": {
    "@std/kv-storage": [
      "@std/kv-storage",
      "/node_modules/kvs-polyfill/index.mjs"
    ]
  }
}

If a mapping to x.js should resolve to ./x.js, then the mapping to @std/kv-storage could be read as resolving to ./@std/kv-storage.

@domenic domenic added this to the Pre-spec milestone Mar 11, 2019
@domenic
Copy link
Collaborator

domenic commented Mar 11, 2019

@rictic your argument makes sense, but to me it argues for a simpler solution: use URL-like built-in module specifiers, like std:kv-storage, instead of bare-specifier-like ones, like @std/kv-storage. Then the addresses (i.e. the right hand sides) are always URLs, and we can have confidence that we would interpret x.js or @std/kv-storage the same as ./x.js or ./@std/kv-storage.

@mikesamuel
Copy link

Banning adjacent // in paths would allow you to use //@std/kv-storage to move the std out of the path component without introducing something that looks like a scheme or a Windows volume.

I know that module specifiers aren't URLs and import: URLs aren't hierarchical, but the less work code that produces and mixes&matches import maps can do with existing URL libraries, the easier it will be to adopt them.

@domenic domenic removed this from the Pre-spec milestone Apr 2, 2019
@domenic domenic added this to the MVP milestone Jul 3, 2019
@domenic
Copy link
Collaborator

domenic commented Jul 3, 2019

Now that we have a pretty solid foundation, I'd be happy to take a PR for the spec/tests adding support for this.

@guybedford
Copy link
Collaborator Author

I was just looking at the spec at https://wicg.github.io/import-maps/#sort-and-normalize-a-specifier-map, and according to the outline there this is already supported I believe since there are no explicit checks for ./ anymore.

I think we can close this then?

@domenic
Copy link
Collaborator

domenic commented Aug 2, 2019

I think we'd need tests at the very least. Also, this may change if #137 happens; that proposes that the right-hand side no longer be URLs, but instead module specifiers.

@domenic
Copy link
Collaborator

domenic commented Sep 23, 2019

I'll consider this blocked on multiple import maps and the work on #137. Since multiple import maps are no longer part of the MVP, moving the milestone.

@domenic domenic modified the milestones: MVP, Full featured, Multiple import maps Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants