Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include the shim as defer instead of async #385

Open
JanPokorny opened this issue Aug 8, 2023 · 5 comments
Open

Include the shim as defer instead of async #385

JanPokorny opened this issue Aug 8, 2023 · 5 comments

Comments

@JanPokorny
Copy link

Hello! I have found a possible bug when using importShim for dynamic imports, that can manifest when the script is included in the recommended way with the async attribute.

This example (https://jsfiddle.net/tac5oz0v/), adapted from the README:

<script async src="https://ga.jspm.io/npm:es-module-shims@1.8.0/dist/es-module-shims.js"></script>

<script type="module">
  console.log(importShim);
</script>

...fails with Uncaught ReferenceError: importShim is not defined when ran in a browser that fully supports modules, and at the same time can execute defer scripts before async -- like Firefox 116.0. There is nothing preventing the type="module" script from executing before the shim, even though in this case the shim is still needed for the importShim function. This does not happen in browsers that can't run the module scripts without the shim (missing support for some feature which is polyfilled after the shim is loaded), since by that point the shim is already loaded by definition.

This is a simple example, but the issue can get pretty confusing when combined with a complex codebase with several levels of imports, testing in multiple browsers and better yet having the script inclusion hidden inside a gem (which correctly includes async according to the README). And it's non-deterministic by principle, which is fun to debug.

I would suggest changing the recommended way of inclusion of the shim from async to defer, which avoids this edge case -- the HTML spec requires defer and type="module" scripts to execute in the order of appearance.

@guybedford
Copy link
Owner

While defer will definitely work you typically don't want to delay all code execution on full parsing of the HTML. So I wouldn't recommend making this change.

Note that if you do console.log(importShim) your code isn't actually being polyfilled. Instead the usage pattern is to use features which require polyfilling (import maps), so the code fails early then gets polyfilled. <script type="module">import 'thing'</script>.

This then also ensures that the import maps polyfill being loaded doesn't block code exection when it isn't needed as well.

If you really need to check for and use the importShim global, then perhaps consider using shim mode, in which case yes the recommendation would be different. Ideally you shouldn't need to access the global in polyfill mode though.

@JanPokorny
Copy link
Author

@guybedford I have oversimplified the example a bit, apologies. So do I understand correctly that I don't actually need to use importShim(...) for dynamic imports since as long as the module fails on a static import and gets polyfilled, plain import(...) will also work? I may have misunderstood that part of README.

@guybedford
Copy link
Owner

@JanPokorny yes that's correct.

@JanPokorny
Copy link
Author

@guybedford Understood, thanks! In that case I understand that the only need for defer instead of async is in the edge case described in this section? https://github.com/guybedford/es-module-shims#polyfill-edge-case-dynamic-import IMO it's worth mentioning there (perhaps in the code example even) that this is the case since it may be non-obvious (at least I got confused a bit there 😅)

@guybedford
Copy link
Owner

@JanPokorny you're correct, we could update that note to note that doing this means changing to defer for the load order semantics. It's a shame browsers don't have an "async in order" mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants