-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Woo Store: Remove LabelSettings and dependencies #50491
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~323 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~12067 bytes removed 📉 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~549 bytes removed 📉 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
Thanks for pinging me on this @sirbrillig -- I'll take a look at this today. |
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'm not comfortable reviewing the WC-related functionality, but I did check that none of this code is used elsewhere. There may be some still-used stuff that can be moved elsewhere after this cleanup but that can happen later.
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 did not exhaustively verify that the remove components are not used anywhere, but I did verify that you can no longer access the page in the WooCommerce extension that was using LabelSettings. We'd have some issues if we were to disable the feature flag removing Store, but there are no plans to do that and we are starting the work of removing that feature flag so that there is no way to re-enable Store.
So, I think that is is okay to merge this PR.
Nice cleanup!
3b9bbc2
to
6178d6c
Compare
Changes proposed in this Pull Request
This removes
LabelSettings
from the Woo Store code (which has been disabled).By so doing, it also allows us to remove
AddCardDialog
, which allows removingPaymentMethodForm
,CreditCardFormFields
, andPaymentCountrySelect
as none of these are used anywhere else.PaymentCountrySelect
is the last place to use the code fromlib/cart/actions
which will also allow us to remove theCartStore
in the near future (see #24019).If there's some desire to have label settings continue to work on the Woo Store, we could put a link from there to the wp-admin version, but I'm not sure it's necessary.
Fixes #48421
Fixes #48966
Related to #49433
Testing instructions
LabelSettings
is being removed from cannot be accessed. This is at Store > Settings > Shipping as described here.