Open bakura10 opened 1 year ago
Agreed that now that we have full browser support conditional loading would be worth fleshing out more clearly.
This is something that would need to be designed as a core feature of the project because we have feature detection scripts which would still need to run. So effectively we would change the structure of es-module-shims to first include a simple feature detection script, that in turn would dynamically import the main polyfill only when needed.
There is still a tradeoff on that for a little while while browsers that don't support import maps would be penalized by that extra round trip. But as import maps gain in widespread usage this seems like a must-have to me.
For those trying the approach I have outlined (dynamically importing the module) this does not work all the time. I have timing issues, so I will wait for a built-in approach :D.
For anyone interested in doing that, to make it work you actually need to append the element to the dom (and not try to dynamically importing it). This approach works to conditionally load the script:
<script>
if (!(HTMLScriptElement.supports && HTMLScriptElement.supports('importmap'))) {
const importMapPolyfill = document.createElement('script');
importMapPolyfill.async = true;
importMapPolyfill.src = "URL-TO-POLYFILL";
document.head.appendChild(importMapPolyfill);
}
</script>
It seems like conditionally loading is working pretty well as Wordpress landed in https://github.com/WordPress/gutenberg/pull/57256/files using a HTMLScriptElement.supports('importmap')
and document.write
approach.
Perhaps the solution here is then just simply to document this as a first-class feature and continue supporting it as the primary approach going forward.
Perhaps the solution here is then just simply to document this as a first-class feature and continue supporting it as the primary approach going forward
That sounds good to me!
To be honest, I don't know why document.head.appendChild
didn't work in our case. Maybe I did something wrong. I want to try again.
Or does it make sense that document.write
works, but document.head.appendChild
doesn't?
I'm going to need some time to properly dig into the list of DOM attachments and implications for dynamic attachment. The current assumptions for the shim are very much that it's there first for performance reasons and is part of the loading lifecycle of the page (refiring dom events as necessary for the new loaded scripts). I suppose at least the document.write
applies only in the legacy cases now so shouldn't apply for modern browsers at least, but agreed it would be preferable to figure out.
I'm going to track this issue and get back with further details when I find some time. In the mean time contributions are of course welcome too.
I suppose at least the
document.write
applies only in the legacy cases now so shouldn't apply for modern browsers at least
Yes. We don't mind releasing it in WordPress with document.write
because it will only affect a small and shrinking percentage of users. So it's not critical at all. But I wanted to understand why it needs to use that method before committing to it.
The current assumptions for the shim are very much that it's there first for performance reasons and is part of the loading lifecycle of the page
There's one part that still doesn't make sense to me. Your recommendation is:
<script async src="es-module-shims.js"></script>
Async scripts can load at any time. They are not even guaranteed to load before the DOMContentLoaded
event.
So, at least theoretically, there should be no difference between an async script and the document.head.appendChild
method.
Sorry for the delay here - if there is a well-tested PR that can support an appendChild
attachment I'm open to landing that.
We finally went with document.write
for WP 6.5.
Hi :),
This is not a bug but rather a question on how to embed the script. We are using this only for import map support. For now, we have followed the embed instructions to use an async script. As all browsers now support it natively, we would prefer to load it conditionally (we're not saving a lot, I know, but we are selling themes and a lot of our users are running their stores on PageSpeed, and those non-technical merchants often ask us how to "optimize their store by removing scripts" ; we therefore try to load as little JS as possible to avoid support debt).
I tried the following approach which works on all the iOS versions I have tested:
However, I am not 100% sure about possible consequences or potential timing issue. Considering this polyfill architecture, would this be as safe as embedding it using the async way?
Thanks.