[commerce-sdk-react] Implement remaining Shopper Baskets cache logic

[adamraya]

Description

The PR refactors Shopper Baskets cache logic following the same pattern established in the rest of the endpoints, and implements the cache for the Shopper Baskets mutations that were missing cache logic.

Types of Changes

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Documentation update
• Breaking change (could cause existing functionality to not work as expected)
• Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

• Removing a public function or component or prop
• Adding a required argument to a function
• Changing the data type of a function parameter or return value
• Adding a new peer dependency to package.json

Changes

• (change1)

How to Test-Drive This PR

• (step1)

Checklists

General

• Changes are covered by test cases
• CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

• There are no changes to UI

or...

• Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
• Changes comply with WCAG 2.0 guidelines levels A and AA
• Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

• Changes include a UI text update in the Retail React App (which requires translation)

[wjhsf]

I don't think we want to remove all baskets, right? Just the one that got deleted? So just invalidating the query should be sufficient. (Or, if you want to be fancy, you could do an update that removes the matching basket from the baskets array.)

[adamraya]

Makes sense to only invalidating the Shopper Customer basket.

I changed the remove to do a fuzzy match to remove all the cache entries containing the basketId in their queryKey path. That will remove from cache all the rest of the basket related endpoints like: '/payment-methods', '/price-books', etc.

[kevinxh]

I see a pattern where some updates are based on conditions. If I read this code correctly, this is "if customerId is defined, do the update..." The spread operator here is somewhat difficult to read, can we introduce a predicate property to make this a bit more readable?

cc @wjhsf

Suggested change

	...(customerId
	? [
	{
	queryKey: getCustomerBaskets.queryKey({...parameters, customerId}),
	updater: (oldData: CustomerBasketsResult | undefined) =>
	customerBasketsUpdater(parameters, response, oldData)
	}
	]
	: [])
	{
	predicate: () => !!customerId
	queryKey: getCustomerBaskets.queryKey({...parameters, customerId}),
	updater: (oldData: CustomerBasketsResult | undefined) =>
	customerBasketsUpdater(parameters, response, oldData)
	}

[wjhsf]

React query only has predicate for invalidate/remove. For update, there's just queryKey and updater?. We could introduce our own predicate logic (but probably not in this PR) and it would likely break TypeScript (the updater scope wouldn't know what the predicate is asserting, and wouldn't narrow types to eliminate undefined).

[kevinxh]

ok, got it. Let's not do it in this PR.

[wjhsf]

pwa-kit/packages/commerce-sdk-react/src/hooks/types.ts

Lines 182 to 184 in 00c402a

	// TODO: Change Shopper Baskets cache logic from using predicates to creating query filters
	// using the query key helpers, then this 'predicate' type can be removed, as can the one in
	// `CacheUpdateRemove`. The predicate helpers in `utils.ts` should also then be removed.

There's a TODO dependent on the result of this work that I just remembered was there. It's just removing support for the pattern we're moving away from, so you can choose whether to delete that in this PR or leave it as a follow up.

[alexvuong]

So we only update the customer/customerId/baskets cache, and not the basket/basketId anymore?

[adamraya]

The key that updates the basked cache is defined in the line above:

pwa-kit/packages/commerce-sdk-react/src/hooks/ShopperBaskets/cache.ts

Line 65 in 887efe7

{queryKey: getBasket.queryKey(parameters)},

[alexvuong]

should update come before invalidations? Does order matter?

[adamraya]

The order in which we define the cache updates here doesn't matter, since we're working with an object where the concept of order really doesn't exist.

The updateCache method defines in which order the cache updates are executed:

pwa-kit/packages/commerce-sdk-react/src/hooks/utils.ts

Lines 18 to 29 in 5c60483

	export const updateCache = (queryClient: QueryClient, cacheUpdates: CacheUpdate, data: unknown) => {
	cacheUpdates.invalidate?.forEach((invalidate) => {
	queryClient.invalidateQueries(invalidate)
	})
	cacheUpdates.remove?.forEach((remove) => {
	queryClient.removeQueries(remove)
	})
	cacheUpdates.update?.forEach(({queryKey, updater}) =>
	// If an updater isn't given, fall back to just setting the data
	queryClient.setQueryData(queryKey, updater ?? data)
	)
	}

[wjhsf]

The order doesn't matter, it's just merging different properties into the result object (which will always look like {update: [], invalidate: [], remove: []}).

[wjhsf]

Suggested change

export const and =

This one is also a predicate helper that can be removed! (Unless it's actually being used somewhere, I guess.)

[adamraya]

It's still used by ShopperCustomer cache.

pwa-kit/packages/commerce-sdk-react/src/hooks/ShopperCustomers/cache.ts

Line 122 in 246d484

const predicate = and(pathStartsWith(path), isNotGetCustomer)

[wjhsf]

This will only invalidate getPriceBooksForBasket if invalidations.invalidate exists, i.e. only if customerId is defined. It doesn't seem the price books endpoint relies on customerId, so should there be a fallback for invalidations?

[adamraya]

You're right, I updated the invalidation logic.

[wjhsf]

👍

[wjhsf]

Seems like there's a bunch of methods that have this implementation; can they be extracted to a helper?

[adamraya]

Yep, I was thinking the same. I extracted the code to updateCustomerBasketsQuery helper method.