From 1cfa9e5677edc32a519bfcfcc948b528442bc4e0 Mon Sep 17 00:00:00 2001 From: andromia3 Date: Tue, 14 Apr 2026 16:00:19 +0100 Subject: [PATCH 1/2] fix(hydrogen): respect cartId arg in cartGetDefault (#3639) The returned function declares a `cartInput?: CartGetProps` parameter whose `cartId` field is documented as "The cart ID" with "@default cart.getCartId()". But the implementation was calling `getCartId()` unconditionally and then checking `if (!cartId) return null`, so when `getCartId()` returned undefined the function bailed out with null even though the caller had passed a valid cart id. The existing test "should return a cartId passed in" only covered the case where `getCartId()` returns a value. The spread of `...cartInput` into the GraphQL variables masked the bug in that test: the early return never fired. Added a regression test that sets `getCartId: () => undefined` and passes a `cartId` via the input. This test fails on the old code (returns null) and passes after the fix. --- .changeset/cart-get-respects-cartid.md | 5 +++++ .../hydrogen/src/cart/queries/cartGetDefault.test.ts | 11 +++++++++++ packages/hydrogen/src/cart/queries/cartGetDefault.ts | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 .changeset/cart-get-respects-cartid.md diff --git a/.changeset/cart-get-respects-cartid.md b/.changeset/cart-get-respects-cartid.md new file mode 100644 index 0000000000..441d52b972 --- /dev/null +++ b/.changeset/cart-get-respects-cartid.md @@ -0,0 +1,5 @@ +--- +"@shopify/hydrogen": patch +--- + +Fix `cartGetDefault` ignoring the `cartId` argument. Previously, when callers invoked the returned function with `{cartId: '...'}`, that value was ignored and the function called `getCartId()` instead — which could return `undefined` and cause an early `return null`, even though the caller had provided a valid cart id. The resolved cart id now prefers `cartInput.cartId` before falling back to `getCartId()`. diff --git a/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts b/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts index 1f2f2b4b98..d33b9b02e1 100644 --- a/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts +++ b/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts @@ -53,6 +53,17 @@ describe('cartGetDefault', () => { expect(result).toHaveProperty('id', 'gid://shopify/Cart/c1-456'); }); + it('should use the cartId passed in when getCartId returns undefined', async () => { + const cartGet = cartGetDefault({ + storefront: mockCreateStorefrontClient(), + getCartId: () => undefined, + }); + + const result = await cartGet({cartId: 'gid://shopify/Cart/c1-456'}); + + expect(result).toHaveProperty('id', 'gid://shopify/Cart/c1-456'); + }); + describe('run with customerAccount option', () => { it('should add logged_in search param to checkout link if customer is logged in', async () => { const cartGet = cartGetDefault({ diff --git a/packages/hydrogen/src/cart/queries/cartGetDefault.ts b/packages/hydrogen/src/cart/queries/cartGetDefault.ts index 71ddee722b..f207251924 100644 --- a/packages/hydrogen/src/cart/queries/cartGetDefault.ts +++ b/packages/hydrogen/src/cart/queries/cartGetDefault.ts @@ -63,7 +63,7 @@ export function cartGetDefault({ cartFragment, }: CartGetOptions): CartGetFunction { return async (cartInput?: CartGetProps) => { - const cartId = getCartId(); + const cartId = cartInput?.cartId ?? getCartId(); if (!cartId) return null; From e3a242d63d40e440e6e4ffc326b272016927bd93 Mon Sep 17 00:00:00 2001 From: andromia3 Date: Tue, 14 Apr 2026 16:11:24 +0100 Subject: [PATCH 2/2] fix(hydrogen): protect resolved cartId from explicit-undefined override Follow-up to the previous commit on this branch. On review, I traced the full path of the fix and found a second case the one-line change didn't cover: if a caller computes the cart id dynamically and passes it through as `{cartId: maybeUndefined}`, the old spread order `variables: {cartId, ...cartInput}` would overwrite the already-resolved local `cartId` with `undefined` and the query would go out with `cartId: undefined`. Swapped the spread order to `{...cartInput, cartId}` so the resolved local `cartId` always wins over whatever was passed in as the spread, while still picking up `country` / `language` / `numCartLines` / `visitorConsent` from cartInput as before. Added a regression test that pins this by calling `cartGet({cartId: undefined})` and asserting the cookie cart id is returned. Verified all four existing tests still pass by tracing execution against the mock storefront client, and checked that no caller in the repo (including `createCartHandler.test.ts:171`) relies on the old spread-order behaviour. --- .changeset/cart-get-respects-cartid.md | 2 +- .../hydrogen/src/cart/queries/cartGetDefault.test.ts | 11 +++++++++++ packages/hydrogen/src/cart/queries/cartGetDefault.ts | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.changeset/cart-get-respects-cartid.md b/.changeset/cart-get-respects-cartid.md index 441d52b972..7e425ac2eb 100644 --- a/.changeset/cart-get-respects-cartid.md +++ b/.changeset/cart-get-respects-cartid.md @@ -2,4 +2,4 @@ "@shopify/hydrogen": patch --- -Fix `cartGetDefault` ignoring the `cartId` argument. Previously, when callers invoked the returned function with `{cartId: '...'}`, that value was ignored and the function called `getCartId()` instead — which could return `undefined` and cause an early `return null`, even though the caller had provided a valid cart id. The resolved cart id now prefers `cartInput.cartId` before falling back to `getCartId()`. +Fix `cartGetDefault` not respecting the `cartId` argument. The returned function accepts an optional `cartInput.cartId`, but the implementation called `getCartId()` unconditionally and then early-returned `null` when that was falsy — so a caller passing `{cartId: '…'}` with no cart cookie got `null` back. The resolved cart id now prefers `cartInput.cartId` before falling back to `getCartId()`, and the GraphQL `variables` spread order was swapped so the resolved cart id always wins over an explicit `{cartId: undefined}` in the input. diff --git a/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts b/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts index d33b9b02e1..4d9795cea5 100644 --- a/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts +++ b/packages/hydrogen/src/cart/queries/cartGetDefault.test.ts @@ -64,6 +64,17 @@ describe('cartGetDefault', () => { expect(result).toHaveProperty('id', 'gid://shopify/Cart/c1-456'); }); + it('should fall back to getCartId when cartInput.cartId is explicitly undefined', async () => { + const cartGet = cartGetDefault({ + storefront: mockCreateStorefrontClient(), + getCartId: () => CART_ID, + }); + + const result = await cartGet({cartId: undefined}); + + expect(result).toHaveProperty('id', CART_ID); + }); + describe('run with customerAccount option', () => { it('should add logged_in search param to checkout link if customer is logged in', async () => { const cartGet = cartGetDefault({ diff --git a/packages/hydrogen/src/cart/queries/cartGetDefault.ts b/packages/hydrogen/src/cart/queries/cartGetDefault.ts index f207251924..12ca9f15c4 100644 --- a/packages/hydrogen/src/cart/queries/cartGetDefault.ts +++ b/packages/hydrogen/src/cart/queries/cartGetDefault.ts @@ -70,7 +70,7 @@ export function cartGetDefault({ const [isCustomerLoggedIn, {cart, errors}] = await Promise.all([ customerAccount ? customerAccount.isLoggedIn() : false, storefront.query<{cart: Cart | null}>(CART_QUERY(cartFragment), { - variables: {cartId, ...cartInput}, + variables: {...cartInput, cartId}, cache: storefront.CacheNone(), }), ]);