Add moduleRequest to modulepreload fetch parameters#12223
Add moduleRequest to modulepreload fetch parameters#12223KurtCattiSchmidt wants to merge 7 commits intowhatwg:mainfrom
Conversation
|
I was thinking that module type from module request should be performed inside fetch a single imported module script and then optionally passing moduleType; I don't see a reason to have a ModuleRequest record when fetching a modulepreload. |
I may be misunderstanding - the original issue referenced #11981, which allows This approach came from @annevk's remark #12214 (comment) - since |
Sorry I was not clear, by running module type from module request inside fetch a single imported module script, fetch a single module script will optionally have a moduleType instead of a ModuleRequest record. |
Ok, I think I understand now. fetch a single imported module script already has module type from module request as step 2. Are you suggesting adding moduleType as a parameter to fetch a single module script and passing that through in both places? The reason why I didn't do that initially is that fetch a single module script already has moduleRequest, which has the module type, so I didn't want to duplicate that. |
My suggestion is to replace moduleRequest with moduleType |
You're right, that is a lot better! Addressed in the latest version. I believe I got all of the entry points handled - let me know if there's anything I'm missing. |
source
Outdated
| <li><p>If <var>moduleRequest</var> was given, then set <var>moduleType</var> to the result of | ||
| running the <span>module type from module request</span> steps given | ||
| <var>moduleRequest</var>.</p></li> | ||
| <li><p>If <var>moduleType</var> is null, set <var>moduleType</var> to "<code |
There was a problem hiding this comment.
I'm not fully familiar with all the syntax nuances of the spec yet, but I think since moduleType is an optional argument we should keep but rephrase If moduleType was given, then set moduleType to moduleType.
There was a problem hiding this comment.
Can't you get it from the destination parameter you already have rather than pass it as an additional parameter? You have to do that for modulepreload anyway.
There was a problem hiding this comment.
Went with @noamr's suggestion to move this to https://html.spec.whatwg.org/#fetch-a-single-module-script
There was a problem hiding this comment.
This comment implies that this approach won't work, but I think it covers it. @noamr, what do you think?
source
Outdated
| <li><p>Let <var>moduleType</var> be the result of running | ||
| <span>module type from fetch destination</span> on <var>destination</var>.</p></li> |
There was a problem hiding this comment.
I would move this inside fetch a modulepreload module script graph.
source
Outdated
| <li><p>If <var>moduleRequest</var> was given, then set <var>moduleType</var> to the result of | ||
| running the <span>module type from module request</span> steps given | ||
| <var>moduleRequest</var>.</p></li> | ||
| <li><p>If <var>moduleType</var> is null, set <var>moduleType</var> to "<code |
There was a problem hiding this comment.
Can't you get it from the destination parameter you already have rather than pass it as an additional parameter? You have to do that for modulepreload anyway.
|
Thanks for the suggestions @noamr and @Gae24! This is looking a lot cleaner. One weird part is that https://html.spec.whatwg.org/#fetch-a-single-imported-module-script gets |
| <li><p>Let <var>moduleType</var> be the result of running | ||
| <span>module type from fetch destination</span> on <var>destination</var>.</p></li> |
There was a problem hiding this comment.
Unfortunately this wont cover all the cases. When fetching a single imported module script, destination will either be script or the value stored inside LoadState (eg. worker, sharedworker).
We still need to resolve moduleType from moduleRequest record in this case.
There was a problem hiding this comment.
module type from fetch destination currently covers that, because the default case there is "javascript-or-wasm", which all of those request destinations are (worker, sharedworker), right?
There was a problem hiding this comment.
Exactly, moduleType will be javascript-or-wasm even when fetching json or css modules. That's because moduleType is determined by the moduleRequest record not by the top-level destination.
Fixes #12214
(See WHATWG Working Mode: Changes for more details.)
/webappapis.html ( diff )