Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
allstarschh opened this issue Feb 15, 2022 · 4 comments

Comments

@allstarschh
Copy link

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
Copy link
Collaborator

domenic commented Feb 18, 2022

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
Copy link
Author

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 added a commit to whatwg/html that referenced this issue Jul 5, 2022
This imports much of the specification from https://wicg.github.io/import-maps/. The main deltas are:

* Does not include support for external import maps. This allows a good deal of simplification, such as by changing "acquiring import maps" + "pending import map script" + "wait for import maps" to a single "import maps allowed" boolean, or integrating "register an import map" into "execute the script element". If a src="" attribute is present on an importmap script element, then an error event is fired.

* Does not include any support for non-Window import maps. The spec draft included stub support for those, with other globals always having an empty import map. This instead ties import maps to Windows directly. If we revisit that in the future we can refactor, but keeping always-empty import maps around on the other globals was clumsy.

* Adds introductory text and examples.

* Adds conformance requirements, both on the script element and on the import map JSON.

* Fixes a few minor specification issues: WICG/import-maps#267, WICG/import-maps#267, WICG/import-maps#268, WICG/import-maps#270, WICG/import-maps#276, WICG/import-maps#277.
@domenic
Copy link
Collaborator

domenic commented Jul 5, 2022

I believe this is fixed in whatwg/html#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
Copy link
Author

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.

domenic added a commit to whatwg/html that referenced this issue Oct 5, 2022
This imports much of the specification from https://wicg.github.io/import-maps/. The main deltas are:

* Does not include support for external import maps. This allows a good deal of simplification, such as by changing "acquiring import maps" + "pending import map script" + "wait for import maps" to a single "import maps allowed" boolean, or integrating "register an import map" into "execute the script element". If a src="" attribute is present on an importmap script element, then an error event is fired. See #8355.

* Does not include any support for non-Window import maps. The spec draft included stub support for those, with other globals always having an empty import map. This instead ties import maps to Windows directly. If we revisit that in the future we can refactor, but keeping always-empty import maps around on the other globals was clumsy.

* Adds introductory text and examples.

* Adds conformance requirements, both on the script element and on the import map JSON.

* Fixes a few minor specification issues: WICG/import-maps#267, WICG/import-maps#267, WICG/import-maps#268, WICG/import-maps#270, WICG/import-maps#276, WICG/import-maps#277.
domenic added a commit to whatwg/html that referenced this issue Oct 5, 2022
This imports much of the specification from https://wicg.github.io/import-maps/. The main deltas are:

* Does not include support for external import maps. This allows a good deal of simplification, such as by changing "acquiring import maps" + "pending import map script" + "wait for import maps" to a single "import maps allowed" boolean, or integrating "register an import map" into "execute the script element". If a src="" attribute is present on an importmap script element, then an error event is fired. See #8355.

* Does not include any support for non-Window import maps. The spec draft included stub support for those, with other globals always having an empty import map. This instead ties import maps to Windows directly. If we revisit that in the future we can refactor, but keeping always-empty import maps around on the other globals was clumsy.

* Adds introductory text and examples.

* Adds conformance requirements, both on the script element and on the import map JSON.

* Fixes a few minor specification issues: WICG/import-maps#267, WICG/import-maps#267, WICG/import-maps#268, WICG/import-maps#270, WICG/import-maps#276, WICG/import-maps#277.
domenic added a commit to whatwg/html that referenced this issue Oct 5, 2022
This imports much of the specification from https://wicg.github.io/import-maps/. The main deltas are:

* Does not include support for external import maps. This allows a good deal of simplification, such as by changing "acquiring import maps" + "pending import map script" + "wait for import maps" to a single "import maps allowed" boolean, or integrating "register an import map" into "execute the script element". If a src="" attribute is present on an importmap script element, then an error event is fired. See #8355.

* Does not include any support for non-Window import maps. The spec draft included stub support for those, with other globals always having an empty import map. This instead ties import maps to Windows directly. If we revisit that in the future we can refactor, but keeping always-empty import maps around on the other globals was clumsy.

* Adds introductory text and examples.

* Adds conformance requirements, both on the script element and on the import map JSON.

* Fixes a few minor specification issues: WICG/import-maps#267, WICG/import-maps#268, WICG/import-maps#270, WICG/import-maps#276, WICG/import-maps#277.
domenic added a commit to whatwg/html that referenced this issue Oct 5, 2022
This imports much of the specification from https://wicg.github.io/import-maps/. The main deltas are:

* Does not include support for external import maps. This allows a good deal of simplification, such as by changing "acquiring import maps" + "pending import map script" + "wait for import maps" to a single "import maps allowed" boolean, or integrating "register an import map" into "execute the script element". If a src="" attribute is present on an importmap script element, then an error event is fired. See #8355.

* Does not include any support for non-Window import maps. The spec draft included stub support for those, with other globals always having an empty import map. This instead ties import maps to Windows directly. If we revisit that in the future we can refactor, but keeping always-empty import maps around on the other globals was clumsy.

* Adds introductory text and examples.

* Adds conformance requirements, both on the script element and on the import map JSON.

* Fixes a few minor specification issues: WICG/import-maps#267, WICG/import-maps#268, WICG/import-maps#270, WICG/import-maps#276, WICG/import-maps#277.
domenic added a commit to whatwg/html that referenced this issue Oct 5, 2022
This imports much of the specification from https://wicg.github.io/import-maps/. The main deltas are:

* Does not include support for external import maps. This allows a good deal of simplification, such as by changing "acquiring import maps" + "pending import map script" + "wait for import maps" to a single "import maps allowed" boolean, or integrating "register an import map" into "execute the script element". If a src="" attribute is present on an importmap script element, then an error event is fired. See #8355.

* Does not include any support for non-Window import maps. The spec draft included stub support for those, with other globals always having an empty import map. This instead ties import maps to Windows directly. If we revisit that in the future we can refactor, but keeping always-empty import maps around on the other globals was clumsy.

* Adds introductory text and examples.

* Adds conformance requirements, both on the script element and on the import map JSON.

* Fixes a few minor specification issues: WICG/import-maps#267, WICG/import-maps#268, WICG/import-maps#270, WICG/import-maps#276, WICG/import-maps#277.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants