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

Can we lazy load below the fold? #6669

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

andrewHEguardian
Copy link
Contributor

@andrewHEguardian andrewHEguardian commented Jan 2, 2025

What are you doing in this PR?

Putting one of the lower down sections in the checkout (the payment method selector) as a lazy loading import, to try improve the performance of the page

Trello Card

How to test

Uploaded to CODE and put the checkout page through https://pagespeed.web.dev/analysis

Have we considered potential risks?

People seeing "Loading..." (or similar)

Screenshots

Before
image

After
image

Results

Improved overall performance - lower total blocking time and better speed index. First and largest contentful paints remained the same, which makes sense because most of the loading of the page is unchanged

I repeated the test again, and the results got worse. Running page speed insights just once may not be a good enough indicator - however in general they look somewhat better

EDIT:
I think we can't put any stock into individual page speed score results eg I got these results running it back to back
image
image

Copy link
Contributor

github-actions bot commented Jan 2, 2025

Size Change: +1.51 kB (+0.07%)

Total Size: 2.23 MB

Filename Size Change
./public/compiled-assets/javascripts/[countryGroupId]/router.js 264 kB -835 B (-0.32%)
./public/compiled-assets/webpack/170.js 1.11 kB +1.11 kB (new file) 🆕
./public/compiled-assets/webpack/582.js 1.19 kB +1.19 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size Change
./public/compiled-assets/javascripts/[countryGroupId]/events/router.js 90.1 kB 0 B
./public/compiled-assets/javascripts/ausMomentMap.js 108 kB 0 B
./public/compiled-assets/javascripts/contributionsRedirectStyles.js 20 B 0 B
./public/compiled-assets/javascripts/digitalSubscriptionLandingPage.js 235 kB 0 B
./public/compiled-assets/javascripts/downForMaintenancePage.js 70.4 kB +4 B (+0.01%)
./public/compiled-assets/javascripts/error404Page.js 70.4 kB +6 B (+0.01%)
./public/compiled-assets/javascripts/error500Page.js 70.3 kB +6 B (+0.01%)
./public/compiled-assets/javascripts/favicons.js 617 B 0 B
./public/compiled-assets/javascripts/paperSubscriptionCheckoutPage.js 174 kB +3 B (0%)
./public/compiled-assets/javascripts/paperSubscriptionLandingPage.js 87.2 kB +3 B (0%)
./public/compiled-assets/javascripts/payPalErrorPage.js 68.6 kB +4 B (+0.01%)
./public/compiled-assets/javascripts/payPalErrorPageStyles.js 20 B 0 B
./public/compiled-assets/javascripts/promotionTerms.js 73.3 kB +2 B (0%)
./public/compiled-assets/javascripts/subscriptionsLandingPage.js 72.6 kB 0 B
./public/compiled-assets/javascripts/subscriptionsRedemptionPage.js 127 kB +1 B (0%)
./public/compiled-assets/javascripts/supporterPlusLandingPage.js 307 kB 0 B
./public/compiled-assets/javascripts/unsupportedBrowserStyles.js 20 B 0 B
./public/compiled-assets/javascripts/weeklySubscriptionCheckoutPage.js 171 kB +6 B (0%)
./public/compiled-assets/javascripts/weeklySubscriptionLandingPage.js 87 kB -1 B (0%)
./public/compiled-assets/webpack/136.js 2.17 kB +1 B (+0.05%)
./public/compiled-assets/webpack/187.js 21.4 kB +4 B (+0.02%)
./public/compiled-assets/webpack/344.js 2.01 kB 0 B
./public/compiled-assets/webpack/643.js 22.4 kB 0 B
./public/compiled-assets/webpack/706.js 107 kB 0 B

compressed-size-action

import { retryPaymentStatus } from './retryPaymentStatus';
import { setThankYouOrder, unsetThankYouOrder } from './thankYouComponent';

const PaymentSection = lazy(() => import('./paymentSection'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a comment here on why lazy loading used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, if we decided to go with it

Copy link
Contributor

@paul-daniel-dempsey paul-daniel-dempsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice to pull these componets out of the checkout regardless of lazy loading. Great job. Really good example of how to do it, had to check what <suspense> was for, good use of that too. Like this. Cron/Smoke tests also passed.

@GHaberis
Copy link
Contributor

GHaberis commented Jan 7, 2025

Interesting that the reported results vary so much between runs on the identical build. Have you looked into alternative ways of measuring page speed performance such as Chrome DevTools' Lighthouse panel, Lighthouse CLI or WebPageTest to see if these offer more consistent results over multiple runs?

If there's variability could we run several tests for both the baseline and the build with updates, then calculate the average and standard deviation to account for variability?

@GHaberis
Copy link
Contributor

GHaberis commented Jan 7, 2025

Also interested to know if an external factor impacts page speed metrics, eg. I'm thinking the Sourcepoint CMP. Could we we disable the CMP when profiling the page to see whether it produces more consistent results?

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