add tests for import map composition and a "cascading" composition implementation

[michaelficarra]

In the hopes that an implementation will help make the case for #137, I present an implementation of cascading import map composition and tests that demonstrate its behaviour.

---

Preview | Diff

[domenic]

I've discussed this with some folks offline and we'd be willing to go this route if it doesn't add too much complexity.

However, to get a sense of that complexity, we'd need a more complete update to the reference implementation and tests. In particular, this only creates a merging procedure, but we also need to see and test how the resolution works, and how much complexity is added there. And we need to update the types and terminology used; right now the reference implementation represents addresses using URL objects, but if we're changing the right-hand side from a URL to a module specifier, then we need to stop using URL objects there.

In particular, in offline discussions you've explained how the mental model for web developers is something like

// Turns relative URL-like specifiers into absolute-URL-like ones
let specifier = normalize(specifierFromSourceText);

specifier = apply(importMapB, specifier);
specifier = apply(importMapA, specifier);

// Throws on specifiers that are not absolute-URL-like. Identity transform otherwise.
let url = toURL(specifier);

We need to see this manifested in the reference implementation, where it will probably be something more like

// Inside resolve() in resolver.js

// ... existing code to get normalizedSpecifier ...
const mappedSpecifier = apply(mergedImportMap, normalizedSpecifier, scriptURL);

const url = tryURLParse(mappedSpecifier);

if (!url) {
  throw new TypeError(`Unmapped bare specifier "${specifier}"`);
}

// ... existing code to reject un-implemented built-in modules ...

return url;

[michaelficarra]

@domenic Take a look at the updated changes. I've updated/added tests and the resolving implementation.

edit: Oh also I've enabled coverage and enforced 100% coverage for tests, hope you don't mind.

[domenic]

Thanks so much for the work on this, and sorry for the delay. This is a big change, but you've clearly gone out of your way to create a battery of tests, as well as do minor things like match the coding style. That makes it much easier.

I'm also heartened by the relative decrease in code size in resolver.js. Some of that may be incidental (e.g. facotring out some of the logic into tryURLLikeSpecifierParse, or removing our safeguards against some not-implemented cases, or using a functional style instead of a (more spec-translation-friendly) imperative. But the getFallbacks algorithm seems like an actual win.

The main changes I'd like to see is adding a few more of the tests in locations I mentioned inline. Also, I'd like @hiroshige-g to take a look at the new/modified tests, and see if he has any of his own suggestions for devious tests to add. For example, it seems possible to me that there aren't enough tests exploring the consequences of mapping bare specifiers twice, and how that might interact with scopes; I see good coverage for absolute URLs but not as much for bare specifiers. Also, I'd like to make sure we have coverage for nailing down an answer to #153.

Overall, it seems likely this can work. I continue to maintain that the result is a more complex mental model than the current specifiers -> URLs setup; I know you disagree. But I'm willing to accept that this increase in complexity in exchange for the compositional benefits you've worked to explain.

Finally, @hiroshige-g should also take a look at the new code logic, and judge if there are any problematic aspects. In particular I want to be sure that the new resolve() function is not too expensive in some way, since that is semi-hot. It seems to have the same algorithmic complexity over various inputs, but I haven't analyzed it rigorously, and it may benefit from some optimizations (e.g. there may be places you could reduce the number of O(n) iterations over a list).

[guybedford]

I haven't followed the exact details but the overall approach seems like a sensible model, and while adding complexity it doesn't seem to cross any lines to me.

I just thought I would mention that my main concern here would be that with this proposal, once a URL has been overridden, the original can never be referenced again.

So later import maps effectively have less control than earlier ones. Maybe that's a bug or a feature though! (interestingly it can actually be seen as a security feature to restrict builtins...).

[bakkot]

@guybedford That's intentional. Otherwise the README's example of denying access to a built-in module would be impossible; see #137 for the motivations behind this PR.

[michaelficarra]

@domenic All comments addressed. I've left the commits separate to make it easier to review only the changes.

[michaelficarra]

Ping @domenic. No rush, but I wanted to make sure you didn't miss that this is ready for review. Also, @hiroshige-g I believe we are waiting on a review from you as well.

[domenic]

Thanks, this looks good. I have run out of time today, but I will work on porting the reference implementation updates to the spec text next week.

[domenic]

I was trying to explain to @hiroshige-g the compositional mental model you explained to me offline. In particular I think your model was something like: authors expect the import maps to apply in reverse (?) order, eventually bottoming out at a "basic specifier", which gets an identity transform from specifier string -> URL space. But when trying to reproduce this on the "virtualization patterns we expect to see in the wild" example, it didn't quite work that way.

In particular, consider a module that does import "std:blank" from inside https://built-in-enhancement-2/.

const specifierFromSourceText = "std:blank";

const specifier = normalizeToURLString(specifierFromSourceText);
// -> "std:blank"

specifier = apply(map3, specifier);
// -> https://built-in-enhancement-3/

specifier = apply(map2, specifier);
// -> no change...

specifier = apply(map1, specifier);
// -> no change...

const url = parseToURL(specifier);
// -> https://built-in-enhancement-3/

whereas the test states the result should be https://built-in-enhancement-1/.

If the intuition is non-reversed, that fixes the result when the referring script's URL is inside https://built-in-enhancement-2/, but then we have a problem when working from inside https://built-in-enhancement-1/:

const specifierFromSourceText = "std:blank";

const specifier = normalizeToURLString(specifierFromSourceText);
// -> "std:blank"

specifier = apply(map1, specifier);
// -> std:blank

specifier = apply(map2, specifier);
// -> https://built-in-enhancement-2/

specifier = apply(map3, specifier);
// -> no change...

const url = parseToURL(specifier);
// -> https://built-in-enhancement-2/

whereas the test says it should be std:blank.

I know we've implemented a different version of this (which I prefer), which involves creating a single composed import map. We should keep doing that. But I want to try to recapture the intuition you were explaining to me about how authors might think about it.

[bakkot]

The simple mental model we explained makes sense only when all of the mappings are similarly specific. So, if all mappings are in the same scope or all at the top level, that is how composition works. It gets more complicated when adding an import map with less-specific scopes on top of an import map with more-specific scopes: in that case you would not want to apply a mapping from a less-specific scope when a more-specific scope gives a mapping for the module specifier of interest, since more specific scopes are considered to be higher precedence.

(At the time of our conversations with you, we hadn't yet considered the interactions of scope specificity with composition.)

[hiroshige-g]

The examples like #137 (comment) are cascaded as expected, but they are quite simple and we know the both of the cascaded import maps.
When a developer writes an import map A, the developer doesn't know how the import map A will be cascaded with other import maps, and doesn't know what the other import maps will be.

I'm not sure whether the complicated semantics above allows independently-developed import maps to be cascaded nicely.
So I'd like to have something -- simpler semantics of cascading, or something else -- that helps the developers of import maps to be confident that their import maps are cascadable (without prior knowledge of import maps to be cascaded with their import maps).

Any ideas or suggestions?
I would be very happy if we can figure out better composition algorithms, better explanation of the semantics, etc.

(This issue already exists in the current https://wicg.github.io/import-maps/#update-an-import-map algorithm. I felt this wasn't a big problem because

• We assumed the main Document author (or deploying tool etc.) has the control over all import maps and thus it's the responsibility of the author to coordinate those import maps nicely.
• The expected use cases for merging multiple import maps are somehow limited.

However, IIUC this proposal is changing import maps to a more composable thing, so I feel composability without coordination is becoming more important.)

[bakkot]

So I'd like to have something -- simpler semantics of cascading, or something else -- that helps the developers of import maps to be confident that their import maps are cascadable (without prior knowledge of import maps to be cascaded with their import maps).

Sure. I think it actually is pretty simple: "when adding an import map the values on its right-hand side are evaluated in the context of any existing import map" works pretty much the same way as "when adding a module the specifiers in its import statements are evaluated in the context of any existing import map", so if your import map doesn't break modules, it also shouldn't break other import maps.

In the common case, where the left-hand side contains only non-URLs and the right-hand side contains only URLs, no cascading is possible and so no coordination is necessary. In the case where the left-hand side contains a URL - for example, because the import map is establishing a fallback or a wrapper for a built-in module - the author of the import map is responsible for ensuring that the right-hand side will resolve to something which provides the same interface as the resource at the original URL, so that modules which are importing the original URL and receiving the resource at the new URL are not broken. This is necessary regardless of cascading. As long as this is done, cascading will just work: the fact that a later map which has the original URL on its right-hand side will actually end up mapping to the new URL should cause no more problems than the fact that a later module which imports the original URL will get the resource at the new URL.

With scopes, the assumption is that the author of the import map has coordinated with the resources in that scope already (as in the polyfilling built-in modules example: each import map needs to know where the wrapper module is, but does not need to know about other polyfills). Later maps won't interfere with it unless they define a mapping for the same (or a more specific) scope. So again, no coordination between import maps should be necessary.

In particular, in the polyfill example you linked, the authors of each of the two import maps need not know about the other. They don't even know whether there is any other import map. Those are exactly the same maps that you would write to perform polyfilling in the absence of any other polyfills or import maps.

[domenic]

FYI I pushed a few commits as I work to translate the reference implementation into spec language, renaming and tweaking things as I go. The resulting spec is currently incoherent but the reference implementation still passes all the tests even after my tweaks. I'll continue tomorrow. Do not feel obliged to review my changes; it'd probably be better to batch them.

[domenic]

OK, the spec updates are almost done. (And the corresponding reference implementation tweaks.) What's remaining:

• Wait for Add cloning for maps whatwg/infra#265 to get merged and reference that (currently there's a placeholder
• Add an example or two or three of concatenating import maps, and explain the mental model. This replaces the previous, now-inaccurate example. I would appreciate if @michaelficarra or @bakkot were willing to take that on (pushing to this branch).

[hiroshige-g]

Thanks for clarification. So provided the scopes of different import maps don't interfere, then we can interpret the cascading as "apply import maps in-order, while scopes are exempted from further overriding by later import maps".

For example in #137 (comment), the "/kvs-v2-polyfill.mjs" scope in the first import map is exempted from overriding by the second import map.

[hiroshige-g]

I'm still highly concerned about the complexity (to implementation and to users) added by this.
Especially the change adds mixture of URL and non-URLs and parsing/serializing/reparsing URLs, which makes it harder to understand the import maps behavior.

I'd like to defer the final decision at least after we pass Chromium-internal reviews (preparing...) on the basic parts of import maps that are already reflected in the current draft spec.

[bakkot]

@domenic

Add an example or two or three of concatenating import maps, and explain the mental model. This replaces the previous, now-inaccurate example. I would appreciate if @michaelficarra or @bakkot were willing to take that on (pushing to this branch).

Will do. I'll try to get to this early next week.

@hiroshige-g

So provided the scopes of different import maps don't interfere, then we can interpret the cascading as "apply import maps in-order, while scopes are exempted from further overriding by later import maps".

In reverse order, but yes.

An (equivalent) way of describing it is "find the most specific applicable scope which provides a mapping for the module specifier being evaluated, across all maps, and apply maps in reverse order starting from the map with that scope".

Or, if you are thinking about about composition directly, you can (again equivalently) say "when merging in a new map, the keys on its RHS are evaluated in the context of the existing map, just as any other module specifier would be. Scoped mappings are evaluated as if in that scope."

[hiroshige-g]

Another example of breaking the "apply import maps in-order" semantics:
In the current PR, IIUC

<script type="importmap">
{
    "imports": {
      "bar/": "https://bar.example.com/",
      "bar/baz/": "https://baz.example.com/"
     }
}
</script>
<script type="importmap">
{
    "imports": { "foo/": "bar/" }
}
</script>

is combined into

<script type="importmap">
{
    "imports": { "foo/": "https://bar.example.com/" }
}
</script>

and thus foo/baz/a.js is mapped into https://bar.example.com/baz/a.js,
not foo/baz/a.js -> bar/baz/a.js -> https://baz.example.com/a.js.

This is inevitable though...

[hiroshige-g]

Er, can this PR already result in exponential-sized output?
For example:

<script type="importmap">
{
    "imports": {
      "a1": "https://example.com/a1",
      "a2": "https://example.com/a2"
     }
}
</script>
<script type="importmap">
{
    "imports": {
      "b1": ["a1", "a2"],
      "b2": ["a1", "a2"]
     }
}
</script>
<script type="importmap">
{
    "imports": {
      "c1": ["b1", "b2"],
      "c2": ["b1", "b2"]
     }
}
...
</script>

result in O(2^n) addresses for n import maps?

[bakkot]

Er, can this PR already result in exponential-sized output?

Hm. As implemented, yes, because there's nothing forbidding having the same specifier appear multiple times in a fallback list (even within a single map). It doesn't really make sense to allow that, though. If fallback lists are deduplicated during composition I believe that problem goes away.

[hiroshige-g]

I agree that there are no valid use cases for exponentially-exploding cases. But even with deduplication, trailing-slash matching can result in exponential-sized output.

<script type="importmap">
{
    "imports": {
      "a1/": "https://example.com/1/",
      "a2/": "https://example.com/2/"
     }
}
</script>
<script type="importmap">
{
    "imports": {
      "b1/": ["a1/1/", "a2/1/"],
      "b2/": ["a1/2/", "a2/2/"]
     }
}
</script>
<script type="importmap">
{
    "imports": {
      "c1/": ["b1/1/", "b2/1/"],
      "c2/": ["b1/2/", "b2/2/"]
     }
}
...
</script>

[bakkot]

Ah, yes, I keep forgetting about package-prefix mappings.

In this case that seems to be what the user is asking for, though, so I'm not sure to what extent it should be regarded as a problem. In an actual implementation it could be represented in memory without the exponential overhead, if that's the worry.

On the other hand, would browsers in practice cap how many times they attempted to fetch a given module? If so, it seems reasonable to cap the number of fallbacks to that number.

[guybedford]

Might this be an argument for treating cascade resolution as actually performing the fallback selection?

[lemanschik]

just my 5 cent doesen't that overall add a big extra problem for that we need to create additional tooling i see we shifting the problems around we change the place where they happen this should when get done be done only inside a serviceWorker that that it gives a JS static analyzeable way to audit track what does resolve to what.

<base> // already shows great where that concepts do fail and it is even able to implement them partial

i feel overall that i code many more resolvers for resolvers then for real existing targets. see tsconfig amd systemjs all this stuff nodejs every one trys to map over the other then this hazard started. We Should keep on track with real resolve able items that exist in a fetch able way with as less url translation outside service workers as needed.

so that serviceWorkers get the only url rewriting point in the application.