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

Fix: async property of scripts is not assigned properly #674

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gp27
Copy link

@gp27 gp27 commented Mar 1, 2022

This fix addresses a problem that happens when trying to set the async attribute of a script tag.
Intuitively, setting the async attribute on the tag should also set the the element async property. However this is not the case.

This not a bug but part of the specification.
The async attribute is used only by the HTML/XML parser when the page is initially loaded, but it's ignored on dynamic scripts, which are async by default (you can check the section about "force-async": https://www.w3.org/TR/2014/PR-html5-20140916/scripting-1.html ).

For this reason script.setAttribute('async', value) will not work as expected.
In order to actually change the async behavior of the tag, the property must be set directly with script.async = value.

Setting async=false is particularly useful to maintain the execution order of dynamic scripts.
Inside issue #419 someone actually suggested this solution, but when I tried some code like the following one the tags executed out of order most of the time.

<Helmet>
  <script async={false} src="big-file.js" />
  <script async={false} src="small-file.js" /> 
</Helmet>

Related issues: #419

@CLAassistant
Copy link

CLAassistant commented Mar 1, 2022

CLA assistant check
All committers have signed the CLA.

@davideghz
Copy link

is this going to be merged?

@annale84
Copy link

Bump. When will this be merged?

@gp27
Copy link
Author

gp27 commented Jan 14, 2023

I'm going to leave here the patch file for anyone who might need it:
https://pastebin.com/fmHGfSFb

You can use patch-package to apply it in your project.

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

Successfully merging this pull request may close these issues.

None yet

5 participants