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•1 year 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•1 year ago
|
Assignee | ||
Comment 2•1 year 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•1 year 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•1 year ago
|
||
Assignee | ||
Comment 5•1 year ago
|
||
Assignee | ||
Comment 6•1 year ago
|
||
Assignee | ||
Comment 7•1 year 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•1 year ago
|
||
Assignee | ||
Comment 9•1 year ago
|
||
Assignee | ||
Comment 10•1 year ago
|
||
Assignee | ||
Comment 11•1 year ago
|
||
Assignee | ||
Comment 12•1 year ago
|
||
If an adding import map is disallowed, we bail out early to prevent creating a
ScriptLoadRequest.
Assignee | ||
Comment 13•1 year ago
|
||
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Comment 14•1 year 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•1 year 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•1 year 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•1 year 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•1 year 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•1 year ago
|
||
The PR has been merged.
See https://html.spec.whatwg.org/multipage/webappapis.html#import-maps
Comment 20•1 year ago
|
||
Pushed by allstars.chh@gmail.com: https://hg.mozilla.org/integration/autoland/rev/88647780854a Part 1: Rename 'parse a URL-Like import specifier' to 'resolve a URL-Like module specifier'. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/b68b8f0d608c Part 2: Rename to 'import maps allowed' and add 'disallow import maps'. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/3e2e1fbc651a Part 3: Update spec links in ModuleLoaderBase::ParseImportMap. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/829f0f41f7fc Part 4: Don't call onerror if parsing import maps fails. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/6489cf84de36 Part 5: Update spec in RegisterImportMap. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/3e1a89dcc16c Part 6: Update comments for external import maps. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/03257f7c2f05 Part 7: Update spec in ImportMaps.cpp/h. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/69c1c66dd528 Part 8: Update spec for prepare-script when type="importmap". r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/b17fa8f361f1 Part 9: Bail out early if adding an import map is disallowed. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/fcd66d7f4917 Part 10: Update comments for 'register an import map'. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/4a1dc4785996 Part 11: Update comments for ResolveModuleSpecifier. r=jonco,yulia https://hg.mozilla.org/integration/autoland/rev/36d3f80544ec Part 12: Update spec links as the PR is merged. r=jonco,yulia
Comment 21•1 year 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
•