[AssetMapper] Remove async from the polyfill loading script
#59549
Conversation
|
Hey! Thanks for your PR. You are targeting branch "7.3" but it seems your PR description refers to branch "7.2". Cheers! Carsonbot |
| if (!HTMLScriptElement.supports || !HTMLScriptElement.supports('importmap')) (function () { | ||
| const script = document.createElement('script'); | ||
| script.src = '{$this->escapeAttributeValue($polyfillPath, \ENT_NOQUOTES)}'; | ||
| script.setAttribute('async', 'async'); |
There was a problem hiding this comment.
This is not what Mozilla doc states 🤔
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#notes
Scripts without async, defer or type="module" attributes, as well as inline scripts without the type="module" attribute, are fetched and executed immediately before the browser continues to parse the page.
And this seems to match html spec chart here
Not found either in the spec something that would suggest async is default (but i may have missed it)
There was a problem hiding this comment.
It’s from https://html.spec.whatwg.org/multipage/scripting.html#script-force-async:
A
scriptelement has a force async boolean, initially true. It is set to false by the HTML parser […] onscriptelements [it inserts], and when the element gets anasynccontent attribute added.
The polyfill script is not parser-inserted, but MDN only mentions these 😕
You can easily check a script-inserted script is async by default though: https://jsfiddle.net/nL509xph/
There was a problem hiding this comment.
It is set to false by the HTML parser and the XML parser on script elements they insert
There was a problem hiding this comment.
The polyfill script is not parser-inserted
Did you check the fiddle?
There was a problem hiding this comment.
I did check the fiddle :)
I'm just not entirely sure this is enough to state how the browsers will behave.
Your fiddle code "console log" attributes... this is not granted to be the behaviour off the parsers when they will start to load this (external) script.
And I think you will agree the MDN docs claims are something that creates doubts here.
I'm thinking except if we have a 100.00% insurance the script will always be loaded after the main JS files, the async attribute in the generated script does not cost us much, is perfectly W3 valid, and could be a safe bet.
:)
There was a problem hiding this comment.
If you don’t trust the browsers nor the spec I fear you cannot be sure of anything.
But then why would we trust the W3 validator? And are we really 100% sure removing the async attribute from the inline script won’t have any unattended consequence?
There was a problem hiding this comment.
If you don’t trust the browsers nor the spec I fear you cannot be sure of anything.
Not what i tried to say, nevermind :) If you're sure .. i don"t need to be convinced here 😅
My point was just a small fear I had this would have unwanted consequences.. that's all :)
|
Note that this is not a bugfix so it's for 7.3 |
c6e1ff8 to
164af69
Compare
|
Thank you @MatTheCat. |
From https://javascript.info/script-async-defer:
and