-
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
Purchase Management: Replace getUserPurchases with React Hook useUserPurchases #96367
base: trunk
Are you sure you want to change the base?
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~805 bytes added 📈 [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 (~110 bytes added 📈 [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. |
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
I've pushed a commit to try to allow sharing the user and site-level data. Next I'll try adding single purchase data to that. |
This was added in #95176 as a way to populate the Redux store so it could be used in the call to `PaymentMethodDeleteDialog` in `PaymentMethodDelete`. Now that `PaymentMethodDelete` uses the React Hook, we no longer need the Redux data populated here.
This replaces `hasLoadedUserPurchasesFromServer`.
It's possible that other subcomponents that have not yet been migrated might depend on the Redux store being populated.
The Component explicitly loads user purchases now so there's no need to wait for site purchases also (and that would require a query component).
We only need to display loading screens on initial fetch.
Since it is now using user purchases, we do not need to also fetch the site purchases. Also, userPurchases will always be truthy now (`[]` is truthy) so `sitePurchases` would never have been used anyway.
ba9d97f
to
7687e9b
Compare
I've pushed a commit now to deal with a single purchase ID (eg: for the https://wordpress.com/me/purchases/SITE-here/ID-here page), but AFAIK there's actually no endpoint for that currently; the current code actually fetches all the user or site's purchases if the specific purchase is not in our store. That's unnecessary and will be quite slow for users with lots of purchases, so I've invented a new endpoint, |
Proposed Changes
This PR creates a new React Hook using useQuery called
useUserPurchases()
which fetches all of a user's purchases and returns them. This takes the place of thegetUserPurchases()
Redux function and its associated state.This PR then uses the new Hook in the purchase management pages.
Note
This does not remove the
QueryUserPurchases
component in the purchase list because other sub-components of the page might still rely on the Redux store being populated.Note
There are a few other places that use
getUserPurchases()
within other parts of calypso but to keep this PR easy to review and test, I've limited it to only the main purchase list page. Follow-up PRs can migrate the other consumers and then eventually remove the Redux state itself.To do
The main issue with this approach currently is that the Redux store is a single place where three different mechanisms can populate the local cache with purchase data: fetching user purchases, fetching site purchases, and fetching a single purchase. By changing these components to use a hook, we no longer share that cache and instead will potentially have three separate caches that may all contain the same data.
hasLoadedUserPurchasesFromServer
inclient/me/purchases
that may be affected by this change if they also depended on the query component.client/me/purchases
uses the mainisDataLoading()
function.getByPurchaseId
inclient/me/purchases
.Why are these changes being made?
Using Redux is overkill for fetching a list of purchases and it limits these components to be used only within the main calypso package. useQuery (aka: React Query/Tanstack Query) is the main replacement for Redux currently used by calypso and this migration allows removing a lot of state from our Redux tree.
There is already a hook like this for site purchases (
useSitePurchases
added in #84761). This PR addsuseUserPurchases
alongside it in the@automattic/data-stores
package.Testing Instructions
/me/purchases
for an account with multiple purchases and verify that they show up and operate the same before and after this PR.