[ECMA-262 layering] Refactor import-related host hooks

[nicolo-ribaudo]

Hi!

I plan to propose a refactor to the import-related host hooks of ECMA-262, and this PR shows how it would affect HTML.

Currently, ECMA-262 has two host hooks:

• HostResolveImportedModule(referrer, specifier), which synchronously returns the Module Record corresponding to the (referrer, specifier) pair;
• HostImportModuleDynamically(referrer, specifier), used for dynamic import(), which asynchronously loads the Module Record corresponding to (referrer, specifier) and all its dependencies, calls .Link()/.Evaluate() on it and then "returns" to ECMA-262 by calling FinishDynamicImport.

We are working on three ECMA-262 proposals related to modules, and each one of them has needs that are not satisfied by the existing layering:

• Module Blocks allows declaring a module inline. It needs a way to load the dependencies of an already available module record.
• Import Reflection, at least when it comes to JS modules, allows loading a module without loading/linking/evaluating its dependencies. It needs a way to load a module without doing a "full import", and it needs a way to later load/link/evaluate its dependencies.
• "Layer 0" of Compartments allows implementing custom module loaders, and thus needs to specify how the graph loading algorithm works. This is currently only handled by hosts, for example HTML defines it in fetch a single module script and fetch the descendants of a module script.

Instead of adding multiple new host hooks and duplicating the graph fetching algorithm, we can:

• expose a single host hook, HostLoadImportedModule(referrer, specifier), that loads a single Module Record and can copmlete asynchronously: it's an async version of HostResolveImportedModule.
• move the graph traversal logic for the load phase from hosts to ECMA-262: it delegates the actual fetching to hosts with HostLoadImportedModule, which is called recursively for each dependency.

Module Records will expose a third method, other than .Link() and .Evaluate(): .LoadRequestedModules(), which is the one responsible of calling HostLoadImportedModule.

The host algorithm to load/execute a top-level module (for example, using a <script type="module"> tag) will now look like this.

1. (HOST) Fetch the top-level Module Record
2. (HOST) Call module.LoadRequestedModules():
   • (ECMA-262) For each dependency, call HostLoadImportedModule(module, specifier):
     • (HOST) Fetch the corresponding Module Record and return it.
3. (HOST) When it finishes, call module.Link()
   • ... internal steps in ECMA-262 ...
4. (HOST) Call module.Evaluate()
   • ... internal steps in ECMA-262 ...

The algorithm for dynamic import will look like this.

1. (ECMA-262) Call HostLoadImportedModule(referrer, specifier):
   • (HOST) Fetch the corresponding Module Record and return it.
2. (ECMA-262) Call module.LoadRequestedModules():
   • (ECMA-262) For each dependency, call HostLoadImportedModule(module, specifier):
     • (HOST) Fetch the corresponding Module Record and return it.
3. (ECMA-262) When it finishes, call module.Link()
4. (ECMA-262) Call module.Evaluate()

For the Module Blocks proposal, the algorithm to import a module block will look like the above dynamic import algorithm, except that we skip step 1.

For the Import Reflection proposal, the algorithm to load an uninstantiated JS module will just call HostLoadImportedModule without then calling the various Module Record methods.

For the Compartments proposal, we will be able to use the new graph loading algorithm by just replacing calls to HostLoadImportedModule with calls to an user-provided function.

---

Some more precise remarks about these HTML changes

1. The only observable change (observable from HTML, rather than users) is that when you import the same specifier twice from the same file, only the first one causes a call to HostLoadImportedModule (unless the first one fails):

   await import("./dep.js"); // Calls HostLoadImportedModule
   await import("./dep.js"); // Returns the already loaded module

   In practice this doesn't change much, because HostImportModuleDynamically/HostResolveImportedModule were already required to return the same Module Record both times.

   We still call the host hook multiple times if the first dynamic import fails, because there was an explicit goal of allowing to retry failed dynamic imports (Normative: change idempotency for HostImportModuleDynamically tc39/ecma262#1645).
   EDIT: For now I reverted this change, since even if it's not observable by HTML it's observable in other places.

2. The first commit is a small refactor only within HTML (it doesn't affect layering) that makes it easier to move the graph traversal to ECMA-262. I believe that it should not have observable effects (I wrote why in the commit description), but if it does I'll find an alternative solution.

3. I had to add an hostDefined parameter to LoadRequestedModules, which ECMA-262 passes as-is to HostLoadImportedModule, so that HTML can propagate the correct request destination. I couldn't just store the destination on the referrer Module Record's [[HostDefined]] slot because it should only be propagated through static imports and not dynamic ones.

4. On the ECMA-262 side, this refactor doesn't take into account import assertions, because they have not been merged to the main spec yet. I wrote these HTML changes assuming that HostLoadImportedModule will receive a { [[Specifier]], [[Assertions]] } record instead of just the specifier, similarly to how the import assertions proposal updated HostResolveImportedModule to receive { [[Specifier]], [[Assertions]] }.

---

Links

• ECMA-262 PR: Layering: Add HostLoadImportedModule hook tc39/ecma262#2905
• ECMA-262 preview: https://ci.tc39.es/preview/tc39/ecma262/pull/2905
• Slides presented in the TC39 meeting: https://docs.google.com/presentation/d/1RVUE-MENQT8dj2wxvMLMDxg_VoMOwiwNQQged39QIEU/edit?usp=sharing

---

/index.html ( diff )
/infrastructure.html ( diff )
/webappapis.html ( diff )

[domenic]

I am skeptical about this change, as the goal of moving graph traversal out of HTML's control and into ES-262's seems likely to be problematic for future extensions we might want to add on the host side. However, I do not have any concrete examples of problems it will cause in mind. Similarly, I am very skeptical about the three proposals you mention, so am unsure about doing work on this side to support them; but, perhaps that's something we can wait for later to figure out.

Can you address how well your changes here would work with proposals such as #6768 and #7996? Similarly, would they allow #4400 or the path-not-taken for #4419? (The latter are not current problems, but knowing whether they would be possible within this framework would help put my mind at ease about whether we are losing important abilities via this change, or not.)

[nicolo-ribaudo]

Thank you for taking a look at this!

---

Failed dynamic import should not always be cached (#6768)

HostLoadImportedModule does not cache (and does not require caching) failures: when you import the same (referrer, specifier) pair twice, if the first import results in a failure it will call HostLoadImportedModule also the second time.

Failure caching is only implemented in HTML, and this PR does not change how we can remove it. We can support retrying by replaciny step 6 of fetch a single module script with "If moduleMap[(url, moduleType)] exists and is not null, asynchronously complete this algorithm with moduleMap[url / moduleType], and return.".

Editorial cleanups to fetching scripts (#7996) _{(I love this, it addresses all the doubts I initially had when reading these algorithms :) )}

• Everything takes a (probably named) onComplete argument, to replace the current "asynchronous completion" paradigm. Yes, callers will have extra indentation. Oh well.

There are a few less places where we need to change this (with this PR there are 39 occurrences of "asynchronously complete" instead of 54 in webappapis.html), but except for that it remains the same task.

• Make "perform the fetch" an optional named argument and thread it through explicitly.

Are the "perform the fetch" step passes through to fetch the descendants of and link, for example in the last step of fetch an external module script graph?

• Yes → we need to add a performTheFetch "function" to the options bag passed to module.LoadImportedModule(options), which is passed as-is to all the HostLoadImportedModule calls.
• No → if it only affects the loading of the top-level module, then this PR doesn't affect it because the top-level loading is still done before calling ECMA-262 algorithms.

• Use modern Fetch's callbacks. Notably processResponseConsumeBody is better than how we operate on the "body" concept.

We can/should still do it; it makes it clearer when we invoke HostLoadImportedModules synchronously/asynchronously (for example, we could invoke it synchronously when returning a module which is already in the importMap).

Yielding to the event loop (in some way) between module evaluations (#4400)

This refactor doesn't change "how much async" module.Evaluate() is, so it doesn't affect that proposal. A proper solution would probably require a new host hook in Evaluate like HostContinueGraphEvaluation(callback) which must just call callback but it could do it either synchronously (default and current behavior) or asynchronously (after yielding to the event loop).

It is possible to "hack a solution" to that with this refactor (or even with the current spec), by replacing step 11 of HostLoadImportedModule with something like this:

11. Otherwise,
    1. let completion be Completion { [[Type]]: normal, [[Value]]: result's record, [[Target]]: empty }.
    2. After this algorithm returns, optionally perform the following steps:
       1. Let module be result's record.
       2. Await module.LoadRequestedModules(loadState).
       3. module.Link()
       4. module.Evaluate()

to evaluate module even if the graph is still loading, just making sure that all its dependencies have already been loaded (11.ii.2). However, it feels "wrong" to hack the expected semantics like this (even if it's compliant to the hook requirements), and I would strongly suggest proposing a first-class mechanism to do it.

path-not-taken for Top-level await integration and multiple script graphs (#4419)

This is at a different level from this PR, and it touches different parts of the spec. Execute the script element would have to await the promise returned by run the module script instead of just ignoring it.

---

I also took a look at other issues related to module loading:

• This PR fixes Behavior of cyclical dynamic import() is unclear #7472, because the refactored dynamic import algorithm on the ECMA-262 side ensures that evaluation of the imported module is delayed by at least one promise tick.
• This PR fixes Track the source of fetching errors in module graphs #3025, because step 9 of HostLoadImportedModule creates an TypeError for the single module that fails loading, rather than for the whole module graph (or do we need to explicitly associate the URL with that TypeError?).
• This PR is unrelated to Web compat: don't run async scripts between adjacent sync <script>s #6761, Fetching worker script algorithms should take "perform the fetch" as an optional param. #6339, and Add means to detect if a script is loading, has loaded, or has errored #4594.
• import() should use referring script's base URL as the referrer #3744 made me realize that this PR has a bug, because it always sets referrer as "client". I'll push a commit to fix this regression, and it will also fix import() should use referring script's base URL as the referrer #3744 (because HostLoadImportedModule will compute the referrer without differentiating between static and dynamic import).

Ultimately one goal of this refactor is to not change host capabilities, and the issues fixed by this PR could also be fixed with the existing layering hooks.

---

I also tried a "minimal" alternative approach. Instead of deleting the "recurse into imported modules" algorithm from HTML, HostLoadImportedModules(referrer, specifier) can look like this:

1. Let baseURL be the result of resolving (referrer, specifier).
2. If the module record corresponding to baseURL has already been loaded, synchronously return it.
3. Otherwise,
   1. Fetch the module record corresponding to baseURL, and all its recursive dependencies. Wait until it asynchronously completes.
   2. Return the module record.

This is more similar to the existing two hooks: use cases of HostResolveImportedModule are covered by steps 1 and 2, while use cases of HostImportModuleDynamically are covered by steps 1 and 3.

That leaves most of the HTML algorithms intact, to better show that this refactor shouldn't change host capabilities. If you are interested, I can complete/rebase it and open a parallel PR.

[domenic]

Thank you so much for the thorough, above-and-beyond analysis! This addresses my concerns. At this point I'm somewhat eager for the refactor to land, and no longer think we need to wait for a TC39 proposal that needs it.

Are the "perform the fetch" step passes through to fetch the descendants of and link, for example in the last step of fetch an external module script graph?

Yes, that is the intent. (With the "is top-level" flag being used to allow branching behavior as necessary.)

I guess we can make this work by using the [[HostDefined]] field of the ModuleLoadState Record?

Speaking of that, I noticed that it says loadState is undefined for dynamic import(). Is that a problem? Is it something we could change in the future, if e.g. we wanted to change the fetch destination for dynamic import(), or change the "perform the fetch" steps?

It is possible to "hack a solution" to that with this refactor (or even with the current spec), by replacing step 11 of HostLoadImportedModule with something like this:

Well, with the current spec, we could just choose not to call the top-level Evaluate(). Instead we would Evaluate() portions of the module graph, as the host determines would be best. This seems possible because the host has the graph-traversal power, so it can delay the scheduling of Evaluate() on various subgraphs as necessary.

It sounds like maybe we have another mechanism for exerting that control, by basically inserting host-defined top-level awaits? Maybe that's enough; back when we first raised #4400, top-level await wasn't integrated, so that option wasn't available.

This PR fixes Behavior of cyclical dynamic import() is unclear #7472, because the refactored dynamic import algorithm on the ECMA-262 side ensures that evaluation of the imported module is delayed by at least one promise tick.

This PR fixes Track the source of fetching errors in module graphs #3025, because step 9 of HostLoadImportedModule creates an TypeError for the single module that fails loading, rather than for the whole module graph (or do we need to explicitly associate the URL with that TypeError?).

This is great news!

it will also fix import() should use referring script's base URL as the referrer #3744 (because HostLoadImportedModule will compute the referrer without differentiating between static and dynamic import).

Fixing this would be a normative change, and thus require web platform tests, and (depending on what the tests show) possibly implementer agreement. So it might be best to hold off on that for now?

I also tried a "minimal" alternative approach. Instead of deleting the "recurse into imported modules" algorithm from HTML, HostLoadImportedModules(referrer, specifier) can look like this:

Oh, this is very helpful. No need to write up a full PR with this, but knowing that we have that option in our back pocket if necessary is really helpful.

[nicolo-ribaudo]

Speaking of that, I noticed that it says loadState is undefined for dynamic import(). Is that a problem? Is it something we could change in the future, if e.g. we wanted to change the fetch destination for dynamic import(), or change the "perform the fetch" steps?

We need that LoadStateRecord.[[HostDefined]] record to differentiate between "the current graph loading process" (static imports) and "future graph loading processes" (dynamic imports). Each loading process has comes from a separate LoadRequestedModules call, and thus can have its own loading-process-specific data.

If we wanted to pass data from the referrer module to the imported module, regardless of when its imported, then we can store it on the module script struct, which HostLoadImportedModule can access using referrer.[[HostDefined]] (this was my original attempt, until I realized that the destination for dynamic import is always "script" and thus I introduced LoadStateRecord.[[HostDefined]]).

Fixing this would be a normative change, and thus require web platform tests, and (depending on what the tests show) possibly implementer agreement. So it might be best to hold off on that for now?

Ok, I'll keep it for a future PR. It's fun that with this refactor is easier to fix it than to not, and I'll have to explicitly unset the referrer for dynamic imports :)

[nicolo-ribaudo]

@domenic I have rebased this on top of the asynchronous completions removal, fixed the referrer handling to restore the old behavior and properly fixed the "perform the fetch" steps propagation.

I plan to continue working on #7996 (which obviously has conflicts with this PR), but I would still love a review of this one in parallel.

[annevk]

Minor drive-by nits, deferring to @domenic for overall review.

[annevk]

Is the plan to land this with these kind of links? Or will we wait for the JS PR to land and then adjust this?

[nicolo-ribaudo]

We can land the JS PR first and then adjust the links here, but the JS PR is blocked on this one being reviewed.

[annevk]

This might slightly clash with @domenic's #8075 btw. In particular the referrer-related aspects look like they conflict.

[nicolo-ribaudo]

Just at a diff level, right? Because to merge both PRs we can do

1. If referrer is a Script Record or a Module Record, let referencing script be referrer.[[HostDefined]]
2. Else, let referencing script be null.

[domenic]

I'll mark "do not merge yet" blocking on landing the JS PR first. I didn't realize this needed review; will do that now.

[domenic]

Generally looks good. I think we have some editorial work to do, which we'll probably make harder for ourselves due to the other concurrent module-related PRs. But I have no general objections.

[domenic]

Hmm, will proposal-import-assertions be updated at the same time? We had readers confused recently in #8262 by the HTML spec not referring to a version of HostResolveImportedModule that worked well with import assertions; I hope we can avoid regressing on that.

[domenic]

I'll mark "do not merge yet" blocking on landing the JS PR first. I didn't realize this needed review; will do that now.

[domenic]

I guess this change is fine, since the two consumers of "is top-level" (modulepreload and service workers) don't care about whether dynamic import() triggers that code path or not?

[nicolo-ribaudo]

Exactly, that flag only has effect if:

• the destination is "worker" or "serviceworker", but for dynamic imports it's always "script", or
• there are custom perform the fetch steps, but they are never present for dynamic imports

[nicolo-ribaudo]

Thanks for the review! I plan to prepare a PR in the import assertions proposal (edit: tc39/proposal-import-attributes#124), but I don't know how long it will take to get merged. I added a <p class="note"> to the HostLoadImportedModule implementation explaining how it expects the import assertions proposal to update the hook's signature, to avoid confusion about mismatched types.

[nicolo-ribaudo]

I rebased this on top of the import maps changes, hopefully I didn't miss anything.

While doing so, I found a bug in the workaround to preserve #3744: instead of checking if the current import was directly caused by import(), I'm checking if it's caused by import() even indirectly (for example, by a static import in a dynamically imported file). I'll push a fix soon.

[domenic]

LGTM with nits. I would prefer if import assertions were merged into the JS spec as close as possible to your refactoring, to avoid the temporary confusion. But this is reasonable for now.

[domenic]

Why change the style=""?

[nicolo-ribaudo]

Since the image got smaller, width: 100% was zooming it a lot making the text in the image way bigger than the other text in the document. width: 80% was my first attempt to make it smaller and I forgot to put it back at 100%; it's now width: 100%; max-width: 1024px.

[domenic]

We try to treat "let" as block-scoped, so it's not great to declare them inside ifs (even single-line ifs). So maybe make substep 1 establish the default (currently set on step 3), and then the other substeps can be changed from "let" to "set".

[nicolo-ribaudo]

I ended up initializing it as null and keep the Completion Record creations where they are, it felt strange to initialize it to a record representing success and then later be like "oh actually, it was an error".

[nicolo-ribaudo]

The ECMA-262 side of this refactor has been merged, I'll rebase this PR now!

[nicolo-ribaudo]

Note: tc39/proposal-import-attributes#124 has not been merged yet, but the HTML implementation of HostLoadImportedModule has a note explaining how it expects the import assertions proposal to update its signature. I will open a PR to remove that note once the import assertions PR is merged.