Specify aquiring import maps

[hiroshige-g]

This PR the semantics around "aquiring import maps", including
how import maps are loaded via <script> elements, and
how import map loading and module script loading interact and block each other.

---

Preview | Diff

[domenic]

Great work; thank you!!

A minor overall nit: can you match the rest of the spec in:

• Using <div algorithm> to wrap algorithms
• Using two-space indent
• Not doing any manual line wrapping (instead using your text editor's line wrapping)?

[domenic]

I know we have both had a long-running TODO to tighten up the current spec and implementation on this.

If possible, it would be good to implement our latest preferred approach for import maps from the beginning. I don't know whether that's possible without refactoring a lot of the existing spec, though. I remember when we tried previously we discovered we needed more refactoring, and that may apply here too.

[hiroshige-g]

I added a check for moving between documents, and add a link to the spec PR whatwg/html#2673 for other script cases.
I feel this PR is not strongly blocked by #2673, as we can amend the the condition here later according to the future discussion on #2673.

[hiroshige-g]

Thank you for review! I reflected some of comments, and will address the rest later.

Using two-space indent
Not doing any manual line wrapping (instead using your text editor's line wrapping)?

Is there any formatting tool (e.g. something clang-format-like) for bikeshed?

[domenic]

Looking pretty good. If you can fix the wrapping, I can probably tidy up any minor issues and land.

[hiroshige-g]

Updated the PR.

I tried to move pending import maps count and acquiring import maps flag to Document, but I couldn't make the environment settings object's members read-only, because wait for import maps have to modify the acquiring import maps flag via settings object. I left these flags as-is. Any opinions?

Also fixed wait for import maps (6e71eb2).

Remaining TODO is to fix links to HTML specs (e.g. fetch a module script graph -> fetch an external module script graph) after the HTML-spec PR lands.

[domenic]

I tried to move pending import maps count and acquiring import maps flag to Document, but I couldn't make the environment settings object's members read-only, because wait for import maps have to modify the acquiring import maps flag via settings object. I left these flags as-is. Any opinions?

I will try moving this all to Document, since this acquiring stuff is Document-specific, i.e. we will probably use a different mechanism in workers.

[domenic]

Moving it to Document is too hard because the module script fetching algorithms are all agnostic to document vs. worker. I guess it is OK as-is for now, although it is a bit strange to put state directly on environment settings objects. We can try to figure out a better solution later perhaps.

I pushed minor editorial fixes, but it mostly looks good to me now!

The only remaining issue I have is that I do not understand the second warning, that says

Depending on the exact location of wait for import maps, there would be observable behavior differences between HTML-spec- and Fetch-spec-based import maps. For example, the current HTML-spec-based draft here calls wait for import maps at very early stages of module loading before resolve a module specifier. Therefore, acquiring import maps is always set back to false, even if resolve a module specifier fails e.g. in parsing http://:invalid. When we switch to Fetch-spec-based, import map resolution would be done inside Fetch spec after resolve a module specifier and thus we might also want to wait for import maps after resolve a module specifier. In that case, acquiring import maps wouldn’t be reset to false for http://:invalid, because we early-exit module loading.

Can you expand (just on GitHub is fine) with an example? I'm not sure, for example, where http://:invalid appears. It's also unclear to me why we could reset to false with one design, but not the other---what prevents us from resetting to false even in a Fetch-spec-based design?

I'd like to get that cleared up before merging since I don't want to have text in the spec I don't understand :).

[hiroshige-g]

Can you expand (just on GitHub is fine) with an example? I'm not sure, for example, where http://:invalid appears. It's also unclear to me why we could reset to false with one design, but not the other---what prevents us from resetting to false even in a Fetch-spec-based design?

I digged more into this and found that:

• <script src="http://:invalid" type="module">
  • NOT reset to false (because of early return at Step 24.5 of https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script).
• new Worker("http://:invalid", {type: "module"})
  • NOT reset to false (because of early return at Step 4 of https://html.spec.whatwg.org/multipage/workers.html#dom-worker).
• import('http://:invalid');
  • RESET to false in the current PR, by the wait for import maps call at the head of fetch an import() module script graph.
    • Avoiding resetting the flag requires additional complexity inside resolve-a-module-specifier.
  • NOT reset to false if we move wait for import maps to Fetch spec.
    • I basically assume that, as for the resolution part (not the registering part) "Fetch-spec-based" import maps modifies Fetch spec only (e.g. wait for import maps calls are inserted to Fetch spec), to support many non-module script cases with a single modification to Fetch spec, without adding many modifications for each resource type to HTML spec.
    • If we keep some logic at the head of fetch an import() module script graph to reset the flag, we'll still have "RESET to false" behavior.

So import('http://:invalid');'s flag resetting behavior

• Might be changed when switching to Fetch spec,
• But it looks a little inconsistent with <script src="http://:invalid" type="module"> even within the current PR.

Probably it's good to land this PR with the second warning removed, and file a github issue about this.

[domenic]

Thanks for explaining! I will reword the note a bit. I don't think it's a big deal to have this inconsistency because import() is so different (operating on specifiers instead of URLs).