Feat: add checkout stripe payments - #22
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Stripe payment integration (client & server), refactors checkout flow into billing-update + payment-complete steps, adds an Order Placed page, new payment/session APIs, cart cookie constant and utilities, credit-card helpers, dependency updates, env example change, and broad UI styling/token updates. Changes
Sequence DiagramsequenceDiagram
participant User
participant PaymentStep
participant StripeForm as StripePaymentForm
participant Backend as ServerAPI
participant Stripe as StripeService
participant OrderPlaced
User->>PaymentStep: submit billing address
PaymentStep->>Backend: onUpdateBillingAddress(orderId, address)
Backend-->>PaymentStep: success / failure
alt billing update success
PaymentStep->>Backend: createCheckoutPaymentSession(orderId, paymentMethodId?)
Backend->>Stripe: request session
Stripe-->>Backend: clientSecret, sessionId
Backend-->>PaymentStep: { clientSecret, paymentSessionId }
PaymentStep->>StripeForm: init(clientSecret)
User->>StripeForm: enter card or choose saved card
User->>PaymentStep: click Pay Now
alt saved card
PaymentStep->>Backend: confirmWithSavedCard(clientSecret, paymentMethodId)
Backend-->>PaymentStep: confirmationResult
else new card
PaymentStep->>StripeForm: confirmPayment(returnUrl)
StripeForm->>Stripe: stripe.confirmPayment(...)
Stripe-->>StripeForm: result
StripeForm-->>PaymentStep: result
end
alt payment confirmed
PaymentStep->>Backend: completeCheckoutPaymentSession(orderId, paymentSessionId)
Backend->>Backend: completeCheckoutOrder(orderId)
Backend-->>PaymentStep: success
PaymentStep->>OrderPlaced: redirect to order-placed/{orderId}
else payment failed
PaymentStep->>User: show error
end
else billing update failed
PaymentStep->>User: show validation errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 338-375: handlePaymentComplete currently treats a null return from
getCheckoutOrder as success and proceeds to redirect; explicitly guard against
updatedOrder being null by treating it as an error case: after calling
getCheckoutOrder(order.id) check if updatedOrder is falsy and, if so, call
setError with a clear message (e.g., "Order not found after payment"), call
setProcessing(false), and return before attempting to call completeCheckoutOrder
or router.push; keep existing flows for updatedOrder.state !== "complete"
(calling completeCheckoutOrder and handling its .success) and preserve the
try/catch behavior around completeCheckoutPaymentSession, completeCheckoutOrder,
and router.push.
- Around line 318-336: handleUpdateBillingAddress currently calls
updateOrderAddresses without catching exceptions; wrap the await
updateOrderAddresses(...) call in a try/catch inside handleUpdateBillingAddress
so any thrown errors are caught, call setError with a specific message (e.g.,
"Failed to save billing address: " + err.message or the error string) and return
false on catch, and keep the existing updateResult.success handling for
non-throwing failures; reference handleUpdateBillingAddress and
updateOrderAddresses so you modify that function to prevent exceptions from
bubbling to PaymentStep.handleSubmit.
In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx:
- Around line 142-146: Replace the plain <img> in page.tsx with Next.js' Image
component: import Image from 'next/image' at the top, and swap the <img
src={item.thumbnail_url} alt={item.name} className="w-full h-full object-cover"
/> for an <Image> using src={item.thumbnail_url} and alt={item.name} and
appropriate sizing (either width/height props or the fill prop with parent
positioned container and className="object-cover"); also ensure any external
host serving item.thumbnail_url is added to next.config.js' images.domains so
the image can be optimized.
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 88-119: The useEffect watching stripePaymentMethod is being
retriggered due to unstable object identity from order.payment_methods?.find;
change the logic to depend on a stable scalar instead (e.g.,
stripePaymentMethod?.id) or memoize stripePaymentMethod with useMemo where you
compute it (the order.payment_methods?.find(...) call), so the effect that
creates the session (useEffect that checks clientSecret, setLoadingSession,
createCheckoutPaymentSession, setClientSecret, setPaymentSessionId,
setStripeError) only runs when the actual selected payment method ID changes
rather than on every order object reset.
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 12-14: Remove the non-null assertion and add a guard that checks
process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY before calling loadStripe: read
the value into a local const, if it's falsy throw or log a clear runtime error
(e.g., "Missing NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY") then call loadStripe(key)
to initialize stripePromise; reference stripePromise, loadStripe and
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY when making the change.
In `@src/contexts/CartContext.tsx`:
- Around line 118-123: The current useEffect with dependencies [refreshCart,
pathname] triggers refreshCart() on every route change (useEffect in
CartContext.tsx), causing excessive getCartAction() calls; narrow the re-fetch
by either (A) keeping the effect but adding a conditional that only calls
refreshCart() when pathname matches checkout-related routes (e.g., check
pathname startsWith '/checkout' or matches a checkout regex) or when navigating
away from checkout by tracking previousPath via a useRef and comparing
previousPath to pathname, or (B) remove the pathname-driven effect and instead
invoke refreshCart() explicitly from the checkout completion flow (where
checkout success is handled) so only post-checkout triggers the cart refresh;
update the effect or checkout handlers accordingly and keep refreshCart and
getCartAction references intact.
🧹 Nitpick comments (4)
🤖 Fix all nitpicks with AI agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx: - Around line 318-336: handleUpdateBillingAddress currently calls updateOrderAddresses without catching exceptions; wrap the await updateOrderAddresses(...) call in a try/catch inside handleUpdateBillingAddress so any thrown errors are caught, call setError with a specific message (e.g., "Failed to save billing address: " + err.message or the error string) and return false on catch, and keep the existing updateResult.success handling for non-throwing failures; reference handleUpdateBillingAddress and updateOrderAddresses so you modify that function to prevent exceptions from bubbling to PaymentStep.handleSubmit. In `@src/components/checkout/PaymentStep.tsx`: - Around line 88-119: The useEffect watching stripePaymentMethod is being retriggered due to unstable object identity from order.payment_methods?.find; change the logic to depend on a stable scalar instead (e.g., stripePaymentMethod?.id) or memoize stripePaymentMethod with useMemo where you compute it (the order.payment_methods?.find(...) call), so the effect that creates the session (useEffect that checks clientSecret, setLoadingSession, createCheckoutPaymentSession, setClientSecret, setPaymentSessionId, setStripeError) only runs when the actual selected payment method ID changes rather than on every order object reset. In `@src/components/checkout/StripePaymentForm.tsx`: - Around line 12-14: Remove the non-null assertion and add a guard that checks process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY before calling loadStripe: read the value into a local const, if it's falsy throw or log a clear runtime error (e.g., "Missing NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY") then call loadStripe(key) to initialize stripePromise; reference stripePromise, loadStripe and NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY when making the change. In `@src/contexts/CartContext.tsx`: - Around line 118-123: The current useEffect with dependencies [refreshCart, pathname] triggers refreshCart() on every route change (useEffect in CartContext.tsx), causing excessive getCartAction() calls; narrow the re-fetch by either (A) keeping the effect but adding a conditional that only calls refreshCart() when pathname matches checkout-related routes (e.g., check pathname startsWith '/checkout' or matches a checkout regex) or when navigating away from checkout by tracking previousPath via a useRef and comparing previousPath to pathname, or (B) remove the pathname-driven effect and instead invoke refreshCart() explicitly from the checkout completion flow (where checkout success is handled) so only post-checkout triggers the cart refresh; update the effect or checkout handlers accordingly and keep refreshCart and getCartAction references intact.src/contexts/CartContext.tsx (1)
118-123: Cart re-fetch on every navigation may be excessive.This fires
getCartAction()on every route change, not just post-checkout. On a typical storefront with frequent browsing, this adds an API call per navigation. Consider scoping the re-fetch more narrowly — e.g., only when navigating to/from checkout-related paths, or by having the checkout completion flow explicitly callrefreshCart()instead of relying on a pathname side-effect.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/contexts/CartContext.tsx` around lines 118 - 123, The current useEffect with dependencies [refreshCart, pathname] triggers refreshCart() on every route change (useEffect in CartContext.tsx), causing excessive getCartAction() calls; narrow the re-fetch by either (A) keeping the effect but adding a conditional that only calls refreshCart() when pathname matches checkout-related routes (e.g., check pathname startsWith '/checkout' or matches a checkout regex) or when navigating away from checkout by tracking previousPath via a useRef and comparing previousPath to pathname, or (B) remove the pathname-driven effect and instead invoke refreshCart() explicitly from the checkout completion flow (where checkout success is handled) so only post-checkout triggers the cart refresh; update the effect or checkout handlers accordingly and keep refreshCart and getCartAction references intact.src/components/checkout/StripePaymentForm.tsx (1)
12-14: Non-null assertion on environment variable could produce a confusing runtime error.If
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYis not configured,loadStripewill silently receiveundefined, leading to hard-to-debug failures. Consider adding a guard.Suggested guard
+const stripePublishableKey = process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY; + +if (!stripePublishableKey) { + console.error("Missing NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY environment variable"); +} + -const stripePromise = loadStripe( - process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY!, -); +const stripePromise = stripePublishableKey + ? loadStripe(stripePublishableKey) + : Promise.resolve(null);🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/components/checkout/StripePaymentForm.tsx` around lines 12 - 14, Remove the non-null assertion and add a guard that checks process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY before calling loadStripe: read the value into a local const, if it's falsy throw or log a clear runtime error (e.g., "Missing NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY") then call loadStripe(key) to initialize stripePromise; reference stripePromise, loadStripe and NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY when making the change.src/components/checkout/PaymentStep.tsx (1)
88-119:stripePaymentMethodin the dependency array is an unstable object reference.
order.payment_methods?.find(...)on line 74–76 produces a new object reference wheneverorderis re-set (e.g., after coupon apply/remove). While theclientSecretguard on line 90 prevents duplicate session creation in most cases, after a total change resetsclientSecrettonull(lines 79–85), the effect will correctly re-create the session. However, iforderis updated without a total change (e.g., an address update), this effect re-evaluates with a newstripePaymentMethodreference, butclientSecretis still set, so it exits early — no bug, just unnecessary effect scheduling.Consider memoizing
stripePaymentMethodor usingstripePaymentMethod?.idin the dependency array for clearer intent:Suggested optimization
- }, [stripePaymentMethod, order.id, clientSecret]); + }, [stripePaymentMethod?.id, order.id, clientSecret]);🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/components/checkout/PaymentStep.tsx` around lines 88 - 119, The useEffect watching stripePaymentMethod is being retriggered due to unstable object identity from order.payment_methods?.find; change the logic to depend on a stable scalar instead (e.g., stripePaymentMethod?.id) or memoize stripePaymentMethod with useMemo where you compute it (the order.payment_methods?.find(...) call), so the effect that creates the session (useEffect that checks clientSecret, setLoadingSession, createCheckoutPaymentSession, setClientSecret, setPaymentSessionId, setStripeError) only runs when the actual selected payment method ID changes rather than on every order object reset.src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (1)
318-336: Missing error handling for thrown exceptions inhandleUpdateBillingAddress.If
updateOrderAddressesthrows (e.g., network error), the exception propagates unhandled toPaymentStep.handleSubmit. WhilehandleSubmithas a catch block, the error message there is generic ("An error occurred during payment"), which may confuse users since the actual failure was an address update. Consider wrapping with try/catch for a more specific error message, or this is acceptable if the generic message is intentional.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx around lines 318 - 336, handleUpdateBillingAddress currently calls updateOrderAddresses without catching exceptions; wrap the await updateOrderAddresses(...) call in a try/catch inside handleUpdateBillingAddress so any thrown errors are caught, call setError with a specific message (e.g., "Failed to save billing address: " + err.message or the error string) and return false on catch, and keep the existing updateResult.success handling for non-throwing failures; reference handleUpdateBillingAddress and updateOrderAddresses so you modify that function to prevent exceptions from bubbling to PaymentStep.handleSubmit.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/components/checkout/StripePaymentForm.tsx (1)
62-66:useEffectcallingonReadyre-fires wheneverconfirmPaymentidentity changes.Since
confirmPaymentis recreated whenstripeorelementschange,onReadycan be invoked multiple times. This works today because the consumer stores the handle in a ref (overwrite is harmless), but it's a subtle contract. Consider documenting this expectation or using a ref internally to callonReadyonly once after Stripe loads.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/StripePaymentForm.tsx` around lines 62 - 66, The useEffect currently calls onReady whenever stripe or the derived confirmPayment identity changes, causing multiple invocations; change it to call onReady only once after Stripe has first loaded by introducing an internal ref (e.g., readyCalledRef) that tracks whether onReady has been invoked, then in the effect check if (stripe && !readyCalledRef.current) { readyCalledRef.current = true; onReady({ confirmPayment }); } and keep dependencies minimal (stripe and confirmPayment) so the handle is passed once; reference the useEffect, confirmPayment, onReady, stripe, and elements symbols when making this change.src/components/checkout/PaymentStep.tsx (2)
141-176: InitializationuseEffectfetches data and creates a payment session on mount.This works but is a pattern the coding guidelines discourage for data fetching. The saved cards could be fetched server-side and passed as a prop to avoid the client-side fetch. That said, since the payment session creation is inherently client-initiated and depends on the Stripe payment method from the order, keeping it here is pragmatic for now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentStep.tsx` around lines 141 - 176, The effect currently fetches saved cards client-side (getCreditCards) and then creates a Stripe session (createSession); update it to prefer server-provided saved cards passed in as a prop (e.g., savedCardsProp) instead of doing the client fetch inside the useEffect: in the useEffect (initRef, stripePaymentMethod) use savedCardsProp to call setSavedCards and determine initialCardId (use gateway_payment_profile_id and default logic to call setSelectedCardId) and only fall back to calling getCreditCards if savedCardsProp is missing; keep the client-only createSession(initialCardId) behavior and retain initRef and the dependencies (stripePaymentMethod, isAuthenticated, createSession) so the session creation remains client-initiated.
155-164: TypeScript won't narrowgateway_payment_profile_idthrough.filter().After filtering for truthy
gateway_payment_profile_id, the type isn't narrowed, sodefaultCard.gateway_payment_profile_idmay still be typed asstring | null | undefined. Consider using a type guard or non-null assertion after the filter to be explicit.Suggested narrowing
- const stripeCards = result.data.filter( - (card) => card.gateway_payment_profile_id, - ); + const stripeCards = result.data.filter( + (card): card is StoreCreditCard & { gateway_payment_profile_id: string } => + !!card.gateway_payment_profile_id, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentStep.tsx` around lines 155 - 164, The filtered array stripeCards (from result.data.filter) doesn't narrow gateway_payment_profile_id so defaultCard.gateway_payment_profile_id can still be nullable; update the filter to use a type guard that asserts the field is non-null (e.g., filter((c): c is { gateway_payment_profile_id: string; ... } => Boolean(c.gateway_payment_profile_id))) or otherwise narrow/mapping result.data to a typed structure before calling setSavedCards, then safely read gateway_payment_profile_id from defaultCard (or use a non-null assertion) when assigning initialCardId and calling setSelectedCardId to ensure the value is string only.src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (1)
318-336:handleUpdateBillingAddresslacks try/catch — exceptions propagate to the caller.If
updateOrderAddressesthrows (e.g., network error), the exception will bubble up toPaymentStep'shandleSubmitcatch block, which sets a generic Stripe error message. This means a billing address failure would show "An error occurred during payment" rather than a billing-specific message. Consider wrapping in try/catch for a more accurate error message, or accept the current behavior since the catch in PaymentStep does handle it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx around lines 318 - 336, The handleUpdateBillingAddress function calls updateOrderAddresses without a try/catch, so thrown exceptions bubble up to PaymentStep.handleSubmit and produce a generic Stripe error; wrap the await updateOrderAddresses(...) call in a try/catch inside handleUpdateBillingAddress, catch any error, call setError with a billing-specific message (include the caught error message for context), and return false on catch so the caller gets a clear billing-address failure instead of the generic payment error.src/lib/data/payment.ts (1)
10-24: Consider adding explicit return types for stronger type safety.The return types of all three functions are inferred, and the
session/orderobjects coming from the SDK are untyped. Adding explicit return types (or at least typing the SDK responses) would improve type safety and make the API contract clearer for consumers. As per coding guidelines, explicit return types should be defined andanyshould be avoided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/payment.ts` around lines 10 - 24, The function createCheckoutPaymentSession (and the other two related functions in this file) currently rely on inferred/untyped SDK responses; add explicit return types and typed SDK response types instead of any: import or declare a PaymentSession (and Order) interface matching the SDK shape, type the local variables (e.g., session returned from createPaymentSession) with that interface, and update the function signature to return a precise Promise type (e.g., Promise<...> rather than inferred) and, if actionResult is generic, pass the response type into actionResult so the result wrapper is typed; reference createCheckoutPaymentSession, createPaymentSession, actionResult and stripePaymentMethodId when making these changes.
🤖 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/checkout/PaymentStep.tsx`:
- Around line 473-477: The submit button's disabled prop in PaymentStep
currently allows the button when stripePaymentMethod is null because the
condition uses (!clientSecret && !!stripePaymentMethod); update the logic so the
button is disabled whenever there is no stripePaymentMethod or no clientSecret
or no paymentSessionId (matching the early bail in handleSubmit). Locate the
disabled expression in the PaymentStep component and change it to include a
check for !stripePaymentMethod (and optionally !paymentSessionId) so disabled
becomes true when any required payment artifact is missing, preventing clicks
that would immediately bail in handleSubmit.
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 116-123: The block handling stripe.confirmCardPayment in
StripePaymentForm.tsx is misformatted; reformat the await call and the
subsequent error branch to match project style by ensuring consistent spacing
and indentation around the await expression
(stripe.confirmCardPayment(clientSecret, { payment_method: paymentMethodId,
return_url: returnUrl })) and the if (result.error) return { error:
result.error.message || "An error occurred during payment." }; statement (no
stray blank lines, proper semicolons if used elsewhere, and consistent
brace/newline placement) so the CI formatter no longer flags lines referencing
result, clientSecret, paymentMethodId, and returnUrl.
- Around line 12-14: The code currently uses a non-null assertion when calling
loadStripe with NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY which yields a confusing
runtime error if the env var is missing; update the initialization of
stripePromise to first validate process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY
and if it is missing throw a clear error (e.g.,
"NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY is not set") or return null and surface that
to the StripePaymentForm component so it can render a helpful message; refer to
the stripePromise constant, loadStripe call, and the
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY env var to locate and change the code.
---
Duplicate comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 357-367: getCheckoutOrder can return null and the current code
silently skips completion; change the guard so a null updatedOrder is treated as
"not complete": replace the check with if (!updatedOrder || updatedOrder.state
!== "complete") and then call completeCheckoutOrder(order.id) as currently done;
on null or failed completion, setError with a descriptive message (include that
getCheckoutOrder returned null), setProcessing(false) and return so we do not
redirect to the placed page for an incomplete order; reference getCheckoutOrder,
updatedOrder, completeCheckoutOrder, setError, setProcessing.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 318-336: The handleUpdateBillingAddress function calls
updateOrderAddresses without a try/catch, so thrown exceptions bubble up to
PaymentStep.handleSubmit and produce a generic Stripe error; wrap the await
updateOrderAddresses(...) call in a try/catch inside handleUpdateBillingAddress,
catch any error, call setError with a billing-specific message (include the
caught error message for context), and return false on catch so the caller gets
a clear billing-address failure instead of the generic payment error.
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 141-176: The effect currently fetches saved cards client-side
(getCreditCards) and then creates a Stripe session (createSession); update it to
prefer server-provided saved cards passed in as a prop (e.g., savedCardsProp)
instead of doing the client fetch inside the useEffect: in the useEffect
(initRef, stripePaymentMethod) use savedCardsProp to call setSavedCards and
determine initialCardId (use gateway_payment_profile_id and default logic to
call setSelectedCardId) and only fall back to calling getCreditCards if
savedCardsProp is missing; keep the client-only createSession(initialCardId)
behavior and retain initRef and the dependencies (stripePaymentMethod,
isAuthenticated, createSession) so the session creation remains
client-initiated.
- Around line 155-164: The filtered array stripeCards (from result.data.filter)
doesn't narrow gateway_payment_profile_id so
defaultCard.gateway_payment_profile_id can still be nullable; update the filter
to use a type guard that asserts the field is non-null (e.g., filter((c): c is {
gateway_payment_profile_id: string; ... } =>
Boolean(c.gateway_payment_profile_id))) or otherwise narrow/mapping result.data
to a typed structure before calling setSavedCards, then safely read
gateway_payment_profile_id from defaultCard (or use a non-null assertion) when
assigning initialCardId and calling setSelectedCardId to ensure the value is
string only.
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 62-66: The useEffect currently calls onReady whenever stripe or
the derived confirmPayment identity changes, causing multiple invocations;
change it to call onReady only once after Stripe has first loaded by introducing
an internal ref (e.g., readyCalledRef) that tracks whether onReady has been
invoked, then in the effect check if (stripe && !readyCalledRef.current) {
readyCalledRef.current = true; onReady({ confirmPayment }); } and keep
dependencies minimal (stripe and confirmPayment) so the handle is passed once;
reference the useEffect, confirmPayment, onReady, stripe, and elements symbols
when making this change.
In `@src/lib/data/payment.ts`:
- Around line 10-24: The function createCheckoutPaymentSession (and the other
two related functions in this file) currently rely on inferred/untyped SDK
responses; add explicit return types and typed SDK response types instead of
any: import or declare a PaymentSession (and Order) interface matching the SDK
shape, type the local variables (e.g., session returned from
createPaymentSession) with that interface, and update the function signature to
return a precise Promise type (e.g., Promise<...> rather than inferred) and, if
actionResult is generic, pass the response type into actionResult so the result
wrapper is typed; reference createCheckoutPaymentSession, createPaymentSession,
actionResult and stripePaymentMethodId when making these changes.
Comment on lines
+12
to
+14
| const stripePromise = loadStripe( | ||
| process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY!, | ||
| ); |
Contributor
There was a problem hiding this comment.
Non-null assertion on env var will produce a confusing runtime error if unset.
If NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY is not configured, loadStripe receives undefined (cast via !), leading to a cryptic Stripe SDK error rather than a clear message.
Suggested guard
-const stripePromise = loadStripe(
- process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY!,
-);
+const stripePublishableKey = process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY;
+if (!stripePublishableKey) {
+ console.warn("NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY is not set. Stripe payments will not work.");
+}
+const stripePromise = stripePublishableKey ? loadStripe(stripePublishableKey) : Promise.resolve(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const stripePromise = loadStripe( | |
| process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY!, | |
| ); | |
| const stripePublishableKey = process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY; | |
| if (!stripePublishableKey) { | |
| console.warn("NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY is not set. Stripe payments will not work."); | |
| } | |
| const stripePromise = stripePublishableKey ? loadStripe(stripePublishableKey) : Promise.resolve(null); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/checkout/StripePaymentForm.tsx` around lines 12 - 14, The code
currently uses a non-null assertion when calling loadStripe with
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY which yields a confusing runtime error if the
env var is missing; update the initialization of stripePromise to first validate
process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY and if it is missing throw a
clear error (e.g., "NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY is not set") or return
null and surface that to the StripePaymentForm component so it can render a
helpful message; refer to the stripePromise constant, loadStripe call, and the
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY env var to locate and change the code.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[country]/[locale]/(storefront)/account/credit-cards/page.tsx (1)
19-24:⚠️ Potential issue | 🟡 Minor
setDeleting(false)is skipped ifonDeletethrows.If
deleteCreditCard(called viaonDelete) rejects, theawaiton line 22 throws, and line 23 is never reached — the button stays stuck on "Removing...".Suggested fix
const handleDelete = async () => { if (!confirm("Are you sure you want to remove this card?")) return; setDeleting(true); - await onDelete(); - setDeleting(false); + try { + await onDelete(); + } finally { + setDeleting(false); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/credit-cards/page.tsx around lines 19 - 24, handleDelete currently sets deleting true then awaits onDelete but never resets it if onDelete throws; wrap the await in try/finally (or try/catch/finally) so setDeleting(false) always runs. Update the handleDelete function to call setDeleting(true) before the try, await onDelete() inside the try, and call setDeleting(false) in the finally block; optionally handle or rethrow errors in the catch to preserve existing behavior while ensuring the "Removing..." state is cleared.
🧹 Nitpick comments (3)
src/components/checkout/PaymentStep.tsx (2)
223-224: Fragile URL construction via regex replacement.
window.location.pathname.replace(/\/checkout\/.*/, ...)assumes a specific URL structure. If the checkout route pattern changes, this silently produces a wrong redirect URL.Consider constructing the return URL from known route segments or a shared route helper instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentStep.tsx` around lines 223 - 224, The current returnUrl is built using a fragile regex replace on window.location.pathname (see returnUrl and the replace(/\/checkout\/.*/, ...) call in PaymentStep.tsx), which can break if routes change; instead construct the URL from known route segments or a shared route helper: derive the base origin via window.location.origin and combine it with a deterministic path (e.g., `/order-placed/${order.id}`) or use your app's route builder/utility to produce the checkout success path, replacing the regex-based logic in the returnUrl assignment with a call to that route helper or an explicit concatenation of known segments.
108-121:createSessionsilently swallows aresult.success && !secretscenario without surfacing the session object.When
result.successis true andresult.sessionexists butexternal_data?.client_secretis missing (line 110–112), the user sees a generic error. Consider logging the session response in development to aid debugging, since this would indicate an unexpected API response shape.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentStep.tsx` around lines 108 - 121, The handler for createSession currently treats a missing client_secret as a generic user error and discards the successful session object; update the branch where result.success && result.session but client_secret is missing to log or capture the full result.session (only in non-production/dev) for debugging and still set a helpful user-facing error via setStripeError. Specifically, inside the block checking secret (the result.session.external_data?.client_secret path), add a development-only console.log or call to your logging utility that records result.session and any external_data, and keep calling setStripeError("Failed to initialize payment. Please try again.") while preserving existing state functions setClientSecret and setPaymentSessionId behavior. Ensure logging is gated by NODE_ENV !== 'production' (or your appEnv) to avoid leaking sensitive session data.src/lib/utils/credit-card.ts (1)
3-42: Consider consolidating the duplicate key mappings into a single data structure.
CC_TYPE_MAP(lines 3–14) and thegetCardLabelswitch (lines 21–42) enumerate the exact same set of card-type keys. Adding a new type requires updating both in lockstep.A single lookup table could drive both functions:
♻️ Suggested refactor
-const CC_TYPE_MAP: Record<string, PaymentType> = { - visa: "Visa", - mastercard: "Mastercard", - master: "Mastercard", - american_express: "AmericanExpress", - amex: "AmericanExpress", - discover: "Discover", - jcb: "JCB", - diners_club: "DinersClub", - maestro: "Maestro", - unionpay: "UnionPay", -}; - -export function getCardIconType(ccType: string): PaymentType { - return CC_TYPE_MAP[ccType.toLowerCase()] ?? "Generic"; -} - -export function getCardLabel(ccType: string): string { - switch (ccType.toLowerCase()) { - case "visa": - return "Visa"; - case "mastercard": - case "master": - return "Mastercard"; - case "american_express": - case "amex": - return "Amex"; - case "discover": - return "Discover"; - case "jcb": - return "JCB"; - case "diners_club": - return "Diners Club"; - case "maestro": - return "Maestro"; - case "unionpay": - return "UnionPay"; - default: - return ccType || "Card"; - } -} +const CARD_TYPES: Record<string, { icon: PaymentType; label: string }> = { + visa: { icon: "Visa", label: "Visa" }, + mastercard: { icon: "Mastercard", label: "Mastercard" }, + master: { icon: "Mastercard", label: "Mastercard" }, + american_express: { icon: "AmericanExpress", label: "Amex" }, + amex: { icon: "AmericanExpress", label: "Amex" }, + discover: { icon: "Discover", label: "Discover" }, + jcb: { icon: "JCB", label: "JCB" }, + diners_club: { icon: "DinersClub", label: "Diners Club" }, + maestro: { icon: "Maestro", label: "Maestro" }, + unionpay: { icon: "UnionPay", label: "UnionPay" }, +}; + +export function getCardIconType(ccType: string): PaymentType { + return CARD_TYPES[ccType.toLowerCase()]?.icon ?? "Generic"; +} + +export function getCardLabel(ccType: string): string { + return CARD_TYPES[ccType.toLowerCase()]?.label ?? ccType || "Card"; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/credit-card.ts` around lines 3 - 42, The CC_TYPE_MAP and getCardLabel switch duplicate the same card-type keys; replace them with a single shared map (e.g., CARD_TYPE_INFO) that maps normalized keys (visa, mastercard, master, american_express, amex, etc.) to an object containing both the PaymentType icon value and the display label, then update getCardIconType to return CARD_TYPE_INFO[ccType.toLowerCase()]?.icon ?? "Generic" and update getCardLabel to return CARD_TYPE_INFO[ccType.toLowerCase()]?.label ?? (ccType || "Card"); keep aliases (master → Mastercard, amex → AmericanExpress) in the map so adding a new type only requires one change.
🤖 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/app/`[country]/[locale]/(storefront)/account/credit-cards/page.tsx:
- Around line 3-8: The imports at the top of page.tsx are not sorted and failing
CI; reorder the import statements to satisfy your project's
organizeImports/formatter rules (e.g., group external packages then local
modules, alphabetize within groups). Specifically, reorder the lines importing
StoreCreditCard, PaymentIcon, useEffect/useState, CreditCardIcon/LockIcon,
deleteCreditCard/getCreditCards, and getCardIconType/getCardLabel so they follow
the canonical import ordering for the project and then run your
formatter/organize-imports; the unique symbols to locate are StoreCreditCard,
PaymentIcon, useEffect, useState, CreditCardIcon, LockIcon, deleteCreditCard,
getCreditCards, getCardIconType, and getCardLabel.
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 194-198: handleCardSelect triggers createSession(cardId) without
awaiting/cancelling, allowing race conditions that can leave
clientSecret/paymentSessionId from a stale response; update handleCardSelect to
(a) set a local "requestedCardId" or use the existing selectedCardId and await
createSession(cardId) (or return early when loading) and disable the radio
inputs while loading, or (b) add a response guard inside createSession that
compares the response's cardId to the latest selectedCardId before calling
setClientSecret/setPaymentSessionId; reference functions/vars: handleCardSelect,
createSession, selectedCardId, setSelectedCardId, loading, clientSecret,
paymentSessionId.
- Around line 3-27: The CI failure is caused by unsorted imports in
PaymentStep.tsx; run your project's formatting/organizing tool (e.g., eslint
--fix, tsserver organizeImports, or the repo's Prettier/organizeImports command)
to reorder the import statements so they satisfy organizeImports. Specifically,
update the import block containing AddressParams/StoreState, PaymentIcon, React
hooks (useCallback/useEffect/...), CreditCardIcon, getCreditCards,
createCheckoutPaymentSession, the address helpers
(AddressFormData/addressesMatch/addressToFormData/formDataToAddress),
credit-card utils (getCardIconType/getCardLabel), AddressFormFields, and
StripePaymentForm/StripePaymentFormHandle/confirmWithSavedCard so they are
sorted per the repo rules, then commit the changed file.
---
Outside diff comments:
In `@src/app/`[country]/[locale]/(storefront)/account/credit-cards/page.tsx:
- Around line 19-24: handleDelete currently sets deleting true then awaits
onDelete but never resets it if onDelete throws; wrap the await in try/finally
(or try/catch/finally) so setDeleting(false) always runs. Update the
handleDelete function to call setDeleting(true) before the try, await onDelete()
inside the try, and call setDeleting(false) in the finally block; optionally
handle or rethrow errors in the catch to preserve existing behavior while
ensuring the "Removing..." state is cleared.
---
Duplicate comments:
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 456-462: The submit button's disabled condition only checks
(!clientSecret && !!stripePaymentMethod) and still allows submission when
required prerequisites like paymentSessionId or stripeHandleRef are missing;
update the disabled logic to mirror the early-return checks in handleSubmit (the
checks around paymentSessionId, stripePaymentMethod, selectedCardId, and
stripeHandleRef.current) so the button is disabled unless all of: clientSecret
is present, paymentSessionId exists (when needed), and either a selectedCardId
exists or stripeHandleRef.current is set for new-card flows; reference the
button rendering (disabled prop), handleSubmit, clientSecret,
stripePaymentMethod, paymentSessionId, selectedCardId, and stripeHandleRef to
implement the guard, and consider converting the Stripe “ready” flag to state if
you need fully reactive UI updates.
---
Nitpick comments:
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 223-224: The current returnUrl is built using a fragile regex
replace on window.location.pathname (see returnUrl and the
replace(/\/checkout\/.*/, ...) call in PaymentStep.tsx), which can break if
routes change; instead construct the URL from known route segments or a shared
route helper: derive the base origin via window.location.origin and combine it
with a deterministic path (e.g., `/order-placed/${order.id}`) or use your app's
route builder/utility to produce the checkout success path, replacing the
regex-based logic in the returnUrl assignment with a call to that route helper
or an explicit concatenation of known segments.
- Around line 108-121: The handler for createSession currently treats a missing
client_secret as a generic user error and discards the successful session
object; update the branch where result.success && result.session but
client_secret is missing to log or capture the full result.session (only in
non-production/dev) for debugging and still set a helpful user-facing error via
setStripeError. Specifically, inside the block checking secret (the
result.session.external_data?.client_secret path), add a development-only
console.log or call to your logging utility that records result.session and any
external_data, and keep calling setStripeError("Failed to initialize payment.
Please try again.") while preserving existing state functions setClientSecret
and setPaymentSessionId behavior. Ensure logging is gated by NODE_ENV !==
'production' (or your appEnv) to avoid leaking sensitive session data.
In `@src/lib/utils/credit-card.ts`:
- Around line 3-42: The CC_TYPE_MAP and getCardLabel switch duplicate the same
card-type keys; replace them with a single shared map (e.g., CARD_TYPE_INFO)
that maps normalized keys (visa, mastercard, master, american_express, amex,
etc.) to an object containing both the PaymentType icon value and the display
label, then update getCardIconType to return
CARD_TYPE_INFO[ccType.toLowerCase()]?.icon ?? "Generic" and update getCardLabel
to return CARD_TYPE_INFO[ccType.toLowerCase()]?.label ?? (ccType || "Card");
keep aliases (master → Mastercard, amex → AmericanExpress) in the map so adding
a new type only requires one change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/components/checkout/StripePaymentForm.tsx (1)
62-66:onReadyin the dependency array may cause unnecessary re-invocations if the parent doesn't memoize it.The
useEffectwill re-fire wheneveronReadychanges identity. CurrentlyPaymentStepwrapshandleGatewayReadyinuseCallback([], [])so this is stable, but the contract is fragile — any future caller that passes an unstable callback will trigger repeatedonReadycalls.Consider either documenting that
onReadymust be stable, or guarding against re-invocation internally (e.g., only callonReadyonce via a ref).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/StripePaymentForm.tsx` around lines 62 - 66, The useEffect in StripePaymentForm currently depends on onReady and will re-invoke whenever its identity changes; to avoid fragile contracts with callers, guard the invocation so onReady({ confirmPayment }) is only called once: add a ref like calledOnReadyRef checked inside the useEffect and set it after first call, keep the useEffect dependencies as [stripe, confirmPayment, onReady] (or drop onReady if you prefer) and reference the ref to prevent repeated calls; mention PaymentStep/handleGatewayReady as the expected stable caller but implement this internal once-only guard in useEffect to make the component robust to unstable parent callbacks.src/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsx (2)
46-79: DuplicategetCheckoutOrdercall whenpayment_intentis present.When handling a Stripe redirect,
getCheckoutOrder(orderId)is called at line 52 and again at line 62. The second call could reuse the result of the first (or the completed order) to save a network round-trip.Suggested optimization
async function loadAndComplete() { try { + let orderData: StoreOrder | null = null; + // Handle Stripe redirect return (3DS, etc.) const paymentIntent = searchParams.get("payment_intent"); if (paymentIntent) { - const orderData = await getCheckoutOrder(orderId); + orderData = await getCheckoutOrder(orderId); if (orderData && orderData.state !== "complete") { await completeCheckoutOrder(orderId); + // Re-fetch to get the completed state + orderData = await getCheckoutOrder(orderId); } } - // Load the order - const orderData = await getCheckoutOrder(orderId); + // Load the order if not already fetched + if (!orderData) { + orderData = await getCheckoutOrder(orderId); + } + if (!cancelled) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx around lines 46 - 79, The code calls getCheckoutOrder(orderId) twice in loadAndComplete when a payment_intent is present; change the flow to call getCheckoutOrder once, store its result in a local orderData variable, and reuse it: if paymentIntent is present, await getCheckoutOrder(orderId) into orderData, if orderData exists and orderData.state !== "complete" call completeCheckoutOrder(orderId) and then update orderData by using the response from completeCheckoutOrder if it returns the updated order, or only call getCheckoutOrder again once more to refresh state if completeCheckoutOrder returns no order; finally use that single orderData to setOrder/setError and set loading flags (preserve loadedRef and cancelled checks).
86-86:searchParamsin the dependency array may cause unnecessary effect re-registrations.In Next.js App Router,
useSearchParams()returns a new object on each render. TheloadedRefguard prevents re-fetching, but the effect cleanup/setup cycle still runs. Consider extractingsearchParams.get("payment_intent")into a variable and using that as the dependency instead, or simply use[]sinceloadedRefalready prevents re-execution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx at line 86, The effect currently depends on searchParams which is a new object each render; compute const paymentIntent = searchParams.get("payment_intent") above the useEffect and replace searchParams in the dependency array with paymentIntent (so use [orderId, paymentIntent]) or, if you prefer relying solely on the loadedRef guard, change the dependency array to [] for the effect that calls your fetch logic; update references inside the effect to use paymentIntent and keep loadedRef, orderId and the effect body unchanged.src/components/checkout/PaymentStep.tsx (1)
224-224:returnUrlconstruction via regex is fragile.
pathname.replace(/\/checkout\/.*/, ...)assumes a specific URL structure. If the pathname doesn't contain/checkout/, the replace is a no-op, producing an incorrect return URL. Consider usingextractBasePath(already imported elsewhere in the project) orbasePathfor a more robust construction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentStep.tsx` at line 224, The returnUrl construction in PaymentStep.tsx is fragile because pathname.replace(/\/checkout\/.*/, ...) can be a no-op; replace that logic by deriving the base path via the project's extractBasePath (or existing basePath) helper and then build returnUrl as `${window.location.origin}${basePath}/order-placed/${order.id}` (or, if extractBasePath returns empty, fall back to just `/order-placed/${order.id}`) so the URL is correct even when the path doesn't contain /checkout/; update the code that sets returnUrl to call extractBasePath(window.location.pathname) or use basePath and append `/order-placed/${order.id}`.
🤖 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/checkout/PaymentStep.tsx`:
- Around line 459-461: The submit button can be enabled while Stripe Elements
hasn't finished initializing because we only check
processing/loading/sessionPaymentMethod/clientSecret; update the disabled logic
to also require the gateway handle to be ready: add a gatewayReady boolean
(e.g., state like gatewayReady) that you set true when gatewayHandleRef.current
is populated (or when the Elements onReady callback fires) and false when
tearing down, then include gatewayReady (or gatewayHandleRef.current != null) in
the disabled expression for the submit button in PaymentStep; also ensure
handleSubmit still guards against missing gateway handle but prefer disabling
the button when isAddingNew and gateway isn't ready so the user cannot click
prematurely.
---
Duplicate comments:
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 194-198: handleCardSelect currently fires createSession regardless
of in-flight requests, causing race conditions on rapid card switches; add a
minimal guard to prevent concurrent sessions and disable the radio inputs while
loading. Specifically, in handleCardSelect check the loading flag and return
early if loading is true (prevent calling setSelectedCardId/createSession), and
pass loading to your radio inputs (or RadioGroup) to set the disabled prop so
the UI cannot be clicked while a session is being created; keep createSession
resolution logic unchanged but ensure loading is set/unset around the async call
so the inputs re-enable when complete.
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 12-14: The code uses a non-null assertion on
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY when calling loadStripe (stripePromise),
which can pass undefined and yield an opaque Stripe SDK error; update the
initialization to validate process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY first
(in the module or a helper) and throw or log a clear error if it's missing, then
call loadStripe with the validated value (or return a safe fallback) so that
stripePromise/loadStripe always receives a defined publishable key and failures
are surfaced with a descriptive message.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx:
- Around line 46-79: The code calls getCheckoutOrder(orderId) twice in
loadAndComplete when a payment_intent is present; change the flow to call
getCheckoutOrder once, store its result in a local orderData variable, and reuse
it: if paymentIntent is present, await getCheckoutOrder(orderId) into orderData,
if orderData exists and orderData.state !== "complete" call
completeCheckoutOrder(orderId) and then update orderData by using the response
from completeCheckoutOrder if it returns the updated order, or only call
getCheckoutOrder again once more to refresh state if completeCheckoutOrder
returns no order; finally use that single orderData to setOrder/setError and set
loading flags (preserve loadedRef and cancelled checks).
- Line 86: The effect currently depends on searchParams which is a new object
each render; compute const paymentIntent = searchParams.get("payment_intent")
above the useEffect and replace searchParams in the dependency array with
paymentIntent (so use [orderId, paymentIntent]) or, if you prefer relying solely
on the loadedRef guard, change the dependency array to [] for the effect that
calls your fetch logic; update references inside the effect to use paymentIntent
and keep loadedRef, orderId and the effect body unchanged.
In `@src/components/checkout/PaymentStep.tsx`:
- Line 224: The returnUrl construction in PaymentStep.tsx is fragile because
pathname.replace(/\/checkout\/.*/, ...) can be a no-op; replace that logic by
deriving the base path via the project's extractBasePath (or existing basePath)
helper and then build returnUrl as
`${window.location.origin}${basePath}/order-placed/${order.id}` (or, if
extractBasePath returns empty, fall back to just `/order-placed/${order.id}`) so
the URL is correct even when the path doesn't contain /checkout/; update the
code that sets returnUrl to call extractBasePath(window.location.pathname) or
use basePath and append `/order-placed/${order.id}`.
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 62-66: The useEffect in StripePaymentForm currently depends on
onReady and will re-invoke whenever its identity changes; to avoid fragile
contracts with callers, guard the invocation so onReady({ confirmPayment }) is
only called once: add a ref like calledOnReadyRef checked inside the useEffect
and set it after first call, keep the useEffect dependencies as [stripe,
confirmPayment, onReady] (or drop onReady if you prefer) and reference the ref
to prevent repeated calls; mention PaymentStep/handleGatewayReady as the
expected stable caller but implement this internal once-only guard in useEffect
to make the component robust to unstable parent callbacks.
Comment on lines
+459
to
+461
| disabled={ | ||
| processing || loading || !sessionPaymentMethod || !clientSecret | ||
| } |
Contributor
There was a problem hiding this comment.
Submit button can be clicked before Stripe Elements is ready when adding a new card.
When isAddingNew is true and clientSecret is set, the button is enabled. But if gatewayHandleRef.current hasn't been populated yet (Stripe Elements still loading), handleSubmit silently bails at line 204. Consider also disabling the button until the gateway handle is ready, e.g. by tracking it in state.
Sketch
+ const [gatewayReady, setGatewayReady] = useState(false);
const handleGatewayReady = useCallback((handle: StripePaymentFormHandle) => {
gatewayHandleRef.current = handle;
+ setGatewayReady(true);
}, []);
// Reset when clientSecret changes (new session)
+ useEffect(() => { setGatewayReady(false); }, [clientSecret]);
...
disabled={
processing || loading || !sessionPaymentMethod || !clientSecret
+ || (isAddingNew && !gatewayReady)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| disabled={ | |
| processing || loading || !sessionPaymentMethod || !clientSecret | |
| } | |
| const [gatewayReady, setGatewayReady] = useState(false); | |
| const handleGatewayReady = useCallback((handle: StripePaymentFormHandle) => { | |
| gatewayHandleRef.current = handle; | |
| setGatewayReady(true); | |
| }, []); | |
| useEffect(() => { | |
| setGatewayReady(false); | |
| }, [clientSecret]); | |
| // ... other code ... | |
| disabled={ | |
| processing || loading || !sessionPaymentMethod || !clientSecret || (isAddingNew && !gatewayReady) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/checkout/PaymentStep.tsx` around lines 459 - 461, The submit
button can be enabled while Stripe Elements hasn't finished initializing because
we only check processing/loading/sessionPaymentMethod/clientSecret; update the
disabled logic to also require the gateway handle to be ready: add a
gatewayReady boolean (e.g., state like gatewayReady) that you set true when
gatewayHandleRef.current is populated (or when the Elements onReady callback
fires) and false when tearing down, then include gatewayReady (or
gatewayHandleRef.current != null) in the disabled expression for the submit
button in PaymentStep; also ensure handleSubmit still guards against missing
gateway handle but prefer disabling the button when isAddingNew and gateway
isn't ready so the user cannot click prematurely.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[country]/[locale]/(storefront)/account/orders/page.tsx (1)
20-32:⚠️ Potential issue | 🟡 MinorRemove unused
getStatusColorfunction (lines 20–32).This function is dead code and never called. Only
getPaymentStatusColorandgetShipmentStatusColorare used in the template.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/orders/page.tsx around lines 20 - 32, Remove the unused getStatusColor function: locate the function named getStatusColor and delete its entire declaration (the switch returning various tailwind class strings) since only getPaymentStatusColor and getShipmentStatusColor are used in the template; after removal, run a quick project build or search to ensure there are no remaining references to getStatusColor and that no imports or types depend on it.
🧹 Nitpick comments (8)
src/app/[country]/[locale]/(storefront)/account/orders/page.tsx (2)
162-162:replace("_", " ")only replaces the first underscore.If a payment state like
"balance_due"is the only multi-underscore case this is fine, but for robustness usereplaceAll("_", " ")or a regex with the global flag.Proposed fix
- {order.payment_state?.replace("_", " ") || "N/A"} + {order.payment_state?.replaceAll("_", " ") || "N/A"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/orders/page.tsx at line 162, The code uses order.payment_state?.replace("_", " ") which only replaces the first underscore; update the transformation to replace all underscores (e.g., use replaceAll("_", " ") or replace(/_/g, " ")) so multi-underscore states like "balance_due_now" are fully converted; locate the usage of order.payment_state?.replace("_", " ") in the page.tsx render and replace it with a global-replace variant while keeping the fallback "N/A".
64-78: Consider converting to a Server Component to eliminate client-side data fetching.This page uses
useEffectsolely to fetch orders on mount, then filters them. Since there are no event handlers, interactive state, or browser-only APIs driving the fetch, this entire page could be a Server Component thatawaitsgetOrders()directly and renders the result — with a siblingloading.tsxfor the skeleton UI. This would also allow adding agenerateMetadataexport for SEO.As per coding guidelines: "Avoid using useEffect for data fetching triggered by state changes; use event handlers or Server Actions instead" and "Use Server Components by default; only add 'use client' when needing event handlers, hooks."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/orders/page.tsx around lines 64 - 78, The OrdersPage component should be converted from a client component to a Server Component: remove any "use client" directive and the useEffect/loadOrders logic, and instead call await getOrders({ per_page: 50 }) directly inside the OrdersPage function, filter the returned data for o.state === "complete" and render the orders synchronously; add a sibling loading.tsx for the skeleton UI and add a generateMetadata export to provide page metadata/SEO. Ensure references to the existing getOrders, OrdersPage, and the current filtering logic are preserved when moving to server-side rendering.src/components/checkout/AddressSelector.tsx (1)
36-53: Pre-existing:useEffectused to derive state from props.This
useEffectcomputesselectedAddressIdfromsavedAddressesandcurrentAddress, which is exactly the pattern the coding guidelines ask to avoid. Additionally, theeslint-disableon line 53 suppresses a legitimate warning —currentAddressis read inside the effect but excluded from the dependency array, so the matching logic goes stale when only the address fields change.Consider computing the initial match with
useMemoduring render instead:♻️ Suggested approach
- const [selectedAddressId, setSelectedAddressId] = useState<string | "new">( - "new", - ); - - // Check if current address matches a saved address - useEffect(() => { - if (savedAddresses.length === 0) return; - - const matchingAddress = savedAddresses.find( - (addr) => - addr.address1 === currentAddress.address1 && - addr.city === currentAddress.city && - addr.zipcode === currentAddress.zipcode && - addr.country_iso === currentAddress.country_iso, - ); - - if (matchingAddress) { - setSelectedAddressId(matchingAddress.id); - } else if (currentAddress.address1) { - // If there's an address but it doesn't match saved ones, show as new - setSelectedAddressId("new"); - } - }, [savedAddresses]); // eslint-disable-line react-hooks/exhaustive-deps + const initialAddressId = useMemo<string | "new">(() => { + const match = savedAddresses.find( + (addr) => + addr.address1 === currentAddress.address1 && + addr.city === currentAddress.city && + addr.zipcode === currentAddress.zipcode && + addr.country_iso === currentAddress.country_iso, + ); + return match ? match.id : "new"; + }, [savedAddresses, currentAddress]); + + const [selectedAddressId, setSelectedAddressId] = useState<string | "new">(initialAddressId);As per coding guidelines, "Avoid using useEffect to reset state when props change; use component key prop to reset state or compute initial state from props".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/AddressSelector.tsx` around lines 36 - 53, The effect is incorrectly deriving component state from props and suppresses the missing dependency (currentAddress); replace the useEffect logic with a derived value computed during render (e.g., useMemo) so selected address is computed from savedAddresses and currentAddress instead of being set in useEffect; remove the eslint-disable and the effect block, compute const selectedAddressId = useMemo(() => { /* find matchingAddress by comparing address1, city, zipcode, country_iso and return matchingAddress?.id || (currentAddress.address1 ? "new" : undefined) */ }, [savedAddresses, currentAddress]) and update the component to use this derived selectedAddressId (or lift control to parent if mutable state is required) so matching stays correct when currentAddress changes.src/app/globals.css (1)
14-25: Primary palette has a noticeable hue/saturation jump between 400 and 500.The 50–400 shades are Tailwind's default
bluevalues (e.g.,#60a5fafor 400), while 500–950 are custom shades derived from#0077FF. This creates a visible discontinuity —primary-400is a lighter sky-blue that doesn't smoothly transition into the deeperprimary-500. Components usingprimary-400or lighter shades (e.g., hover states, backgrounds) may look off-brand.Consider generating the full 50–950 range from the
#0077FFbase using a tool like UIColors or Tailwind's color generator for a harmonious palette.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/globals.css` around lines 14 - 25, The primary color scale currently mixes Tailwind's default blues (50–400) with custom shades (500–950) causing a visible jump; regenerate the entire --color-primary-50 through --color-primary-950 scale from the single base color `#0077FF` (using a generator like UIColors or Tailwind color generator) and replace the mismatched 50–400 values so all --color-primary-50..--color-primary-950 are derived consistently from `#0077FF` to ensure smooth hue/saturation transitions.src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (1)
509-515: Simplify identical conditional branches in step indicator.Both
index < currentStepIndexandindex === currentStepIndexproduce the same class"bg-primary-600 text-white". These can be collapsed.Suggested simplification
className={`w-8 h-8 rounded-full flex items-center justify-center text-sm font-medium ${ - index < currentStepIndex - ? "bg-primary-600 text-white" - : index === currentStepIndex - ? "bg-primary-600 text-white" - : "bg-gray-200 text-gray-500" + index <= currentStepIndex + ? "bg-primary-600 text-white" + : "bg-gray-200 text-gray-500" }`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx around lines 509 - 515, The conditional in the step indicator JSX can be simplified because both index < currentStepIndex and index === currentStepIndex yield the same classes; update the className expression (the JSX that references index and currentStepIndex) to use a single branch like index <= currentStepIndex ? "bg-primary-600 text-white" : "bg-gray-200 text-gray-500" so you remove the redundant nested ternary and keep the same visual behavior.src/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsx (2)
42-86: DoublegetCheckoutOrdercall whenpayment_intentis present.When a
payment_intentquery param exists,getCheckoutOrderis called on line 52 and again on line 62. If the order was already complete (line 53 check), the second fetch is redundant — the result from line 52 could be reused.Proposed optimization
async function loadAndComplete() { try { // Handle Stripe redirect return (3DS, etc.) const paymentIntent = searchParams.get("payment_intent"); + let orderData: StoreOrder | null = null; + if (paymentIntent) { - // Find the payment session and complete it - const orderData = await getCheckoutOrder(orderId); - if (orderData && orderData.state !== "complete") { + orderData = await getCheckoutOrder(orderId); + if (orderData && orderData.state !== "complete") { await completeCheckoutOrder(orderId); + // Re-fetch to get updated order after completion + orderData = await getCheckoutOrder(orderId); } } - // Load the order - const orderData = await getCheckoutOrder(orderId); + // Load the order (skip if already fetched above) + if (!orderData) { + orderData = await getCheckoutOrder(orderId); + } + if (!cancelled) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx around lines 42 - 86, The effect double-fetches the order: loadAndComplete calls getCheckoutOrder twice when searchParams has payment_intent; reuse the first fetched orderData instead of refetching. Update loadAndComplete to store the initial getCheckoutOrder(orderId) result in a local variable, pass that into the complete flow (call completeCheckoutOrder only if needed) and then use the same orderData for setOrder/setError, keeping the existing cancelled/loadedRef/setLoading logic in the function; reference loadAndComplete, getCheckoutOrder, completeCheckoutOrder, loadedRef, setOrder, setError, and setLoading when making the change.
22-23: Consider addinggenerateMetadatafor this page.As a page component, this file should ideally export a
generateMetadatafunction for SEO (e.g., "Order Confirmed -#12345"). Since this is a client component, metadata generation could be handled in a separatelayout.tsxor by splitting server/client concerns. As per coding guidelines, page files should usegenerateMetadatato dynamically generate SEO metadata.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx around lines 22 - 23, Add a generateMetadata export that produces dynamic SEO metadata for this page (e.g., title "Order Confirmed - `#12345`") by reading the route params (same shape as OrderPlacedPageProps / params -> { id: orderId }) or by moving metadata into the parent layout.tsx if you prefer server-only logic; implement generateMetadata(params) to format the order id into a title and return a metadata object (title, description, maybe openGraph) so the OrderPlacedPage route has proper dynamic metadata.src/components/checkout/StripePaymentForm.tsx (1)
62-66:onReadyin the dependency array can cause re-render loops if the parent doesn't memoize it.The
useEffectdepends ononReady. If a consumer passes an unstable callback, this triggers infinite re-renders. CurrentlyPaymentStepwrapshandleGatewayReadyinuseCallback, so it works — but this is a fragile contract.Consider either documenting the stability requirement or defending against it:
Defensive approach using a ref
+const onReadyRef = useRef(onReady); +onReadyRef.current = onReady; useEffect(() => { if (stripe) { - onReady({ confirmPayment }); + onReadyRef.current({ confirmPayment }); } -}, [stripe, confirmPayment, onReady]); +}, [stripe, confirmPayment]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/StripePaymentForm.tsx` around lines 62 - 66, The effect currently lists onReady in its dependency array which can cause re-render loops if the parent passes an unstable callback; instead defend against instability by storing the latest onReady in a ref and calling that ref inside the effect so the effect only depends on stable values (stripe and confirmPayment). Update StripePaymentForm to create an onReadyRef (e.g., useRef) and update onReadyRef.current whenever onReady changes, then change the existing useEffect (the one that calls onReady({ confirmPayment })) to depend only on stripe and confirmPayment and invoke onReadyRef.current?.({ confirmPayment }); this preserves current behavior while avoiding infinite re-renders even if PaymentStep/handleGatewayReady is not memoized.
🤖 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/app/`[country]/[locale]/(storefront)/account/credit-cards/page.tsx:
- Around line 73-84: The loadCards function and its caller loadData don't handle
getCreditCards() failures, which can leave the UI stuck loading; update
loadCards to wrap the getCreditCards() call in try/catch (or let it rethrow) and
ensure loadData always calls setLoading(false) in a finally block so the
skeleton is cleared on error, and in the catch set an error state or surface the
failure (e.g., via setCards([]) or a toast) so the user isn't left waiting;
reference the functions loadCards, loadData, getCreditCards, setCards, and
setLoading when making the changes.
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 91-124: The createSession callback lacks protection against thrown
exceptions from createCheckoutPaymentSession which can leave loading true; wrap
the async body of createSession in try/catch/finally: call
createCheckoutPaymentSession inside try, handle success/result like today inside
try, in catch set an appropriate gateway error via setGatewayError(error.message
|| "Failed to create payment session.") and clear client/session state as needed
(setClientSecret(null), setPaymentSessionId(null), gatewayHandleRef.current =
null), and ensure setLoading(false) runs in finally so loading is always
cleared; keep references to createSession, createCheckoutPaymentSession,
setLoading, setGatewayError, setClientSecret, setPaymentSessionId, and
gatewayHandleRef when making the change.
---
Outside diff comments:
In `@src/app/`[country]/[locale]/(storefront)/account/orders/page.tsx:
- Around line 20-32: Remove the unused getStatusColor function: locate the
function named getStatusColor and delete its entire declaration (the switch
returning various tailwind class strings) since only getPaymentStatusColor and
getShipmentStatusColor are used in the template; after removal, run a quick
project build or search to ensure there are no remaining references to
getStatusColor and that no imports or types depend on it.
---
Duplicate comments:
In `@src/components/checkout/PaymentStep.tsx`:
- Around line 194-198: handleCardSelect calls createSession(cardId) without
awaiting and the radio inputs remain enabled when loading, so rapid card
switches can race and leave stale clientSecret/paymentSessionId; fix by
disabling all payment method radio inputs while loading (use the existing
loading state) including the "Add new payment method" radio, and either await or
serialize createSession calls in handleCardSelect (e.g., await
createSession(cardId) or cancel/ignore results from older calls) to ensure the
latest selection wins; update references: handleCardSelect, createSession,
selectedCardId, loading, and the "Add new payment method" radio.
- Around line 457-465: The submit button can become enabled before Stripe
Elements is ready because we only check clientSecret and sessionPaymentMethod;
update the component to track gateway readiness (e.g., add a state like
isGatewayReady) and set it true when gatewayHandleRef.current is populated (or
when the Stripe Elements mount callback completes) and false while loading; then
include isGatewayReady in the disabled expression alongside processing, loading,
sessionPaymentMethod and clientSecret so the button stays disabled until
gatewayHandleRef is ready, and ensure handleSubmit still checks
gatewayHandleRef.current for safety.
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 12-14: Replace the non-null assertion on
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY so loadStripe never receives undefined: check
process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY at startup and throw or log a
clear, descriptive error if it's missing, then pass the validated value into
loadStripe (the symbol stripePromise and the loadStripe call are where to make
the change); this ensures you fail fast with a readable message instead of
producing a cryptic Stripe SDK runtime error.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 509-515: The conditional in the step indicator JSX can be
simplified because both index < currentStepIndex and index === currentStepIndex
yield the same classes; update the className expression (the JSX that references
index and currentStepIndex) to use a single branch like index <=
currentStepIndex ? "bg-primary-600 text-white" : "bg-gray-200 text-gray-500" so
you remove the redundant nested ternary and keep the same visual behavior.
In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx:
- Around line 42-86: The effect double-fetches the order: loadAndComplete calls
getCheckoutOrder twice when searchParams has payment_intent; reuse the first
fetched orderData instead of refetching. Update loadAndComplete to store the
initial getCheckoutOrder(orderId) result in a local variable, pass that into the
complete flow (call completeCheckoutOrder only if needed) and then use the same
orderData for setOrder/setError, keeping the existing
cancelled/loadedRef/setLoading logic in the function; reference loadAndComplete,
getCheckoutOrder, completeCheckoutOrder, loadedRef, setOrder, setError, and
setLoading when making the change.
- Around line 22-23: Add a generateMetadata export that produces dynamic SEO
metadata for this page (e.g., title "Order Confirmed - `#12345`") by reading the
route params (same shape as OrderPlacedPageProps / params -> { id: orderId }) or
by moving metadata into the parent layout.tsx if you prefer server-only logic;
implement generateMetadata(params) to format the order id into a title and
return a metadata object (title, description, maybe openGraph) so the
OrderPlacedPage route has proper dynamic metadata.
In `@src/app/`[country]/[locale]/(storefront)/account/orders/page.tsx:
- Line 162: The code uses order.payment_state?.replace("_", " ") which only
replaces the first underscore; update the transformation to replace all
underscores (e.g., use replaceAll("_", " ") or replace(/_/g, " ")) so
multi-underscore states like "balance_due_now" are fully converted; locate the
usage of order.payment_state?.replace("_", " ") in the page.tsx render and
replace it with a global-replace variant while keeping the fallback "N/A".
- Around line 64-78: The OrdersPage component should be converted from a client
component to a Server Component: remove any "use client" directive and the
useEffect/loadOrders logic, and instead call await getOrders({ per_page: 50 })
directly inside the OrdersPage function, filter the returned data for o.state
=== "complete" and render the orders synchronously; add a sibling loading.tsx
for the skeleton UI and add a generateMetadata export to provide page
metadata/SEO. Ensure references to the existing getOrders, OrdersPage, and the
current filtering logic are preserved when moving to server-side rendering.
In `@src/app/globals.css`:
- Around line 14-25: The primary color scale currently mixes Tailwind's default
blues (50–400) with custom shades (500–950) causing a visible jump; regenerate
the entire --color-primary-50 through --color-primary-950 scale from the single
base color `#0077FF` (using a generator like UIColors or Tailwind color generator)
and replace the mismatched 50–400 values so all
--color-primary-50..--color-primary-950 are derived consistently from `#0077FF` to
ensure smooth hue/saturation transitions.
In `@src/components/checkout/AddressSelector.tsx`:
- Around line 36-53: The effect is incorrectly deriving component state from
props and suppresses the missing dependency (currentAddress); replace the
useEffect logic with a derived value computed during render (e.g., useMemo) so
selected address is computed from savedAddresses and currentAddress instead of
being set in useEffect; remove the eslint-disable and the effect block, compute
const selectedAddressId = useMemo(() => { /* find matchingAddress by comparing
address1, city, zipcode, country_iso and return matchingAddress?.id ||
(currentAddress.address1 ? "new" : undefined) */ }, [savedAddresses,
currentAddress]) and update the component to use this derived selectedAddressId
(or lift control to parent if mutable state is required) so matching stays
correct when currentAddress changes.
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 62-66: The effect currently lists onReady in its dependency array
which can cause re-render loops if the parent passes an unstable callback;
instead defend against instability by storing the latest onReady in a ref and
calling that ref inside the effect so the effect only depends on stable values
(stripe and confirmPayment). Update StripePaymentForm to create an onReadyRef
(e.g., useRef) and update onReadyRef.current whenever onReady changes, then
change the existing useEffect (the one that calls onReady({ confirmPayment }))
to depend only on stripe and confirmPayment and invoke onReadyRef.current?.({
confirmPayment }); this preserves current behavior while avoiding infinite
re-renders even if PaymentStep/handleGatewayReady is not memoized.
Comment on lines
+73
to
+84
| const loadCards = useCallback(async () => { | ||
| const response = await getCreditCards(); | ||
| setCards(response.data); | ||
| }; | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| async function loadData() { | ||
| await loadCards(); | ||
| setLoading(false); | ||
| } | ||
| loadData(); | ||
| }, []); | ||
| }, [loadCards]); |
Contributor
There was a problem hiding this comment.
Data fetching error silently swallowed.
loadCards doesn't handle errors from getCreditCards(). If the API call fails, the promise rejects and setLoading(false) on line 81 is never reached, leaving the user stuck on the skeleton loader.
Proposed fix
const loadCards = useCallback(async () => {
- const response = await getCreditCards();
- setCards(response.data);
+ try {
+ const response = await getCreditCards();
+ setCards(response.data);
+ } catch {
+ // getCreditCards uses withFallback, but guard against unexpected errors
+ setCards([]);
+ }
}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[country]/[locale]/(storefront)/account/credit-cards/page.tsx around
lines 73 - 84, The loadCards function and its caller loadData don't handle
getCreditCards() failures, which can leave the UI stuck loading; update
loadCards to wrap the getCreditCards() call in try/catch (or let it rethrow) and
ensure loadData always calls setLoading(false) in a finally block so the
skeleton is cleared on error, and in the catch set an error state or surface the
failure (e.g., via setCards([]) or a toast) so the user isn't left waiting;
reference the functions loadCards, loadData, getCreditCards, setCards, and
setLoading when making the changes.
Comment on lines
+91
to
+124
| const createSession = useCallback( | ||
| async (cardId: string | null) => { | ||
| if (!sessionPaymentMethod) return; | ||
|
|
||
| setLoading(true); | ||
| setGatewayError(null); | ||
| setClientSecret(null); | ||
| setPaymentSessionId(null); | ||
| gatewayHandleRef.current = null; | ||
|
|
||
| const result = await createCheckoutPaymentSession( | ||
| order.id, | ||
| sessionPaymentMethod.id, | ||
| cardId ?? undefined, | ||
| ); | ||
|
|
||
| setLoading(false); | ||
|
|
||
| if (result.success && result.session) { | ||
| const secret = result.session.external_data?.client_secret as | ||
| | string | ||
| | undefined; | ||
| if (secret) { | ||
| setClientSecret(secret); | ||
| setPaymentSessionId(result.session.id); | ||
| } else { | ||
| setGatewayError("Failed to initialize payment. Please try again."); | ||
| } | ||
| } else if (!result.success) { | ||
| setGatewayError(result.error || "Failed to create payment session."); | ||
| } | ||
| }, | ||
| [sessionPaymentMethod, order.id], | ||
| ); |
Contributor
There was a problem hiding this comment.
createSession has no error boundary for unexpected exceptions.
If createCheckoutPaymentSession throws (network failure, etc.), the error is unhandled. The loading state would remain true since setLoading(false) on line 107 wouldn't be reached, leaving the user stuck on the spinner.
Proposed fix: wrap in try/catch
const createSession = useCallback(
async (cardId: string | null) => {
if (!sessionPaymentMethod) return;
setLoading(true);
setGatewayError(null);
setClientSecret(null);
setPaymentSessionId(null);
gatewayHandleRef.current = null;
+ try {
const result = await createCheckoutPaymentSession(
order.id,
sessionPaymentMethod.id,
cardId ?? undefined,
);
setLoading(false);
if (result.success && result.session) {
const secret = result.session.external_data?.client_secret as
| string
| undefined;
if (secret) {
setClientSecret(secret);
setPaymentSessionId(result.session.id);
} else {
setGatewayError("Failed to initialize payment. Please try again.");
}
} else if (!result.success) {
setGatewayError(result.error || "Failed to create payment session.");
}
+ } catch {
+ setLoading(false);
+ setGatewayError("Failed to initialize payment. Please try again.");
+ }
},
[sessionPaymentMethod, order.id],
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/checkout/PaymentStep.tsx` around lines 91 - 124, The
createSession callback lacks protection against thrown exceptions from
createCheckoutPaymentSession which can leave loading true; wrap the async body
of createSession in try/catch/finally: call createCheckoutPaymentSession inside
try, handle success/result like today inside try, in catch set an appropriate
gateway error via setGatewayError(error.message || "Failed to create payment
session.") and clear client/session state as needed (setClientSecret(null),
setPaymentSessionId(null), gatewayHandleRef.current = null), and ensure
setLoading(false) runs in finally so loading is always cleared; keep references
to createSession, createCheckoutPaymentSession, setLoading, setGatewayError,
setClientSecret, setPaymentSessionId, and gatewayHandleRef when making the
change.
This was referenced Feb 18, 2026
Merged
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores