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

Introduce timeout for scripts such as Zendesk, Hubspot and defer GTM #2088

Closed
wants to merge 1 commit into from

Conversation

arslanashraf7
Copy link
Contributor

Pre-Flight checklist

  • Testing
    • Code is tested
    • Changes have been manually tested
  • Settings
    • New settings are documented and present in app.json
    • New settings have reasonable development defaults, if applicable

What are the relevant tickets?

#2065

What's this PR do?

  • It introduces a time interval of 3s on loading the third party scripts such as Zendesk, Hubspot.
  • Adds defer to the GTM/Analytics script.

Pros:
Introduction of time interval based lazy loading of above scripts gets the performance up by 1-2 points.
The page loading looks faster.
It gets rid of third-party code impact from problem pointed out by Lighthouse

Cons
While it give a better performance for sure but it will take a minimum of 3 seconds for Zendesk & Hubspot widgets to show up and being functional.

How should this be manually tested?

  • Generate a report through LightHouse
  • Check that we don't see third-party impact (Third-party scripts blocking the main thread)

Any background context you want to provide?

Our third party widgets/plugins used to load right away when the web page is requested hence blocking the main thread while executing their scripts. We saw the impact of this in Lighthouse's report as well.

@odlbot odlbot temporarily deployed to xpro-ci-pr-2088 January 26, 2021 13:58 Inactive
@arslanashraf7 arslanashraf7 force-pushed the arslan/await-third-party-scripts branch from ce7e246 to 60b77d3 Compare January 26, 2021 13:59
@codecov-io
Copy link

codecov-io commented Jan 26, 2021

Codecov Report

Merging #2088 (60b77d3) into master (c0379cc) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2088      +/-   ##
==========================================
- Coverage   88.02%   87.99%   -0.03%     
==========================================
  Files         328      328              
  Lines       14936    14759     -177     
  Branches     1035     1030       -5     
==========================================
- Hits        13147    12987     -160     
+ Misses       1548     1533      -15     
+ Partials      241      239       -2     
Impacted Files Coverage Δ
cms/blocks.py 80.95% <0.00%> (-2.73%) ⬇️
static/js/containers/pages/CheckoutPage.js 82.22% <0.00%> (-2.10%) ⬇️
hubspot/tasks.py 92.20% <0.00%> (-0.39%) ⬇️
cms/factories.py 94.08% <0.00%> (-0.39%) ⬇️
users/admin.py 92.59% <0.00%> (-0.27%) ⬇️
hubspot/conftest.py 95.83% <0.00%> (-0.25%) ⬇️
hubspot/api.py 85.11% <0.00%> (-0.09%) ⬇️
static/js/lib/auth.js 100.00% <0.00%> (ø)
authentication/urls.py 100.00% <0.00%> (ø)
authentication/utils.py 100.00% <0.00%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0379cc...f210449. Read the comment docs.

@arslanashraf7
Copy link
Contributor Author

@gsidebo, I've created this Draft PR which is an attempt to make third-party scripts to load lazily. I've introduced a time interval of 3 seconds before the scripts start loading giving more time to our own scripts that take part in first paint.
These changes majorly impact Zendesk & Hubspot which take the most time in all third-party scripts.

Other options that we had to load the scrips lazily to use the IntersectionObservers to load the scripts when a specific element enters the viewport but we might have to anchor it on some element and it might not be a good idea for Zendesk as the help button of Zendesk overlays the whole page, If we use this approach we might not see zendesk button untill a specific item has entered viewport.

Other option for Zendesk script was to use their connectOnPageLoad setting as mentioned here. But it didn't work so well in improving the performance.

Please let me know what you think of these changes or suggest if we should move to some other approach here.

@arslanashraf7 arslanashraf7 force-pushed the arslan/await-third-party-scripts branch from 60b77d3 to f210449 Compare February 25, 2021 11:10
@arslanashraf7 arslanashraf7 marked this pull request as ready for review February 25, 2021 12:22
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.

3 participants