Add tests for interaction between import() and microtask queue

[nicolo-ribaudo]

The HTML algorithms for dynamic import (starting from HostImportModuleDynamically) only schedule a task a when actually fetching the corresponding files, and it never happens if the imported module (and all its dependencies) have already been loaded (when step 5 of fetch a single module script doesn't happen and step 6 returns early).

I didn't know how to test "it doesn't schedule a task", so the tests are written using a loop that schedules many microtasks and checking if all those microtasks are run or not before that the import() promise is resolved.

---

Browsers implement this inconsistently. This PR adds three new tests, and some of them fail:

Test	Firefox	Chrome	Safari
import() should not drain the microtask queue if it fails during specifier resolution	✔️	✔️	✔️
import() should not drain the microtask queue if it fails while validating the 'type' assertion	❔	✔️	✔️ 1
import() should not drain the microtask queue when loading an already loaded module	❌	❌	✔️
import() should drain the microtask queue when fetching a new module	✔️	✔️	✔️

It's possible that the Firefox/Chrome behavior is because the HTML spec uses a rare "asynchronous completions" wording which isn't defined anywhere. However, "asynchronous completions" are used for all the test cases, so I would expect the 1st to always behave like the 2nd (since they both only involve asynchronous completions and no fetching). I also opened whatwg/html#8264 to move away from that wording.

Footnotes

1. The fix has not been released yet, so it fails on CI. ↩

[domenic]

So I think we have two consistent options:

1. Always schedule a full task (i.e., always drain the microtask queue) to resolve the import() promise
2. Always synchronously resolve the import() promise if possible (i.e. if no network is involved)

It sounds like Safari follows option (2), and no browser follows option (1). So a path of least resistance would be to spec (2) and update Chrome/Firefox.

@hiroshige-g, what do you think from Chrome's perspective?

[nicolo-ribaudo]

For context, I discovered this behavior when working on tc39/ecma262#2905 / whatwg/html#8253.
With that refactor, ECMA-262 already knows what the result of the second dynamic import call will be and could potentially avoid calling the host hook again (especially given that the host hook is required to "return" the same result). However, I had to keep that unnecessary HTML<->ECMA-262 call so that hosts can decide when to "return" and potentially inject an event loop spin.

[hiroshige-g]

When we implemented module loading, I interpreted the "asynchronous completion" in the spec as something like posting a task to make it always async, while whatwg/html#8264 made it explicitly synchronous in some cases. What was the intention of the "asynchronous completion"?

So the Chromium implementation is roughly:

(3) Always schedule at least one full task (i.e., always drain the microtask queue) for the fetch * module graph algorithms

This is consistent with the test result. import() with invalid URL is rejected synchronously because module specifier resolution is outside the fetch * module graph algorithms.

[nicolo-ribaudo]

Note that URL resolution/validation happens inside fetch an import() module script graph, and before whatwg/html#8264 HostImportModuleDynamically called that algorithm using the "asynchronous completions" pattern (so it should have been consistent with the other parts of fetch ... that do not involve network requests).

[domenic]

What was the intention of the "asynchronous completion"?

I can't really say :(. I was bad and didn't think properly about it.

import() with invalid URL is rejected synchronously because module specifier resolution is outside the fetch * module graph algorithms.

As @nicolo-ribaudo, I don't think that's correct. HostImportModuleDynamically calls fetch an import() module script graph, which if resolution fails, used the "asynchronously complete this algorithm with null" wording.

After whatwg/html#8264, it uses sync wording, but we're wondering what the best path is...

Re-reading the algorithms I noticed we have a number of early-failure modes, actually:

• use inside WorkletGlobalScope and ServiceWorkerGlobalScope
• module specifier resolution failure
• moduleType is not "css", "json", or "javascript"
• moduleType is "css" and we're inside a WorkerGlobalScope

We should probably be consistent with all of these? (And ideally even test all of them.)

So generalizing, I think the options on the table are:

1. Always schedule a full task, including for early-success and early-failure
2. Always perform early-success and early-failure synchronously
3. (Chrome's current behavior, probably) Perform early-success with a full task, and early-failure synchronously

Reiterating my earlier comment, Chrome's current behavior feels inconsistent to me so I would prefer (1) or (2). And Safari matches (2).

[nicolo-ribaudo]

I added a test for invalid type assertions, and the results are even more inconsistent 😅

EDIT: The Safari ❌ maybe be related to the fact that it doesn't properly validate assertions (https://wpt.fyi/results/html/semantics/scripting-1/the-script-element/import-assertions/invalid-type-assertion-error.html?label=master&label=experimental&aligned=&view=subtest&q=import-assertions) but (I think, based on the Webkit source code because I don't have access to Safari TP) just fallbacks to javascript, so it performs the fetch and thus it drains the microtask queue.

[codehag]

Thanks for this @nicolo-ribaudo. Option 2 makes the most sense to me, I don't think there will be an issue updating our behavior to match. I think our interpretation of the asynchronous completions was the similar to hiroshige's, but jonco can confirm if that is true. ccing @jonco3 and @smaug---- to get a couple more eyes from mozilla on this.

I don't believe assertions have shipped yet on our side (https://bugzilla.mozilla.org/show_bug.cgi?id=1777526).

[nicolo-ribaudo]

Now that https://bugs.webkit.org/show_bug.cgi?id=246207 has been fixed, the Safari column should be all ✔️

[domenic]

These look great modulo a minor fix, and they test the desired outcome.

It would be ideal if you also expanded to test:

• moduleType is "css" and we're inside a WorkerGlobalScope
• use inside WorkletGlobalScope
• use inside ServiceWorkerGlobalScope

but of course we're happy to accept just these too.

I believe the spec as written also aligns with these tests, correct? In which case we can merge this shortly.

To fix CI failures, often rebasing on master works.

[nicolo-ribaudo]

Is there a way to say "run this test in a worker/etc", or do I have to write a normal test that spins up a worker/etc?

I believe the spec as written also aligns with these tests, correct?

Yes 👍

[domenic]

The easiest way is to use .any.js tests with a custom global= configuration.

Worklets are harder. I guess maybe modify the test I added in 43fb3b2 .

[nicolo-ribaudo]

I'm not sure about what to do with the CI failure, it looks like the basic tests sometimes timeout in Firefox.

[domenic]

LGTM. The CI is often flaky (see #21706) and so in these cases we summon one of the admins to help force-merge... I'll work on that.