Bump Spree SDK and Spree Next package to 0.19.0 - #90
Conversation
* rename Metafields to CustomFields * move Next server actions to storefront directly for better DX
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRefactors server-side data layer from individual Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ServerCode as Storefront Data Layer
participant SpreeClient as getClient() / `@spree/sdk`
participant CookieHelpers as `@spree/next` Token/Cookie Helpers
participant Cache as next/cache
Browser->>ServerCode: POST /login {email,password}
ServerCode->>SpreeClient: auth.login({email,password})
SpreeClient-->>ServerCode: {access_token, refresh_token, user}
ServerCode->>CookieHelpers: setAccessToken(access_token)
ServerCode->>CookieHelpers: setRefreshToken(refresh_token)
ServerCode->>SpreeClient: carts.associate(cartId, { token, spreeToken }) (optional)
alt association success
SpreeClient-->>ServerCode: updatedCart
ServerCode->>Cache: updateTag("cart")
else association failure
ServerCode->>CookieHelpers: clearCartCookies()
end
ServerCode->>Cache: updateTag("customer")
ServerCode-->>Browser: { success, user }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/data/checkout.ts (1)
90-134:⚠️ Potential issue | 🟡 MinorInconsistent error handling:
requireCartId()called outside try-catch.Unlike
updateOrderAddresses,updateOrderMarket, and other functions that callrequireCartId()insideactionResult(),applyCodecalls it at lines 91-92 before any try-catch. IfrequireCartId()throws (e.g., redirect or custom error when no cart exists), the exception will propagate unhandled instead of returning{ success: false, error: ... }.Consider wrapping the entire function body in a try-catch for consistency:
🛡️ Suggested fix
export async function applyCode(cartId: string, code: string) { + try { const options = await getCartOptions(); const id = await requireCartId(); // Try discount code first (more common) try { const cart = await getClient().carts.discountCodes.apply(id, code, options); // ... rest of function } catch (discountError) { // ... existing catch logic } + } catch (error) { + return { success: false, error: error instanceof Error ? error.message : "Failed to apply code" } as const; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/checkout.ts` around lines 90 - 134, The function applyCode currently calls requireCartId() outside any try/catch so a thrown redirect/custom error escapes; wrap the call (and the rest of the function body) in a top-level try/catch or move requireCartId() inside the existing try so any exception from requireCartId is caught and returned as { success: false, error: errorMessage(err) }; ensure you still call getCartOptions(), getClient().carts.* and updateTag("checkout"/"cart") as before and reference applyCode, requireCartId, and errorMessage when locating and updating the code.
🧹 Nitpick comments (5)
src/lib/data/cached.ts (1)
2-3: Consider using absolute imports for consistency.The coding guidelines prefer
@/alias for imports. However, relative imports within the same module (e.g.,./categories) are acceptable for co-located files.Optional: Use absolute imports
-import { getCategory } from "./categories"; -import { getProduct } from "./products"; +import { getCategory } from "@/lib/data/categories"; +import { getProduct } from "@/lib/data/products";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/cached.ts` around lines 2 - 3, The imports in cached.ts use relative paths; update them to the project's preferred absolute alias by replacing "./categories" and "./products" with the corresponding "@/..." module paths (e.g., reference the modules where getCategory and getProduct are exported using the '@/...' alias) so getCategory and getProduct are imported via absolute imports for consistency with project guidelines.src/lib/data/cart.ts (1)
83-98: Consider usinggetCartOptions()for consistency.
addToCartmanually constructs{ spreeToken, token }by callinggetCartToken()andgetAccessToken()separately (lines 86-87), whileupdateCartItemandremoveCartItemusegetCartOptions(). For consistency and to reduce duplication, consider usinggetCartOptions()here as well.♻️ Suggested refactor
export async function addToCart(variantId: string, quantity: number) { return actionResult(async () => { const cart = await getOrCreateCart(); - const spreeToken = await getCartToken(); - const token = await getAccessToken(); + const options = await getCartOptions(); const updatedCart = await getClient().carts.items.create( cart.id, { variant_id: variantId, quantity }, - { spreeToken, token }, + options, ); updateTag("cart"); return { cart: updatedCart }; }, "Failed to add item to cart"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/cart.ts` around lines 83 - 98, The addToCart function currently calls getCartToken() and getAccessToken() directly and builds { spreeToken, token } for getClient().carts.items.create; replace those separate calls with a single call to getCartOptions() and pass its return value as the options argument to getClient().carts.items.create (remove the explicit getCartToken/getAccessToken calls), keeping the rest of addToCart (including updateTag("cart") and returning { cart: updatedCart }) intact so it matches updateCartItem/removeCartItem usage.src/lib/data/__tests__/cart.test.ts (1)
180-199: Test coverage reduced for error scenarios.The previous test suite included error-handling tests for
clearCartandassociateCartWithUser(including non-Error throw fallback cases). These are now removed, leaving only success-path coverage.While the implementation's error paths are unlikely to be triggered in practice (e.g.,
clearCartCookies()throwing a non-Error), consider adding at least one error test forassociateCartWithUserto verify the internal catch block that clears cookies oncarts.associatefailure.💡 Suggested test for associateCartWithUser error handling
it("clears cart cookies when associate fails", async () => { const { getAccessToken, clearCartCookies } = await import("@spree/next"); (getAccessToken as ReturnType<typeof vi.fn>).mockResolvedValue("jwt-token"); mockClient.carts.associate.mockRejectedValue(new Error("Already associated")); const result = await associateCartWithUser(); expect(clearCartCookies).toHaveBeenCalled(); expect(result).toEqual({ success: true }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/__tests__/cart.test.ts` around lines 180 - 199, Add a test for associateCartWithUser that simulates carts.associate throwing to exercise the catch block: import getAccessToken and clearCartCookies from "@spree/next", mock getAccessToken to resolve "jwt-token" and mock mockClient.carts.associate to reject (e.g., new Error("Already associated")), call associateCartWithUser(), then assert clearCartCookies was called and the function returns { success: true }; also consider a second test variant where clearCartCookies itself throws a non-Error value to ensure the fallback path is handled.src/lib/data/checkout.ts (1)
34-35: UnusedcartIdparameter in multiple functions.The
cartIdparameter is declared but never used inupdateOrderAddresses,updateOrderMarket,selectDeliveryRate,applyCode,removeDiscountCode, andremoveGiftCard. Instead, the cart ID is obtained viarequireCartId(). Consider removing the unused parameter or using it directly to avoid the extra async call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/checkout.ts` around lines 34 - 35, The listed functions (updateOrderAddresses, updateOrderMarket, selectDeliveryRate, applyCode, removeDiscountCode, removeGiftCard) declare a cartId parameter but never use it—currently they always call requireCartId(); fix by either removing the unused cartId parameter from each signature and callers, or (preferred) use the provided cartId when present and fall back to await requireCartId() when not; update the bodies of updateOrderAddresses, updateOrderMarket, selectDeliveryRate, applyCode, removeDiscountCode, and removeGiftCard to use a local const id = cartId ?? await requireCartId() (or equivalent), and adjust all call sites to pass a cartId if they have one or omit it if not.src/lib/data/customer.ts (1)
158-165: Add explicit return types to the remaining exported actions.
requestPasswordReset()andupdateCustomer()still rely on inference, while the rest of this module declares its server-action contract explicitly. In a PR that bumps the SDK surface, leaving these exports inferred makes accidental contract drift easier.As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions, use 'satisfies' for type checking object literals, and avoid 'any' (use 'unknown' instead)."
Also applies to: 191-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/customer.ts` around lines 158 - 165, Add explicit return types for the exported async actions requestPasswordReset and updateCustomer instead of relying on inference: determine the SDK response types returned by getClient().passwordResets.create and getClient().customers.update (or import the corresponding types from your SDK/type declarations) and annotate the functions with those Promise<...> return types (e.g., Promise<PasswordResetResponse> and Promise<CustomerUpdateResponse>) so the exported server-action contract is explicit; update the function signatures for requestPasswordReset and updateCustomer accordingly and import any required types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/account/GiftCardList.tsx`:
- Around line 142-148: The additional-info block in GiftCardList.tsx was made
conditional on card.redeemed_at, removing the "Added on" metadata for active
cards; update the JSX that renders the extra info so the container (the div with
classes "pt-4 border-t border-gray-100") is always rendered and always shows the
"Added on {formatDate(card.created_at)}" line, while keeping the "Fully redeemed
on {formatDate(card.redeemed_at)}" paragraph conditional on card.redeemed_at;
locate the block around the card rendering in GiftCardList and split the content
so formatDate(card.created_at) is always displayed and
formatDate(card.redeemed_at) is rendered only when card.redeemed_at is truthy.
In `@src/lib/data/customer.ts`:
- Around line 77-82: The catch blocks currently return arbitrary error.message
to the UI; replace that with a normalized message by creating a helper like
normalizeErrorMessage(error) that maps known auth/validation errors (e.g.,
"Invalid email or password", "User not found", token/validation messages or SDK
AuthError types) to user-friendly strings and returns a stable fallback such as
"Something went wrong. Please try again." for everything else; use this helper
in the return object (the { success: false, error: ... } returns shown) in all
three catch sites, and still log the original error (console/processLogger) for
debugging rather than exposing it to the UI.
- Around line 57-76: The post-authentication steps (merging guest cart and
tagging) are duplicated in login/register but missing from resetPassword;
extract the shared logic into a helper (e.g., finalizeAuthenticatedSession or
postAuthBootstrap) that performs setAccessToken/setRefreshToken handling,
associates guest cart via getCartToken/getCartId and getClient().carts.associate
(catching errors), calls updateTag("customer") and updateTag("cart"), and
returns the same { success: true, user: result.user } shape; then call that
helper from login, register, and resetPassword (and the other indicated blocks
at 108-127 and 178-182) so all sign-in flows run the same post-auth bootstrap.
- Around line 27-34: The catch block in getCustomer is currently clearing tokens
on any error, which logs out users for transient failures; update the error
handling in the getCustomer call that wraps withAuthRefresh so you only
clearAccessToken() and clearRefreshToken() when the error is an authentication
failure (e.g., inspect the thrown error for an auth-specific code/status like
401/403 or an AuthError type after calling
withAuthRefresh/getClient().customer.get) and rethrow or return null for other
errors so transient transport/backend failures are not treated as logout
triggers.
In `@src/lib/data/payment.ts`:
- Around line 39-44: The call to getClient().carts.paymentSessions.complete is
passing undefined as the third argument but the SDK requires a params object
with at least session_result; replace the undefined with a proper params object
(e.g., build a params variable containing session_result and optional
external_data) and pass that instead of undefined; locate the call to
paymentSessions.complete (using id, sessionId, options) and construct params
from available data (or default session_result to "success"/"failure" as
appropriate) before calling complete.
---
Outside diff comments:
In `@src/lib/data/checkout.ts`:
- Around line 90-134: The function applyCode currently calls requireCartId()
outside any try/catch so a thrown redirect/custom error escapes; wrap the call
(and the rest of the function body) in a top-level try/catch or move
requireCartId() inside the existing try so any exception from requireCartId is
caught and returned as { success: false, error: errorMessage(err) }; ensure you
still call getCartOptions(), getClient().carts.* and
updateTag("checkout"/"cart") as before and reference applyCode, requireCartId,
and errorMessage when locating and updating the code.
---
Nitpick comments:
In `@src/lib/data/__tests__/cart.test.ts`:
- Around line 180-199: Add a test for associateCartWithUser that simulates
carts.associate throwing to exercise the catch block: import getAccessToken and
clearCartCookies from "@spree/next", mock getAccessToken to resolve "jwt-token"
and mock mockClient.carts.associate to reject (e.g., new Error("Already
associated")), call associateCartWithUser(), then assert clearCartCookies was
called and the function returns { success: true }; also consider a second test
variant where clearCartCookies itself throws a non-Error value to ensure the
fallback path is handled.
In `@src/lib/data/cached.ts`:
- Around line 2-3: The imports in cached.ts use relative paths; update them to
the project's preferred absolute alias by replacing "./categories" and
"./products" with the corresponding "@/..." module paths (e.g., reference the
modules where getCategory and getProduct are exported using the '@/...' alias)
so getCategory and getProduct are imported via absolute imports for consistency
with project guidelines.
In `@src/lib/data/cart.ts`:
- Around line 83-98: The addToCart function currently calls getCartToken() and
getAccessToken() directly and builds { spreeToken, token } for
getClient().carts.items.create; replace those separate calls with a single call
to getCartOptions() and pass its return value as the options argument to
getClient().carts.items.create (remove the explicit getCartToken/getAccessToken
calls), keeping the rest of addToCart (including updateTag("cart") and returning
{ cart: updatedCart }) intact so it matches updateCartItem/removeCartItem usage.
In `@src/lib/data/checkout.ts`:
- Around line 34-35: The listed functions (updateOrderAddresses,
updateOrderMarket, selectDeliveryRate, applyCode, removeDiscountCode,
removeGiftCard) declare a cartId parameter but never use it—currently they
always call requireCartId(); fix by either removing the unused cartId parameter
from each signature and callers, or (preferred) use the provided cartId when
present and fall back to await requireCartId() when not; update the bodies of
updateOrderAddresses, updateOrderMarket, selectDeliveryRate, applyCode,
removeDiscountCode, and removeGiftCard to use a local const id = cartId ?? await
requireCartId() (or equivalent), and adjust all call sites to pass a cartId if
they have one or omit it if not.
In `@src/lib/data/customer.ts`:
- Around line 158-165: Add explicit return types for the exported async actions
requestPasswordReset and updateCustomer instead of relying on inference:
determine the SDK response types returned by getClient().passwordResets.create
and getClient().customers.update (or import the corresponding types from your
SDK/type declarations) and annotate the functions with those Promise<...> return
types (e.g., Promise<PasswordResetResponse> and Promise<CustomerUpdateResponse>)
so the exported server-action contract is explicit; update the function
signatures for requestPasswordReset and updateCustomer accordingly and import
any required types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ee0cddb-803c-462c-885c-78f2095059a0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
README.mdpackage.jsonsrc/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetails.tsxsrc/components/account/GiftCardList.tsxsrc/components/checkout/PaymentSection.tsxsrc/components/products/ProductCustomFields.tsxsrc/lib/data/__tests__/cart.test.tssrc/lib/data/__tests__/checkout.test.tssrc/lib/data/__tests__/customer.test.tssrc/lib/data/__tests__/payment.test.tssrc/lib/data/addresses.tssrc/lib/data/cached.tssrc/lib/data/cart.tssrc/lib/data/categories.tssrc/lib/data/checkout.tssrc/lib/data/cookies.tssrc/lib/data/countries.tssrc/lib/data/credit-cards.tssrc/lib/data/customer.tssrc/lib/data/gift-cards.tssrc/lib/data/markets.tssrc/lib/data/orders.tssrc/lib/data/payment.tssrc/lib/data/products.tssrc/lib/seo.ts
💤 Files with no reviewable changes (1)
- src/components/checkout/PaymentSection.tsx
| {card.redeemed_at && ( | ||
| <div className="pt-4 border-t border-gray-100"> | ||
| <p className="text-sm text-gray-500"> | ||
| Fully redeemed on {formatDate(card.redeemed_at)} | ||
| </p> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Restore “Added on” metadata for non-redeemed cards.
Lines 142-148 now hide the entire additional-info section unless redeemed_at exists, so non-redeemed cards lose useful timeline context.
💡 Suggested patch
- {card.redeemed_at && (
- <div className="pt-4 border-t border-gray-100">
- <p className="text-sm text-gray-500">
- Fully redeemed on {formatDate(card.redeemed_at)}
- </p>
- </div>
- )}
+ <div className="pt-4 border-t border-gray-100 space-y-1">
+ <p className="text-sm text-gray-500">
+ Added on {formatDate(card.created_at)}
+ </p>
+ {card.redeemed_at && (
+ <p className="text-sm text-gray-500">
+ Fully redeemed on {formatDate(card.redeemed_at)}
+ </p>
+ )}
+ </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/account/GiftCardList.tsx` around lines 142 - 148, The
additional-info block in GiftCardList.tsx was made conditional on
card.redeemed_at, removing the "Added on" metadata for active cards; update the
JSX that renders the extra info so the container (the div with classes "pt-4
border-t border-gray-100") is always rendered and always shows the "Added on
{formatDate(card.created_at)}" line, while keeping the "Fully redeemed on
{formatDate(card.redeemed_at)}" paragraph conditional on card.redeemed_at;
locate the block around the card rendering in GiftCardList and split the content
so formatDate(card.created_at) is always displayed and
formatDate(card.redeemed_at) is rendered only when card.redeemed_at is truthy.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/data/customer.ts (1)
92-97:⚠️ Potential issue | 🟡 MinorNormalize the auth errors before returning them to the UI.
These catch blocks still forward arbitrary
error.messagevalues. Network/SDK/backend failures will show up verbatim on account screens; map known auth/validation cases and fall back to a stable generic message for everything else.Also applies to: 125-129, 181-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/customer.ts` around lines 92 - 97, The catch blocks in src/lib/data/customer.ts that currently return error instanceof Error ? error.message : "Invalid email or password" should be replaced with a normalized error flow: add a helper function normalizeAuthError(error) that maps known error messages/codes (e.g., invalid_credentials, user_not_found, weak_password, network/error codes from your auth SDK) to user-safe strings and returns a stable generic fallback like "Unable to authenticate. Please try again." Then update the three catch sites (the catch in the sign-in, sign-up and password-reset flows at the shown locations) to call normalizeAuthError(error) and return { success: false, error: normalizeAuthError(error) } instead of exposing raw error.message.
🧹 Nitpick comments (1)
src/lib/data/customer.ts (1)
25-45: Add explicit return types to these server helpers/actions.
finalizeAuth,requestPasswordReset, andupdateCustomerall rely on inference right now. This file is the SDK boundary, so explicit return contracts make upgrade drift visible at compile time instead of leaking through callers. As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions, use 'satisfies' for type checking object literals, and avoid 'any' (use 'unknown' instead)."Also applies to: 155-163, 189-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/customer.ts` around lines 25 - 45, Annotate the async helper functions with explicit return types instead of relying on inference: declare finalizeAuth(): Promise<void>, and determine and add precise Promise<...> return types for requestPasswordReset and updateCustomer that match their actual returned payloads; update any other server helper/action signatures in this file to have explicit return types as well, and replace any use of implicit any in catches/returns with unknown (and narrow/throw as needed) so the SDK boundary has strict, visible contracts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/data/customer.ts`:
- Around line 32-40: The catch block for getClient().carts.associate(cartId, {
token, spreeToken: cartToken }) currently swallows errors and leaves the stale
guest cart cookie; update the catch to clear the guest cart cookies the same way
as src/lib/data/cart.ts (lines 93-112) does so the old cartToken is removed on
association failure. Specifically, in the catch for carts.associate reference
and call the cookie-clearing routine used in cart.ts (or import/implement a
clearGuestCartCookies/clearCartCookies helper) to remove cartToken/cartId
cookies instead of silently ignoring the error.
- Around line 194-198: The updateCustomer() flow calls withAuthRefresh() inside
actionResult() but doesn't clear stale auth cookies on authorization failures;
modify the actionResult callback around getClient().customer.update(...) so it
catches rejections from withAuthRefresh(), detects auth failures (HTTP 401/403
or the specific auth error type returned by withAuthRefresh()), clears the
terminal auth cookies/tokens (e.g., invoke the existing
clearAuthTokens/clearAuthCookies helper or delete the same cookies set during
sign-in), then rethrow or return the error so actionResult can surface the
failure; keep the updateTag("customer") behavior after successful update.
---
Duplicate comments:
In `@src/lib/data/customer.ts`:
- Around line 92-97: The catch blocks in src/lib/data/customer.ts that currently
return error instanceof Error ? error.message : "Invalid email or password"
should be replaced with a normalized error flow: add a helper function
normalizeAuthError(error) that maps known error messages/codes (e.g.,
invalid_credentials, user_not_found, weak_password, network/error codes from
your auth SDK) to user-safe strings and returns a stable generic fallback like
"Unable to authenticate. Please try again." Then update the three catch sites
(the catch in the sign-in, sign-up and password-reset flows at the shown
locations) to call normalizeAuthError(error) and return { success: false, error:
normalizeAuthError(error) } instead of exposing raw error.message.
---
Nitpick comments:
In `@src/lib/data/customer.ts`:
- Around line 25-45: Annotate the async helper functions with explicit return
types instead of relying on inference: declare finalizeAuth(): Promise<void>,
and determine and add precise Promise<...> return types for requestPasswordReset
and updateCustomer that match their actual returned payloads; update any other
server helper/action signatures in this file to have explicit return types as
well, and replace any use of implicit any in catches/returns with unknown (and
narrow/throw as needed) so the SDK boundary has strict, visible contracts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c9003bc-11ba-4082-864d-31c4014bd090
📒 Files selected for processing (2)
src/lib/data/__tests__/customer.test.tssrc/lib/data/customer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/data/tests/customer.test.ts
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
Tests