fix: surface userErrors and warnings from cart mutations in hydrogen-react#3679
fix: surface userErrors and warnings from cart mutations in hydrogen-react#3679umxr wants to merge 4 commits intoShopify:mainfrom
Conversation
…react Cart mutations in hydrogen-react were silently discarding userErrors and warnings returned by the Storefront API. This made it impossible for merchants to display validation errors or warnings to customers. Closes Shopify#3378
Verifies that: - userErrors from mutation responses are surfaced via useCart() - warnings from mutation responses are surfaced via useCart() - userErrors and warnings are cleared on subsequent successful mutations
| cartLineAdd: cartLineAddSpy, | ||
| }); | ||
|
|
||
| void act(() => { |
There was a problem hiding this comment.
blocking: all three new tests are missing "before" assertions for userErrors and warnings.
The tests assert the final state after the mutation, but don't verify the initial state before. A "before" assertion confirms the mutation is what causes the state change, not some pre-existing value.
let's add assertions before each void act() call:
// Before linesAdd
expect(result.current.userErrors).toBeUndefined();
expect(result.current.warnings).toBeUndefined();
void act(() => {
result.current.linesAdd([{merchandiseId: '123'}]);
});Same applies to the "clears userErrors and warnings" test - the initial state before the first mutation should be verified too.
| cart { | ||
| ...CartFragment | ||
| } | ||
| userErrors { |
There was a problem hiding this comment.
non-blocking: the userErrors and warnings field selections are copy-pasted into all 8 mutations (80 lines of duplication).
The hydrogen package avoids this with GraphQL fragments (USER_ERROR_FRAGMENT, CART_WARNING_FRAGMENT in packages/hydrogen/src/cart/queries/cart-fragments.ts). If the Storefront API ever adds a field to CartUserError or CartWarning, all 8 mutations would need updating in lockstep - classic change amplification.
A simple extraction would eliminate this:
const CART_MUTATION_FIELDS = `
userErrors { code field message }
warnings { code message target }
`;Then reference via ${CART_MUTATION_FIELDS} in each mutation. The existing hydrogen-react queries don't use fragments so this is consistent with the current pattern, but the duplication is substantial enough to warrant extraction.
| >, | ||
| ): CartMachineFetchResultEvent { | ||
| if (errors) { | ||
| return {type: 'ERROR', payload: {errors, cartActionEvent}}; |
There was a problem hiding this comment.
non-blocking: when GraphQL-level errors exist, userErrors and warnings from the same response are silently discarded here.
The CartMachineFetchResultEvent ERROR case doesn't include these fields in its type either. If a mutation response contains both GraphQL errors and userErrors, the userErrors are lost.
This follows the pre-existing pattern for how errors takes priority over everything else, so it's consistent. But worth noting as a known behavioural limitation - a Storefront API response can technically contain both.
| cartActionEvent: CartMachineActionEvent, | ||
| cart?: PartialDeep<CartType, {recurseIntoArrays: true}> | null, | ||
| errors?: unknown, | ||
| userErrors?: Array< |
There was a problem hiding this comment.
non-blocking: eventFromFetchResult now takes 5 positional params with a type asymmetry - these last two accept Array<PartialDeep<CartUserError> | undefined> and filter internally to CartUserError[]. The PartialDeep wrapper from the generic fetchCart return type leaks into the function interface.
The internal narrowing is correct (pull complexity downward), but the interface exposes an implementation detail of the data pipeline. Not worth changing in this PR since it follows the existing pattern, but something to keep in mind for future work.
| prevCart: (context) => context?.lastValidCart, | ||
| cart: (context) => context?.lastValidCart, | ||
| errors: (_, event) => event?.payload?.errors, | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars |
There was a problem hiding this comment.
nit: these eslint-disable-next-line comments follow the existing pattern in the file (same comments exist for errors and rawCartResult clearing). Pre-existing pattern issue, not introduced by this PR.
|
|
||
| type CartResponse = PartialDeep<CartType, {recurseIntoArrays: true}>; | ||
|
|
||
| type CartMutationResponse = { |
There was a problem hiding this comment.
praise: nice - the introduction of CartMutationResponse centralises the mutation response shape, makes it impossible to forget userErrors or warnings in a new mutation, and reduces cognitive load across the 8 callbacks.
| cart: cartFromGraphQL(cart), | ||
| rawCartResult: cart, | ||
| cartActionEvent, | ||
| userErrors: userErrors?.filter((e): e is CartUserError => e != null), |
There was a problem hiding this comment.
praise: good use of CartUserError and CartWarning from storefront-api-types rather than defining custom types. The type guard filter here is a proper narrowing pattern - nice.
WHY are these changes introduced?
Fixes #3378
All cart mutations in
hydrogen-reactsilently discarduserErrorsandwarningsreturned by the Storefront API. The mutation queries only select thecartobject from the response payload, meaning validation errors (e.g., invalid quantity) and warnings (e.g., discount not applicable) are lost. This makes it impossible for merchants to provide meaningful feedback to customers when a cart operation partially fails or produces warnings.The
hydrogenpackage already handles this correctly — each mutation query includesuserErrorsandwarningsfields and surfaces them throughformatAPIResult. This PR bringshydrogen-reactin line with that behavior.WHAT is this pull request doing?
cart-queries.ts— AddsuserErrors { code field message }andwarnings { code message target }to all 8 cart mutation queries (cartCreate,cartLinesAdd,cartLinesUpdate,cartLinesRemove,cartNoteUpdate,cartBuyerIdentityUpdate,cartAttributesUpdate,cartDiscountCodesUpdate)cart-types.ts— AddsuserErrorsandwarningstoCartMachineContext,CartWithActions, and theRESOLVEevent payloaduseCartActions.tsx— Updates fetch type generics to includeuserErrorsandwarningsin mutation response typesuseCartAPIStateMachine.tsx— ExtractsuserErrors/warningsfrom each mutation response, passes them through the state machine, and clears them on error/completion transitionsCartProvider.tsx— ExposesuserErrorsandwarningson theuseCart()return valueAfter this change, merchants can access errors and warnings like so:
HOW to test your changes?
pnpm installcd packages/hydrogen-react && npx vitest run src/CartProvider.test.tsxsurfaces userErrors from the mutation responsesurfaces warnings from the mutation responseclears userErrors and warnings on subsequent successful mutationnpx tsc --noEmit --project packages/hydrogen-react/tsconfig.json— no new errorsChecklist