-
Notifications
You must be signed in to change notification settings - Fork 163
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
MWPW-158071: Optimize LCP loading times #2914
base: stage
Are you sure you want to change the base?
Conversation
@@ -731,6 +731,8 @@ function decorateDefaults(el) { | |||
} | |||
|
|||
function decorateHeader() { | |||
const breadcrumbs = document.querySelector('.breadcrumbs'); |
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 noticed that if a breadcrumbs is authored as first block, we immediately load it. The global-navigation should take care of this and on L752 we move the breadcrumbs into the header if (breadcrumbs) header.append(breadcrumbs);
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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2914 +/- ##
========================================
Coverage ? 96.24%
========================================
Files ? 236
Lines ? 54265
Branches ? 0
========================================
Hits ? 52228
Misses ? 2037
Partials ? 0 ☔ View full report in Codecov by Sentry. |
nala tests also failing on stage: #2910 |
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. Are there any unit tests for utils that require updating?
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
Description
It's relatively hard to track performance beforehand, however we observed a +500MS (13%) gain on CC pages speeding up loading placeholders. We've also seen good success with preloading
Decorate.js
This PR initially introduced the idea of preloading
decorate.js
for a performance gain, but introduced too much flexibility. The idea of preloadingdecorate.js
for specifically the two common (hero-)marquees blocks is valuable and worth the added 3 lines of code, as neither is likely to change anytime soon.Impressions
This should provide good benefits on a wide arrange of blocks, consumers and pages by optimizing the load order and chains, however we will want to observe the CRUM/RUM/CRUX data after merging the changes in this PR.
Given the success we seen with preloading & parallelizing work in general, this harmonizes the timeline and ensures the browser loads everything as fast as it can, without any impact on TBT/CLS.
Before
After
Resolves: MWPW-158071
Test URLs:
Bacom
CC