-
Notifications
You must be signed in to change notification settings - Fork 380
feat(cart): add cartGiftCardCodesAdd mutation #3401
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
254bc3e to
f0d7fc3
Compare
.changeset/gift-card-add-mutation.md
Outdated
| ## Verified API Behavior (E2E tested 2026-01-21) | ||
|
|
||
| | Scenario | Behavior | | ||
| |----------|----------| | ||
| | Valid gift card code | Applied successfully | | ||
| | UPPERCASE code | Works (API is case-insensitive) | | ||
| | Duplicate code in same call | Idempotent - applied once, no error | | ||
| | Re-applying already applied code | Idempotent - no error, no duplicate | | ||
| | Multiple different codes | All applied successfully | | ||
| | Invalid code | Silently rejected (no error surfaced) | | ||
| | Code with whitespace | Rejected (API does not trim whitespace) | | ||
| | Empty input | Graceful no-op | | ||
|
|
||
| **Key finding:** The API handles duplicate gift card codes gracefully - submitting an already-applied code results in silent success (idempotent behavior), not an error. No `DUPLICATE_GIFT_CARD` error code exists. | ||
|
|
||
| **Note on whitespace:** The API does NOT trim whitespace from codes. Ensure codes are trimmed before submission if accepting user input. |
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.
Do we think this would be useful information for people building with Hydrogen? I figured it wouldn't hurt but I could go either way on this
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.
yeah i think it’s good to note
i’d just change the wording on "Key Finding" into "Note", as these aren’t findings, they are the ways we (Shopify) have designed the API
| type CartGiftCardCodesAddRequire = { | ||
| action: 'GiftCardCodesAdd'; | ||
| inputs: { | ||
| giftCardCodes: string[]; | ||
| } & OtherFormData; | ||
| }; |
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.
for the future we can probably reduce these types by half with a util like
type ActionWithRequiredInputs<T extends {inputs?: unknown}> = T & {inputs: T['inputs']};
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.
Yeah this is something I'd love to do in the future! Added it to the list. I think that's out of scope for this PR (and the overall 2025-10 API change) though
| | CartCreateRequire | ||
| | CartDiscountCodesUpdateRequire | ||
| | CartGiftCardCodesUpdateRequire | ||
| | CartGiftCardCodesAddRequire | ||
| | CartGiftCardCodesRemoveRequire | ||
| | CartLinesAddRequire | ||
| | CartLinesUpdateRequire |
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.
[Re: lines +309 to +327]
did not test LSP for performance benchmarks BUT
we can delete all the ___Require types and replace this type with
type WithRequiredInputsDistributive<T extends {inputs?: unknown}> = T extends unknown ? T & {inputs: NonNullable<T['inputs']>} : never;
export type CartActionInput = WithRequiredInputsDistributive<CartActionInputProps>
is this something that we want? i could open a separate PR for that.
Pros: fewer manual types to write, fewer mistakes, less maintenance
Cons: LSP may be a liltle bit slower
"no" is a perfectly good answer to this
See this comment inline on Graphite.
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.
| GiftCardCodesAdd: 'GiftCardCodesAdd', | ||
| GiftCardCodesRemove: 'GiftCardCodesRemove', | ||
| LinesAdd: 'LinesAdd', | ||
| LinesRemove: 'LinesRemove', |
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.
[Re: line +376]
} as const satisfies Record<CartActionInput['action'], CartActionInput['action']>;
See this comment inline on Graphite.
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.
adding this has revealed that we may have a typo in MetaFieldDelete (it’s actually MetaFieldsDelete
unless these are unrelated to CartActionInput
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.
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.
yeah but this has to agree with the cart actions not the graphql API, no?
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.
Okay I looked into this a bit more and it looks like for this one it's a typo in terms of it being inconsistent with the other entries, but nothing seems to actually be broken. Wouldn't hurt to fix if this means no breaking changes for consumers, but that's out of scope for this PR
fredericoo
left a comment
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.
besides my type nitpicks and the typo found, it all works great and code is gud
f0d7fc3 to
cbf41a0
Compare
cbf41a0 to
c24fd25
Compare
c24fd25 to
9268ed7
Compare
04f9469 to
bce6ffc
Compare
9268ed7 to
79d73b7
Compare
bce6ffc to
6b7fd54
Compare
6b7fd54 to
b79b6fc
Compare
Adds the new `cartGiftCardCodesAdd` mutation available in Storefront API 2025-10.
## What
- Add `cartGiftCardCodesAddDefault` query implementation
- Add `cart.addGiftCardCodes(codes)` method to cart handler
- Add `CartForm.ACTIONS.GiftCardCodesAdd` form action
- Add types, tests, docs, and examples
- Update skeleton template to use new Add action
- Remove UpdateGiftCardForm from skeleton (Add + Remove covers all use cases)
## Why
The Storefront API 2025-10 introduces `cartGiftCardCodesAdd` which appends gift card
codes to the cart without replacing existing ones. This simplifies the common use case
of adding a single gift card code - previously you had to fetch existing codes, merge
them with the new code, then call `cartGiftCardCodesUpdate`.
## Usage
```typescript
// Via cart handler
await cart.addGiftCardCodes(['SUMMER2025']);
// Via CartForm
<CartForm action={CartForm.ACTIONS.GiftCardCodesAdd}>
<input name="giftCardCode" />
<button>Apply</button>
</CartForm>
```
Related: #3271
79d73b7 to
403c1f5
Compare


WHY are these changes introduced?
Closes https://github.com/Shopify/developer-tools-team/issues/964
The Storefront API 2025-10 introduces
cartGiftCardCodesAddwhich appends gift card codes to the cart without replacing existing ones. This simplifies the common use case of adding a single gift card code.Before (2025-07): You had to fetch existing codes, merge them with the new code, then call
cartGiftCardCodesUpdate:After (2025-10): Just add the new code:
WHAT is this pull request doing?
cartGiftCardCodesAddDefaultquery implementationcart.addGiftCardCodes(codes)method to cart handlerCartForm.ACTIONS.GiftCardCodesAddform actionUpdateGiftCardFormfrom skeleton (Add + Remove covers all use cases)HOW to test your changes?
Note
I think it makes more sense to only fix the recipes once we've finished merging all PRs into the stack, so we can just fix them once at the end. I started up a discussion here in case anyone has different opinions :)
Checklist