WICG / import-maps

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

Implement built-in module resolution and fallback #125

Closed domenic closed 5 years ago

domenic commented 5 years ago

Closes #120.


The test TODO in resolving-builtins.js is a bit troubling and I'd appreciate folks thoughts. We could leave it for now, but if people who haven't been staring at this code for hours have a good intuition, that'd be appreciated. /cc @guybedford @hiroshige-g.

guybedford commented 5 years ago

I must admit my intuition here was always that builtins wouldn't get subpathing support. Do you have any examples of builtins that would need this?

domenic commented 5 years ago

Sure. For example, you may want a built-in element (say virtual scroller) which by default defines the element, when you do import "std:virtual-scroller". But maybe you want to define it under a custom name; then you could do import { HTMLVirtualScrollerElement } from "std:virtual-scroller/element", without the definition side-effects that the top-level module has.

We could just blanket prohibit built-in submodules for now, to keep things simple. (E.g. at parse time, throw away any std:-scheme URLs with slashes in them.) I think we may want them eventually, and I'd be a bit sad to not merge all the code and tests I wrote for that, but those aren't solid reasons :)

guybedford commented 5 years ago

Perhaps restricting initially, then expanding when a well-defined resolution approach can be properly implemented makes sense? Concatenation would be inconsistent when it comes to backtracking here which doesn't seem ideal that eg:

{
  "imports": {
    "virutal-scroller/": "std:virtual-scroller/"
  }
}

would allow import('virtual-scroller/../another-builtin') to resolve to std:another-builtin (if that is correct?).

That definitely would be bad, so I think in this case string concatenation could be a problem unless explicitly restricting these cases.

domenic commented 5 years ago

Fortunately concatenation works as we'd hope in that case: https://jsdom.github.io/whatwg-url/#url=c3RkOnZpcnR1YWwtc2Nyb2xsZXIvLi4vYW5vdGhlci1idWlsdGlu&base=YWJvdXQ6Ymxhbms= , I guess because std: is not a special scheme.

guybedford commented 5 years ago

Hmm so encodings aren't applied either? In that case, since the target domain is well-defined in the stdlib space, perhaps the bad cases are all well-defined failures then?

domenic commented 5 years ago

Yeah, I think everything can be made to work fairly intuitively. The only case I'm unsure on right now is the one I pointed out in the TODO where I wasn't sure whether to apply fallbacks, if you map "foo/": ["std:blank/", "/fallback/"] but then resolve "foo/bar". Should we return "std:blank/bar" (doesn't exist) or "/fallback/bar/"?

There may be other weird cases, and perhaps it'd be best to push solving them until later. That's the only one that stuck out to me immediately though.

guybedford commented 5 years ago

That seems absolutely fine to me. You'd definitely want /fallback/bar right, because the whole point of fallbacks is to handle stdlibs that don't exist now but could in future? While std:blank/bar might never exist, it seems best to have the fallback behave consistently for the whole scheme. Who's to say it won't be specified someday?

hiroshige-g commented 5 years ago

import { HTMLVirtualScrollerElement } from "std:virtual-scroller/element"

Does this mean we spec std:virtual-scroller/element in addition to std:virtual-scroller? I also assumed that we don't allow any access or inspection into things behind std:virtual-scroller.

domenic commented 5 years ago

Good points @hiroshige-g. Right, we would (hypothetically) spec std:virtual-scroller/element. We would not allow introspection into any of the (un-specced) implementation files. I can see how this is potentially more complexity.

Maybe we should just remove submodules of built-in modules for now and wait until we have a concrete spec that actually wants them. Both on the spec and implementation side, they will add complexity, and we should probably avoid paying that complexity until it's needed.

hiroshige-g commented 5 years ago

+1 > remove submodules of built-in modules for now.

justinfagnani commented 5 years ago

Sure. For example, you may want a built-in element (say virtual scroller) which by default defines the element, when you do import "std:virtual-scroller". But maybe you want to define it under a custom name; then you could do import { HTMLVirtualScrollerElement } from "std:virtual-scroller/element", without the definition side-effects that the top-level module has.

@domenic to add support for removing submodules, if the motivation was a mainly the side-effect free import that didn't register the custom element, I think that would actually be a bad idea. Because of the 1-1 tagname-to-class restriction you don't want to allow someone to register a class that you might later.

In this case if module A imports std:virtual-scroller/element and defines it as my-scroller, and loads before module B which imports std:virtual-scroller, there would be an exception. Module A can still import { HTMLVirtualScrollerElement } from "std:virtual-scroller" and define a trivial subclass of it under a different name.

Even if/when we get scoped custom element registries, you still probably want to have the defining module reserve the name in the top-level scope.

hiroshige-g commented 5 years ago

Just to provide more inputs, I'm wondering how import maps should behave on browsers with virtual-scroller versions that do not implement std:virtual-scroller/element yet.

If std:virtual-scroller/element triggers fallback based on whether std:virtual-scroller is implemented, then importing std:virtual-scroller/element via import maps fallbacks can still fail. This might require fallback mechanism outside import maps by e.g. dynamic imports.

If std:virtual-scroller/element triggers fallback based on whether std:virtual-scroller/element is implemented, then importing std:virtual-scroller/element (+ fallbacks) won't fail, but polyfilling might be a little more challenging. For example, when user code imports both std:virtual-scroller and a polyfill of std:virtual-scroller/element, how can we polyfill? Just implementing everything in JavaScript in the polyfill is a simple solution, but this causes duplicated modules.

littledan commented 5 years ago

I'm all for built-in element submodules (seems useful ergonomically), but I am interested to learn more about use cases for side effects on module load. We're sort of going back and forth in WebIDL-land about whether built-in modules should have a space to list side effects when they are loading. Where's the right place to discuss this?

domenic commented 5 years ago

Hmm, well, the Web IDL PR makes sense to me. Personally I think side effects are a must, for the element-registration case alone.

domenic commented 5 years ago

This is ready for another look, if folks have time!