Update Import-Maps implementation to meet the latest whatwg PR 8075
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox107 | --- | fixed |
People
(Reporter: allstars.chh, Assigned: allstars.chh)
References
(Blocks 1 open bug)
Details
Attachments
(12 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Assignee | ||
Comment 1•3 years ago
•
|
||
Below is the list of things changed:
- Parse a URL-like import specifier has been renamed to Resolve a URL-like module specifier.
- Register an import map has been updated, as the script's result will be import map parse result, and also now it's called during execute the script element (Previously it was called during prepare a script).
- Acquiring Import Maps has been renamed to Import Maps allowed, with the new added method: Disallow further Import Maps.
- Resolve a module specifier now takes a script-or-null and a string parameters, and will throw a TypeError if it fails. Originally it takes a URL and a string. See Original Resolve a module specifier. This also changes the callers of the resolve-a-module-specifier. Specifically when Create a Module Script, the exception will be set to the parse error(Step 10.2).
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #1)
Below is the list of things changed:
- Register an import map has been updated, as the script's result will be import map parse result, and also now it's called during execute the script element (Previously it was called during prepare a script).
As the register an import map now moves to execute the script element, I'll try to add a ImportMapLoadRequest to store the parsed import map (script's result from spec), and then register the import map parsed result in EvaluateScriptElement
- Resolve a module specifier now takes a script-or-null and a string parameters, and will throw a TypeError if it fails. Originally it takes a URL and a string. See Original Resolve a module specifier. This also changes the callers of the resolve-a-module-specifier. Specifically when Create a Module Script, the exception will be set to the parse error(Step 10.2).
From the spec,
For each ModuleRequest record requested of result.[[RequestedModules]]:
Let url be the result of resolving a module specifier given script and requested.[[Specifier]], catching any exceptions.
The tricky part is the "catching any exceptions", I'll check if we should address that or leave as it is now.
Assignee | ||
Comment 3•3 years ago
|
||
(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #2)
(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #1)
Below is the list of things changed:
- Register an import map has been updated, as the script's result will be import map parse result, and also now it's called during execute the script element (Previously it was called during prepare a script).
As the register an import map now moves to execute the script element, I'll try to add a ImportMapLoadRequest to store the parsed import map (script's result from spec), and then register the import map parsed result in EvaluateScriptElement
As explained by the spec author: https://github.com/whatwg/html/pull/8075#issuecomment-1196276156
the preparation-time document check will never fail for import maps.
For now we can ignore this spec change, I'll file another bug to address that if we need to register the import map when executing the script element.
Assignee | ||
Comment 4•3 years ago
|
||
Assignee | ||
Comment 5•3 years ago
|
||
Assignee | ||
Comment 6•3 years ago
|
||
Assignee | ||
Comment 7•3 years ago
|
||
script's result will be an import map parse result, which will never be
null.
See explanation from https://github.com/WICG/import-maps/issues/279
Assignee | ||
Comment 8•3 years ago
|
||
Assignee | ||
Comment 9•3 years ago
|
||
Assignee | ||
Comment 10•3 years ago
|
||
Assignee | ||
Comment 11•3 years ago
|
||
Assignee | ||
Comment 12•3 years ago
|
||
If an adding import map is disallowed, we bail out early to prevent creating a
ScriptLoadRequest.
Assignee | ||
Comment 13•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 14•3 years ago
|
||
As Jonco mentioned the spec link is still from the PR (The link is still https://whatpr.org/html/8075/...)
https://phabricator.services.mozilla.com/D152870#inline-843162
As I have no idea when the PR will be merged, so I wrote these patches with the links from the PR.
Once these patches are all got accepted, I'll leave a comment in the PR to notify that the patches are accepted.
We'll wait until the PR is merged, then I'll have a separate patch to update the links and land these patches
Assignee | ||
Comment 15•3 years ago
|
||
According to the spec, the method should throw a TypeError if it fails
to resolve the specifier. Here we add some comments to explain why we
return a result code instead of throwing an error.
Comment 16•3 years ago
|
||
There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:allstars.chh, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 17•3 years ago
|
||
We are still waiting for the PR to be merged, as it might be updated during the review process.
https://github.com/whatwg/html/pull/8075
Assignee | ||
Comment 18•3 years ago
|
||
The PR is merged yesterday.
https://html.spec.whatwg.org/multipage/webappapis.html#import-maps
I'll rebase my patches and update the spec link.
Assignee | ||
Comment 19•3 years ago
|
||
The PR has been merged.
See https://html.spec.whatwg.org/multipage/webappapis.html#import-maps
Comment 20•3 years ago
|
||
Comment 21•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/88647780854a
https://hg.mozilla.org/mozilla-central/rev/b68b8f0d608c
https://hg.mozilla.org/mozilla-central/rev/3e2e1fbc651a
https://hg.mozilla.org/mozilla-central/rev/829f0f41f7fc
https://hg.mozilla.org/mozilla-central/rev/6489cf84de36
https://hg.mozilla.org/mozilla-central/rev/3e1a89dcc16c
https://hg.mozilla.org/mozilla-central/rev/03257f7c2f05
https://hg.mozilla.org/mozilla-central/rev/69c1c66dd528
https://hg.mozilla.org/mozilla-central/rev/b17fa8f361f1
https://hg.mozilla.org/mozilla-central/rev/fcd66d7f4917
https://hg.mozilla.org/mozilla-central/rev/4a1dc4785996
https://hg.mozilla.org/mozilla-central/rev/36d3f80544ec
Description
•