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

Update key assets with preload headers #2261

Merged
merged 1 commit into from Oct 27, 2022

Conversation

bookernath
Copy link
Contributor

@bookernath bookernath commented Sep 12, 2022

What?

Update the key render-blocking resources in base.html to be tagged as preloadable; this will result in preload headers being sent for these resources.

Furthermore, when the Stencil storefront is behind Cloudflare, this will result in HTTP 103 Early Hints being sent when appropriate, which can result in significant performance improvements for some clients.

Limiting this to only the most important, render-blocking resources for right now, as there is probably a point of diminishing returns or even nullifying the benefit if too many resources are tagged.

Outside of the theme, some Scripts in the Script Manager will also be preloaded.

Requirements

  • CHANGELOG.md entry added (required for code changes only)

Tickets / Documentation

Proof

Here's a WebPageTest result showing the effect of Early Hints (resources start downloading before HTML is delivered):

https://www.webpagetest.org/result/220912_BiDcW5_D8J/1/details/#waterfall_view_step1

@Tiggerito
Copy link
Contributor

This is great.

In the webpage test, I see the fonts CSS is loaded twice. An encoding issue makes the preload not match the actual CSS URL. This also means the file is still render-blocking and causing an extra delay on all resources found after it.

https://fonts.googleapis.com/css?family=Karla:400%7CMontserrat:400,500,700&display=block
https://fonts.googleapis.com/css?family=Karla:400|Montserrat:400,500,700&display=block

I'd also test the idea of preloading the font js bundle. With that, the browser may start to preload the web fonts if it has the time. I'm not 100% on if the current setup will preload before it sees the fonts in use.

The final big win I see is in the preload of the LCP image. The ability to add image preloads would make that a possible option. In your test, it would potentially cut 250ms off the LCP value.

@bookernath
Copy link
Contributor Author

bookernath commented Sep 13, 2022

Thanks for your feedback @Tiggerito!

In the webpage test, I see the fonts CSS is loaded twice. An encoding issue makes the preload not match the actual CSS URL. This also means the file is still render-blocking and causing an extra delay on all resources found after it.

Nice catch. Looking into it!

I'd also test the idea of preloading the font js bundle. With that, the browser may start to preload the web fonts if it has the time. I'm not 100% on if the current setup will preload before it sees the fonts in use.

I've made this update, it seems like a sensible change in any case.

The final big win I see is in the preload of the LCP image. The ability to add image preloads would make that a possible option. In your test, it would potentially cut 250ms off the LCP value.

I think this is a good idea, and it may add some benefit. In practice, though, we already have a pretty good image loading strategy between responsive images + LQIP + the progressive loading nature of JPGs (regardless of whether or not we get "credit" for this in the LCP metric). In fact, because we use responsive images, we don't actually know which version of the image to preload, as this decision is made much later by the browser!

I also wonder if the carousel will always be above-the-fold content, it may not be on all storefront designs all the time, especially with Widgets in the mix for example. Or to phrase it another way, I wanted to put more thought into understanding when an image can benefit from preloading vs when that will be a waste of bandwidth, and I kept it out of this PR because I didn't have what felt like a perfect answer. Interested in feedback on this.

@Tiggerito
Copy link
Contributor

Good point on the responsive images. Currently, HTML preload can deal with them, but HTTP headers can't.

However, many images in a theme end up being none responsive. Or responsive with one size. So could be HTTP preloaded.

Maybe it makes sense to let HTML preloads optionally generate the HTTP preload. It could block those with imgsrcset. That would be flexible. And/or let img tags optionally generate the HTTP preload as long as they do not have a srcset or data-srcset. It will not work 100% as I often see srcset lists with a fixed 'sizes', meaning only one image will ever be chosen.

Obviously more complex, so for another day.

At the moment, I manually add image preloads and often have to fix lazyloading of prominent images. Our Core Web Vitals repost tells us which images are most often the LCP making them a strong candidate for preloading.

I did some testing of things like LQIP and preloads:

https://bigcommerce.websiteadvantage.com.au/tag-rocket/articles/improving-image-loading-without-javascript/

@bookernath
Copy link
Contributor Author

Thanks, I'll take a look at this!

I should mention that as part of this update we've made updates to several handlebars helpers (documentation coming soon) which will allow you to tag your own assets for resourcing hinting in custom themes, based on your own understanding of what's important on a particular page in your own theme. Cornerstone, naturally, needs to think very generically and "one-size-fits-all".

So if you're already putting images into preload <link> tags in the page, you can consider moving those to {{cdn}} helpers using the resourceHint argument to emit them as HTTP headers/Early Hints.

@Tiggerito
Copy link
Contributor

I just spotted some preload header issues on a client's store. Error in the Chrome console:

<link rel=preload> must have a valid `as` value

I checked the headers and they look like this:

<https://www.domain.com/script.js?external-type=bigcommerce>; rel=preload; type=application/javascript

When I think they should be:

<https://www.domain.com/script.js?external-type=bigcommerce>; rel=preload; as=script

@Tiggerito
Copy link
Contributor

While I'm here. Here's another header:

link: <https://fonts.gstatic.com>; rel=dns-prefetch, <https://fonts.googleapis.com>; rel=dns-prefetch, <https://cdn11.bigcommerce.com/s-qfy5foqczc>; rel=dns-prefetch

A DNS prefetch is at the domain level. So I don't think including s-qfy5foqczc has any value.

Any reason why dns-prefetch is used and not preconnect?

@rafa-avila-bc
Copy link

rafa-avila-bc commented Sep 21, 2022

I just spotted some preload header issues on a client's store. Error in the Chrome console:

must have a valid `as` value

I checked the headers and they look like this:

<https://www.domain.com/script.js?external-type=bigcommerce>; rel=preload; type=application/javascript

This behavior and Chrome's warning also happen in the https://cfearlyhintstest.cc/ test store.

image

However, the following might not be completely right.

When I think they should be:

<https://www.domain.com/script.js?external-type=bigcommerce>; rel=preload; as=script

Although MDN Link header docs state that "the header is semantically equivalent to the HTML link tag", the Link header standard does not specify an as parameter. As of right now Link headers generation is limited by the standard.

CC @Tiggerito

@Tiggerito
Copy link
Contributor

@bookernath
Copy link
Contributor Author

We've fixed the encoding issue which was causing URLs to not match and cause a cache miss.

We're fixing the as value next to fix Chrome's errors.

@bookernath bookernath force-pushed the preload branch 2 times, most recently from 004c2aa to d02e18c Compare October 21, 2022 17:40
@bookernath
Copy link
Contributor Author

All the known issues are fixed; results on WPT mobile test are looking great:

https://www.webpagetest.org/result/221026_AiDcKK_19X5/

@bigcommerce/themes-team this is ready for final review; please merge if it looks good.

@BC-krasnoshapka BC-krasnoshapka merged commit f70fcd5 into bigcommerce:master Oct 27, 2022
@bookernath bookernath deleted the preload branch October 27, 2022 16:23
@Tiggerito
Copy link
Contributor

I've been tracking the effect of early hints on some of the stores using our Tag Rocket app. That is, I'm assuming early hints was enabled around Sept 23rd when we saw some big TTFB improvements. 103 early hint triggers the TTFB moment.

image

As only Chrome actually supports early hints, most of these improved TTFB scores have no effect on the page's performance. This will improve as more browsers support it.

I'm yet to see early hints showing an effect on FCP or LCP.

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

4 participants