This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Preload Mini Cart inner blocks frontend scripts #8653
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Aljullu
added
status: needs review
type: enhancement
woocommercebot
requested review from
a team and
roykho
and removed request for
a team
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: 0 B Total Size: 1.12 MB ℹ️ View Unchanged
|
|
From the original issue:
It seems we want to lazy load the inner blocks script for the mini cart right? This way it is not loading scripts initially when we don't actually need it? So I am confused here. |
|
@roykho good point! I left an explanatory comment in #7176 (comment), but you are right, in some way this PR is not aligned with the issue title/description. I don't think we can reduce the number of dependencies loaded by the Mini Cart block (I expand this in the comment I linked), however, there are some performance improvements we can do, and one of them is to preload inner block frontend scripts so they are ready when the user interacts with the Mini Cart. Otherwise, they would need to be downloaded later, blocking the Mini Cart render. The total number of requests shouldn't change, but the different is that the scripts would be downloaded when the browser is idle, instead of being downloaded once the user has already interacted with the Mini Cart. Does it make sense? |
roykho
suggested changes
Mar 8, 2023
src/BlockTypes/MiniCart.php
Outdated
| @@ -183,6 +185,25 @@ protected function enqueue_data( array $attributes = [] ) { | |||
| 'translations' => $this->get_inner_blocks_translations(), | |||
| ); | |||
|
|
|||
| // Preload inner blocks frontend scripts. | |||
| $inner_blocks_frontend_scripts = $cart->is_empty() ? array( | |||
There was a problem hiding this comment.
$cart can potentially be null. So you might get a fatal in certain situations such as going to the theme->editor that has the mini cart block.
There was a problem hiding this comment.
Oh, good point. I refactored this in 9d08e0d. I also noticed that we were calling CartController, but if I'm not wrong that was only intended to be used in Rest API calls, so I replaced those calls with calls to the WC_Cart instance.
In some of them I didn't add a check for $cart not being null because they are executed after this check. But maybe it would be better to be extra-cautious and make sure $cart is not null? What do you think?
Aljullu
force-pushed
the
fix/mini-cart-preload-inner-blocks-frontend-scripts
branch
from
f389cc2 to
3e8caca
Compare
|
Thanks for the review, @roykho!
Good catch! The reason was that even if the cart is empty, we are rendering the Filled Mini Cart View and viceversa. I added both of them to the array of scripts to preload in 3e8caca. |
roykho
approved these changes
Mar 9, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #7176.
To improve Mini Cart block performance, we can preload the scripts that will be needed to render the inner blocks. Currently, we are preloading many other scripts, so we have the logic in place, this PR is simply adding the inner blocks scripts to the list of blocks to lazy load.
Testing
User Facing Testing
Testing this PR is about making sure there are no regressions in the Mini Cart block:
empty-cart-frontendorfilled-cart-frontend).WooCommerce Visibility
Changelog