-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
feat(analytics): defer google analytics script #22806
Conversation
I am not sure about this. Here is a quote from the doc you linked:
Also, the original snippet is from the official Google Analytics docs. This comment is also very relevant. Given this information, I incline to close this PR. What do you think? |
Hi ! Thank you for the answer 😄 On that very comment, I don't think that sites built with gatsby fit into the "on sites that load a lot of JavaScript" category. Extract of the comment
Maybe we could add an option to let people decide if they want to defer GA script ? |
@wardpeet What is your opinion on this? |
FYI: This PR is virtually the same as #22071 |
This PR has tests, so I think it makes sense to continue here. @Slashgear So let's follow the "new option" path as suggested in the other PR: Add a |
- add test - update doc
m=s.getElementsByTagName(o)[0];a.async=1;${ | ||
pluginOptions.defer ? `a.defer=1;` : `` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that having both defer
and async
is a gray area and browsers may treat those in a different way. Here is a quote from spec:
There are three possible modes that can be selected using these attributes. If the async attribute is present, then the script will be executed asynchronously, as soon as it is available. If the async attribute is not present but the defer attribute is present, then the script is executed when the page has finished parsing.
The defer attribute may be specified even if the async attribute is specified, to cause legacy Web browsers that only support defer (and not async) to fall back to the defer behavior instead of the synchronous blocking behavior that is the default.
So I think we should prefer one over another but not both:
m=s.getElementsByTagName(o)[0];a.async=1;${ | |
pluginOptions.defer ? `a.defer=1;` : `` | |
m=s.getElementsByTagName(o)[0];${ | |
pluginOptions.defer ? `a.defer=1;` : `a.async=1;` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thank you for the PR! 💜
Holy buckets, @Slashgear — we just merged your PR to Gatsby! 💪💜 Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! |
* 'master' of github.com:gatsbyjs/gatsby: (64 commits) Fix recipe test problems (gatsbyjs#23347) create blog post announcing extended deadline for Virtual Gatsby Days… (gatsbyjs#23430) Correct CFP deadline date on Virtual Gatsby Days registration announcement (gatsbyjs#23432) fix(gatsby-recipes): Raise an error when an unknown resource is used (gatsbyjs#23428) feat(gatsby-recipes): While apply a step, show the time elapsed after 10 seconds (gatsbyjs#23362) markdownASTToHTMLAst isn't async (gatsbyjs#23427) Be more vocal about not doing type checking (gatsbyjs#23397) docs(gatsby-remark-images): mark `sizeByPixelDensity` as deprecated (gatsbyjs#23387) chore(all): Improve renovate (gatsbyjs#23411) chore(gatsby): count sift hits in telemetry (gatsbyjs#23416) chore(showcase): add GeneOS and COVID KPI (gatsbyjs#23405) feat(analytics): defer google analytics script (gatsbyjs#22806) docs: mention passing the .tsx file to createPage (gatsbyjs#23329) fix(www): tweak docsearch to init algolia when tabbed into (gatsbyjs#23040) chore(docs): Fix typo in url (gatsbyjs#23394) chore(gatsby-preset-gatsby-package): Remove tsconfig.json (gatsbyjs#23388) fix(gatsby-recipes): link to the raw gist of .estlintrc.js (gatsbyjs#23390) docs: Create gitlab-continuous-integration.md (gatsbyjs#23367) chore(doc): switch zeit now to Vercel Now for Gatsby deployment (gatsbyjs#23346) chore(showcase): add Resume on the Web (gatsbyjs#23371) ...
Description
In order to improve the rendering performance of a Gatsby application that uses Google Analytics, it is necessary to delay the instantiation of the tracking scripts.
As you can see on this screenshot, the thirdparty Google Analytics blocks the main thread of the application. Google recommends the use of
defer
for third parties in its official documentation.Documentation
The documentation is not affected by this PR but I wonder if this feature should not be opt-out by default. It is possible to do it by adding an option to this plugin.
Other plugins could take advantage of this in a second step.
I've added some unit tests to verify that the script is loaded in a delayed way.
Related Issues
No related issue.