This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 219
Remove wc-settings from Mini Cart block dependencies #8703
Merged
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
skip-changelog
PRs that you don't want to appear in the changelog.
focus: performance
The issue/PR is related to performance.
block: mini-cart
Issues related to the Mini-Cart block.
labels
Mar 9, 2023
Aljullu
removed
the
skip-changelog
PRs that you don't want to appear in the changelog.
label
Mar 9, 2023
woocommercebot
requested review from
a team and
albarin
and removed request for
a team
March 9, 2023 15:18
The release ZIP for this PR is accessible via:
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
assets/js/blocks/mini-cart/frontend.ts
|
Size Change: -30 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
albarin
approved these changes
Mar 10, 2023
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.
👌
Aljullu
force-pushed
the
fix/mini-cart-settings-dependency
branch
from
March 10, 2023 11:20
97d12ef
to
e44c54e
Compare
4 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
block: mini-cart
Issues related to the Mini-Cart block.
focus: performance
The issue/PR is related to performance.
type: enhancement
The issue is a request for an enhancement.
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.
Until now, the Mini Cart block was using
wcSettings
to pass the scripts to lazy load from backend to frontend. That had the issue that we could not lazy-loadwc-settings
and all its dependencies. This PR changes the approach and instead of usingwcSettings
, we directly set a JS variable namedwcBlocksMiniCartFrontendDependencies
.One side effect from not having
wc-settings
as a dependency is thatwcSettings
variable would be undefined in the frontend. That's because we are only printing it ifwc-settings
is enqueued (see this code). That was an issue, because we needwcSettings
to exist when the user interacts with the Mini Cart! To solve it, I used a work-around to enqueuewc-settings
beforeAssetDataRegistry:enqueue_asset_data()
runs, and dequeue it afterwards.Testing
User Facing Testing
This PR doesn't add any new feature, so testing mostly refers to smoke testing that there are no regressions.
WooCommerce Visibility
Performance Impact
This PR reduces the number of scripts that the Mini Cart block depends on, so it should have a positive impact on stores using it.
Changelog