WICG / import-maps

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

Spec doesn't clearly define onerror must be called if import map is added after the 1st module load #268

Closed allstarschh closed 2 years ago

allstarschh commented 2 years ago

https://wicg.github.io/import-maps/#document-acquiring-import-maps

An import map is accepted if and only if it is added (i.e., its corresponding script element is added) before the first module load is started

But it doesn't mention onerror must be called, or define what is 'accepted'

The error event is defined later in register an import map

  1. If element’s the script’s result is null, then fire an event named error at element, and return.

But the registration is done in almost end of Prepare a script

Prepare a script (2) should have mentioned the error behavior when an import map is added after the 1st module load.

domenic commented 2 years ago

I'm unsure whether this is a bug report (i.e. the spec does not match the tests) or a request for increasing the spec's clarity.

The intent of the quoted section is to be a non-normative summary of the normative requirements elsewhere. And I think it achieves that at a high level. It doesn't give the full normative details on what "not accepted" means; if you go read the parts of the spec that consult the "acquiring import maps" flag, then you see that it means an error event is fired and the resulting import map is not registered. But I think it's OK for such notes, like code comments in implementations, to not fully duplicate the "what". They can focus on the "why".

But, if you found the note confusing, we can clarify it. And if you think the spec is just failing to properly reject import maps after the first module, then that's a bug and we should fix it!

allstarschh commented 2 years ago

This is a request for increasing the spec's clarity.

So the paragraph in Prepare a Script, is read to me like: The boolean 'acquiring Import Maps' is used to make sure there's only 1 import map script tag in the document. Because in the 'Prepare a Script' section there's only 1 place mentioning to set acquiring import maps to false.

But after reading the test in wpt of import maps, I realized I need to check the non-normative section of acquiring import maps as well, to find out that the boolean variable is also used to keep track of the first module load. And then onerror should be fired according to the spec

So I have to check the spec back and forth, and also have to check how the test is written in wpt, to figure out the exact meanings. And if something is tested in wpt, shouldn't the spec define it clearly?

domenic commented 2 years ago

I believe this is fixed in https://github.com/whatwg/html/pull/8075. Please check. If so, I will close this when I remove all spec text from this repository and instead let HTML be the new source of truth.

allstarschh commented 2 years ago

Fixed in prepare the script element, Step 31.2.

"importmap" If el's relevant global object's import maps allowed is false, then queue an element task on the DOM manipulation task source given el to fire an event named error at el, and return.