Add Express Checkout (Apple Pay, Google Pay, Link) to cart#69
Add Express Checkout (Apple Pay, Google Pay, Link) to cart#69damianlegawiec merged 19 commits intomainfrom
Conversation
|
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 Express Checkout (Apple Pay / Google Pay / Link): client-side stripe init, ExpressCheckoutButton UI and wiring, server-side express-checkout actions and utilities (amounts/addresses/shipping), cart/checkout integration (processing/refresh), HTTPS dev support and related config/docs. Changes
Sequence DiagramsequenceDiagram
participant User as User (Browser)
participant UI as Cart / Checkout UI
participant Btn as ExpressCheckoutButton
participant Stripe as Stripe.js
participant Server as Backend Server
participant Order as Order System
User->>UI: Open cart/checkout (total > 0)
UI->>Btn: Mount button (dynamic, ssr:false)
Btn->>Stripe: onReady - check wallet availability
Stripe-->>Btn: availability response
User->>Btn: Click express pay
Btn->>Stripe: request shipping/address
Stripe->>Btn: onShippingAddressChange
Btn->>Server: expressCheckoutResolveShipping(address)
Server-->>Btn: shipping options + updated cart
Btn->>Stripe: resolve shipping options
User->>Stripe: Select shipping option
Stripe->>Btn: onShippingRateChange
Btn->>Server: expressCheckoutSelectRates(selection)
Server-->>Btn: updated cart/total
Btn->>Stripe: resolve with new amount
User->>Stripe: Confirm payment
Stripe->>Btn: onConfirm (paymentMethod)
Btn->>Server: expressCheckoutPreparePayment(details)
Server-->>Btn: updated cart
Btn->>Server: expressCheckoutCreateSession(...)
Server-->>Btn: session / client secret
Btn->>Stripe: confirmPayment(client secret)
Stripe->>Order: process payment
Order-->>Stripe: payment success
Stripe-->>Btn: confirmation
Btn->>Server: expressCheckoutFinalize(sessionId)
Server-->>Btn: order placed
Btn->>UI: onComplete -> refreshCart
Btn->>User: navigate to order confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (5)
src/lib/data/express-checkout-flow.ts (1)
42-63: Sequential rate selection may cause latency with multiple shipments.The loop iterates sequentially over
selections, making one API call per shipment. For orders with multiple shipments, this could introduce noticeable latency. However, if the Spree API requires sequential calls (e.g., each call updates cart state needed by the next), this is correct.If the API supports parallel calls:
💡 Parallel execution suggestion
export async function expressCheckoutSelectRates( cartId: string, selections: Array<{ shipmentId: string; rateId: string }>, ) { return actionResult(async () => { - let cart: Cart | null = null; - - for (const { shipmentId, rateId } of selections) { - const result = await selectShippingRate(cartId, shipmentId, rateId); - if (!result.success) { - throw new Error(result.error); - } - cart = result.cart; - } + if (selections.length === 0) { + throw new Error("No shipment selections provided"); + } + + // Sequential calls required - each updates cart state + let cart: Cart | null = null; + for (const { shipmentId, rateId } of selections) { + const result = await selectShippingRate(cartId, shipmentId, rateId); + if (!result.success) { + throw new Error(result.error); + } + cart = result.cart; + } if (!cart) { throw new Error("No shipment selections provided"); } return { cart }; }, "Failed to select shipping rates"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/express-checkout-flow.ts` around lines 42 - 63, The current expressCheckoutSelectRates function calls selectShippingRate sequentially for each selection, causing extra latency for multiple shipments; if the Spree API supports independent/parallel rate selection, replace the sequential loop with parallel execution (use Promise.all over selections calling selectShippingRate) and then aggregate results, failing fast or collecting errors as appropriate; ensure you still validate each result.success and produce a sensible final cart (e.g., pick the last successful result.cart or merge carts per your domain rules) and surface any errors in the thrown Error; if the API requires sequential updates, leave expressCheckoutSelectRates and selectShippingRate as-is but document that sequential calls are intentional.src/lib/utils/stripe.ts (1)
6-14: Consider failing fast when the publishable key is missing.When
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYis undefined,loadStripe("")is called which returns a Promise resolving tonull. While this works since consumers handle null Stripe instances, the error message mentions "Express checkout" specifically, but this module is also used byStripePaymentFormfor regular checkout.💡 Suggested improvement for clearer error messaging
if (!stripePublishableKey) { console.error( - "Missing NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY environment variable. Express checkout will not be available.", + "Missing NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY environment variable. Stripe payments will not be available.", ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/stripe.ts` around lines 6 - 14, The module currently calls loadStripe("") and logs a message about "Express checkout" only; instead, if stripePublishableKey is falsy, fail fast by throwing an Error (or assigning stripePromise = Promise.reject(...)) with a clear, generic message (e.g., "Missing NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY: Stripe cannot be initialized for payments") so consumers like stripePromise, StripePaymentForm, and any code that awaits loadStripe will immediately surface the problem; additionally, update the existing console.error text to remove the "Express checkout" specificity if you prefer not to throw and only log.src/lib/utils/express-checkout.ts (3)
91-114: Add explicit return type.Per coding guidelines, define an explicit return type for
buildSpreeAddress.♻️ Proposed fix
+interface SpreeAddress { + firstname: string; + lastname: string; + address1: string; + address2: string | undefined; + city: string; + zipcode: string; + country_iso: string; + state_name: string | undefined; + phone: string | undefined; +} + /** Build a Spree-compatible address from Stripe address data. */ export function buildSpreeAddress( name: { firstname: string; lastname: string }, address: { line1: string; line2: string | null; city: string; postal_code: string; country: string; state: string | null; }, phone?: string, -) { +): SpreeAddress { return {As per coding guidelines: "Always define explicit return types for functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/express-checkout.ts` around lines 91 - 114, The function buildSpreeAddress lacks an explicit return type; update its signature to include a concrete return type (either a declared interface/type alias like SpreeAddress or an inline object type) that matches the returned shape (fields: firstname, lastname, address1, address2?, city, zipcode, country_iso, state_name?, phone?). Modify the function declaration to use that type (e.g., export function buildSpreeAddress(...): SpreeAddress) and, if needed, add the corresponding type alias or import the existing type to keep the signature explicit and in sync with the returned object.
44-47: Edge case: potential for shorter suffix.
Math.random().toString(36).slice(2, 6)could theoretically return fewer than 4 characters if the random value is very small (e.g., close to 0). While the probability is negligible, a more robust approach would guarantee the suffix length.♻️ Optional: Guaranteed 4-character suffix
/** Generate a random 4-char suffix for Google Pay shipping rate ID workaround. */ export function randomSuffix(): string { - return Math.random().toString(36).slice(2, 6); + return Math.random().toString(36).slice(2, 6).padEnd(4, "0"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/express-checkout.ts` around lines 44 - 47, The randomSuffix function can produce fewer than 4 chars in rare cases; update randomSuffix to guarantee exactly 4 characters by replacing the direct Math.random() slice with a robust approach (for example, use crypto.randomBytes converted to base36 and then slice(0,4) or loop/concatenate Math.random() outputs until the accumulated base36 string length >=4 and then return slice(0,4)); make the change inside the randomSuffix function so its return value is always a 4-char string.
55-73: Add explicit return type.Per coding guidelines, functions should have explicit return types. Define the return type for
buildLineItems.♻️ Proposed fix
-export function buildLineItems(order: Cart) { +export function buildLineItems( + order: Cart, +): Array<{ name: string; amount: number }> { const currency = order.currency; const items: Array<{ name: string; amount: number }> = [];As per coding guidelines: "Always define explicit return types for functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/express-checkout.ts` around lines 55 - 73, The function buildLineItems lacks an explicit return type; update its signature to declare the return type (e.g., Array<{ name: string; amount: number }> or a named type/interface) so callers and the compiler see the exact shape returned; modify the buildLineItems declaration to include that return type while leaving the implementation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/data/express-checkout-flow.ts`:
- Around line 42-63: The current expressCheckoutSelectRates function calls
selectShippingRate sequentially for each selection, causing extra latency for
multiple shipments; if the Spree API supports independent/parallel rate
selection, replace the sequential loop with parallel execution (use Promise.all
over selections calling selectShippingRate) and then aggregate results, failing
fast or collecting errors as appropriate; ensure you still validate each
result.success and produce a sensible final cart (e.g., pick the last successful
result.cart or merge carts per your domain rules) and surface any errors in the
thrown Error; if the API requires sequential updates, leave
expressCheckoutSelectRates and selectShippingRate as-is but document that
sequential calls are intentional.
In `@src/lib/utils/express-checkout.ts`:
- Around line 91-114: The function buildSpreeAddress lacks an explicit return
type; update its signature to include a concrete return type (either a declared
interface/type alias like SpreeAddress or an inline object type) that matches
the returned shape (fields: firstname, lastname, address1, address2?, city,
zipcode, country_iso, state_name?, phone?). Modify the function declaration to
use that type (e.g., export function buildSpreeAddress(...): SpreeAddress) and,
if needed, add the corresponding type alias or import the existing type to keep
the signature explicit and in sync with the returned object.
- Around line 44-47: The randomSuffix function can produce fewer than 4 chars in
rare cases; update randomSuffix to guarantee exactly 4 characters by replacing
the direct Math.random() slice with a robust approach (for example, use
crypto.randomBytes converted to base36 and then slice(0,4) or loop/concatenate
Math.random() outputs until the accumulated base36 string length >=4 and then
return slice(0,4)); make the change inside the randomSuffix function so its
return value is always a 4-char string.
- Around line 55-73: The function buildLineItems lacks an explicit return type;
update its signature to declare the return type (e.g., Array<{ name: string;
amount: number }> or a named type/interface) so callers and the compiler see the
exact shape returned; modify the buildLineItems declaration to include that
return type while leaving the implementation unchanged.
In `@src/lib/utils/stripe.ts`:
- Around line 6-14: The module currently calls loadStripe("") and logs a message
about "Express checkout" only; instead, if stripePublishableKey is falsy, fail
fast by throwing an Error (or assigning stripePromise = Promise.reject(...))
with a clear, generic message (e.g., "Missing
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY: Stripe cannot be initialized for payments")
so consumers like stripePromise, StripePaymentForm, and any code that awaits
loadStripe will immediately surface the problem; additionally, update the
existing console.error text to remove the "Express checkout" specificity if you
prefer not to throw and only log.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9c13f96-5740-48a8-bc60-092377153619
📒 Files selected for processing (12)
.env.local.example.gitignorenext.config.tspackage.jsonsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/components/cart/CartDrawer.tsxsrc/components/checkout/ExpressCheckoutButton.tsxsrc/components/checkout/StripePaymentForm.tsxsrc/components/checkout/index.tssrc/lib/data/express-checkout-flow.tssrc/lib/utils/express-checkout.tssrc/lib/utils/stripe.ts
c3cbe9a to
51cc0ee
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/checkout/StripePaymentForm.tsx (1)
66-69:⚠️ Potential issue | 🟡 MinorGate
onReadyon both Stripe and Elements availability.At Line 67, the effect only checks
stripe, but the exposed handle methods depend on bothstripeandelements. This can surface a handle that is not actually ready yet.Suggested fix
useEffect(() => { - if (stripe) { + if (stripe && elements) { onReady({ confirmPayment, fetchUpdates }); } - }, [stripe, confirmPayment, fetchUpdates, onReady]); + }, [stripe, elements, confirmPayment, fetchUpdates, onReady]);🤖 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 66 - 69, The effect currently calls onReady when only stripe is truthy, but confirmPayment and fetchUpdates rely on both Stripe and Elements; update the useEffect that invokes onReady to check both stripe and elements (i.e., if (stripe && elements) ...) before calling onReady({ confirmPayment, fetchUpdates }) and ensure stripe and elements are included in the effect's dependency array so the ready handler is only exposed when both are available.
🧹 Nitpick comments (3)
.env.local.example (1)
6-8: Reorder Stripe env keys to satisfy dotenv-linter.
dotenv-linterflagsUnorderedKeyhere; putNEXT_PUBLIC_STRIPE_ACCOUNT_IDbeforeNEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYto keep lint clean.Suggested change
- NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY=pk_test_your_key_here - # Optional: Stripe Connect account ID - NEXT_PUBLIC_STRIPE_ACCOUNT_ID= + # Optional: Stripe Connect account ID + NEXT_PUBLIC_STRIPE_ACCOUNT_ID= + NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY=pk_test_your_key_here🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.local.example around lines 6 - 8, The environment keys are out-of-order for dotenv-linter; swap the two variables so NEXT_PUBLIC_STRIPE_ACCOUNT_ID appears before NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY in .env.local.example (move the line "NEXT_PUBLIC_STRIPE_ACCOUNT_ID=" above "NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY=pk_test_your_key_here") to satisfy the linter and preserve the existing comments/values.src/components/checkout/ExpressCheckoutButton.tsx (1)
43-48: Use the providedbasePathinstead of recomputing fromwindow.location.pathname.
basePathis passed in but not used. Prefer it as the primary source (with optional fallback) to avoid brittle path derivation and remove the effectively-unused prop.As per coding guidelines: "Remove unused variables and imports. If intentionally unused, prefix parameters with underscore."Suggested fix
- const currentBasePath = extractBasePath(window.location.pathname); + const currentBasePath = + basePath || extractBasePath(window.location.pathname); @@ - [stripe, elements, cart.id, cart.payment_methods, onComplete, router], + [stripe, elements, cart.id, cart.payment_methods, onComplete, router, basePath],Also applies to: 306-307, 352-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/ExpressCheckoutButton.tsx` around lines 43 - 48, The component ExpressCheckoutInner currently ignores the passed-in basePath and recomputes it from window.location.pathname; update the code in ExpressCheckoutInner (and the other occurrences noted) to use the basePath prop as the primary source for route derivation and only fall back to window.location.pathname when basePath is falsy, removing the recomputation logic; if basePath is intentionally unused in any of the three places, rename the parameter to _basePath to signal that and remove the unused pathname derivation code so the prop is not effectively unused.src/lib/data/express-checkout-flow.ts (1)
19-126: Add explicit return types to exported server actions.The exported functions lack explicit return type annotations. Declare them to match their actual return shapes (e.g.,
Promise<{ success: true; cart: Cart } | { success: false; error: string }>for functions usingactionResult).As per coding guidelines: "Use strict TypeScript type checking. Always define explicit return types for functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/express-checkout-flow.ts` around lines 19 - 126, The exported server actions (expressCheckoutResolveShipping, expressCheckoutSelectRates, expressCheckoutPreparePayment, expressCheckoutCreateSession, expressCheckoutFinalize) need explicit return type annotations instead of relying on inference; annotate each function to the concrete Promise union returned by actionResult or the delegated call (e.g., for functions returning a cart use Promise<{ success: true; cart: Cart } | { success: false; error: string }>, for finalize use Promise<{ success: true } | { success: false; error: string }>, and for expressCheckoutCreateSession use the exact return type of createCheckoutPaymentSession), and import any missing types (Cart, ActionResult/other helper types) so the signatures are fully explicit. Ensure the annotated types match the shapes produced inside the functions (including error string on failure) and update the function declarations (expressCheckoutResolveShipping, expressCheckoutSelectRates, expressCheckoutPreparePayment, expressCheckoutCreateSession, expressCheckoutFinalize) accordingly.
🤖 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/ExpressCheckoutButton.tsx`:
- Around line 65-71: The useEffect currently force-sets available to false after
8s which can hide Express Checkout permanently if Stripe finishes initializing
late; instead stop unconditionally mutating available on a timer—track real init
outcome and only setAvailable(false) when Stripe initialization has explicitly
failed. Concretely, replace the timeout logic in the useEffect with a mechanism
that (a) adds an init state (e.g., stripeInitAttempted or stripeInitFailed)
updated by the Stripe setup callback, and (b) only calls setAvailable(false)
when that init state indicates failure (and still null), or remove the timeout
entirely and let the Stripe callback resolve setAvailable; keep references to
available, setAvailable and the useEffect cleanup but remove the unconditional
timeout path so recovery remains possible.
---
Outside diff comments:
In `@src/components/checkout/StripePaymentForm.tsx`:
- Around line 66-69: The effect currently calls onReady when only stripe is
truthy, but confirmPayment and fetchUpdates rely on both Stripe and Elements;
update the useEffect that invokes onReady to check both stripe and elements
(i.e., if (stripe && elements) ...) before calling onReady({ confirmPayment,
fetchUpdates }) and ensure stripe and elements are included in the effect's
dependency array so the ready handler is only exposed when both are available.
---
Nitpick comments:
In @.env.local.example:
- Around line 6-8: The environment keys are out-of-order for dotenv-linter; swap
the two variables so NEXT_PUBLIC_STRIPE_ACCOUNT_ID appears before
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY in .env.local.example (move the line
"NEXT_PUBLIC_STRIPE_ACCOUNT_ID=" above
"NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY=pk_test_your_key_here") to satisfy the
linter and preserve the existing comments/values.
In `@src/components/checkout/ExpressCheckoutButton.tsx`:
- Around line 43-48: The component ExpressCheckoutInner currently ignores the
passed-in basePath and recomputes it from window.location.pathname; update the
code in ExpressCheckoutInner (and the other occurrences noted) to use the
basePath prop as the primary source for route derivation and only fall back to
window.location.pathname when basePath is falsy, removing the recomputation
logic; if basePath is intentionally unused in any of the three places, rename
the parameter to _basePath to signal that and remove the unused pathname
derivation code so the prop is not effectively unused.
In `@src/lib/data/express-checkout-flow.ts`:
- Around line 19-126: The exported server actions
(expressCheckoutResolveShipping, expressCheckoutSelectRates,
expressCheckoutPreparePayment, expressCheckoutCreateSession,
expressCheckoutFinalize) need explicit return type annotations instead of
relying on inference; annotate each function to the concrete Promise union
returned by actionResult or the delegated call (e.g., for functions returning a
cart use Promise<{ success: true; cart: Cart } | { success: false; error: string
}>, for finalize use Promise<{ success: true } | { success: false; error: string
}>, and for expressCheckoutCreateSession use the exact return type of
createCheckoutPaymentSession), and import any missing types (Cart,
ActionResult/other helper types) so the signatures are fully explicit. Ensure
the annotated types match the shapes produced inside the functions (including
error string on failure) and update the function declarations
(expressCheckoutResolveShipping, expressCheckoutSelectRates,
expressCheckoutPreparePayment, expressCheckoutCreateSession,
expressCheckoutFinalize) accordingly.
🪄 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: c7b4d630-7220-4988-8a6d-5d1294ee0f35
📒 Files selected for processing (12)
.env.local.example.gitignorenext.config.tspackage.jsonsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/components/cart/CartDrawer.tsxsrc/components/checkout/ExpressCheckoutButton.tsxsrc/components/checkout/StripePaymentForm.tsxsrc/components/checkout/index.tssrc/lib/data/express-checkout-flow.tssrc/lib/utils/express-checkout.tssrc/lib/utils/stripe.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/lib/utils/stripe.ts
- src/components/cart/CartDrawer.tsx
- src/app/[country]/[locale]/(storefront)/cart/page.tsx
- package.json
- next.config.ts
- src/lib/utils/express-checkout.ts
ad20360 to
1343cb8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.env.local.example (1)
6-8: Reorder Stripe env keys to satisfy dotenv-linter.At Line 8,
NEXT_PUBLIC_STRIPE_ACCOUNT_IDshould be placed beforeNEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYto clear the current lint warning.Suggested diff
-NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY=pk_test_your_key_here -# Optional: Stripe Connect account ID -NEXT_PUBLIC_STRIPE_ACCOUNT_ID= +# Optional: Stripe Connect account ID +NEXT_PUBLIC_STRIPE_ACCOUNT_ID= +NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY=pk_test_your_key_here🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.local.example around lines 6 - 8, Dotenv-linter flags the key ordering in .env.local.example; move the NEXT_PUBLIC_STRIPE_ACCOUNT_ID entry so it appears before NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY to satisfy the linter. Edit the .env.local.example file and reorder the two variables so NEXT_PUBLIC_STRIPE_ACCOUNT_ID is on the line above NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY (keeping any comments/values intact), then save and re-run the linter.src/app/[country]/[locale]/(storefront)/cart/page.tsx (1)
16-22: Consider adding a loading fallback for better UX.The dynamic import is correctly configured with
ssr: falsefor client-only Stripe components. However, adding aloadingfallback would prevent layout shift and provide visual feedback while the component loads.♻️ Suggested improvement
const ExpressCheckoutButton = dynamic( () => import("@/components/checkout/ExpressCheckoutButton").then((m) => ({ default: m.ExpressCheckoutButton, })), - { ssr: false }, + { + ssr: false, + loading: () => ( + <div className="h-12 bg-gray-100 rounded animate-pulse" /> + ), + }, );As per coding guidelines: "Use dynamic() import with loading fallback for heavy components like ProductReviews to enable code splitting."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/cart/page.tsx around lines 16 - 22, The dynamic import for ExpressCheckoutButton lacks a loading fallback which can cause layout shift and poor UX; update the dynamic(...) call that defines ExpressCheckoutButton to include a loading option (e.g., a small placeholder/spinner or skeleton) so users see visual feedback while the client-only component loads, ensuring the fallback is lightweight and matches expected dimensions of ExpressCheckoutButton to avoid layout jumps.
🤖 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/express-checkout-flow.ts`:
- Around line 133-144: The current flow calls
completeCheckoutPaymentSession(cartId, sessionId) and then
completeCheckoutOrder(cartId) and throws on order failure, risking a captured
payment without an order; change express-checkout finalization to implement a
reconciliation/compensation strategy: after calling
completeCheckoutPaymentSession, persist a durable
"payment_captured_pending_order" marker (or set a flag on the cart/order
entity), then attempt completeCheckoutOrder with an exponential-backoff retry
loop (e.g., 3 attempts) and, if it still fails, do not throw—return a structured
recoverable status object (e.g., {status: "payment_captured_pending_order",
paymentSessionResult, orderResult, retryAfterSeconds}) so callers can poll or
trigger compensating workflows; also ensure any successful order completion
clears the marker and that completeCheckoutPaymentSession and
completeCheckoutOrder function names are referenced when adding logging and
marker updates.
---
Nitpick comments:
In @.env.local.example:
- Around line 6-8: Dotenv-linter flags the key ordering in .env.local.example;
move the NEXT_PUBLIC_STRIPE_ACCOUNT_ID entry so it appears before
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY to satisfy the linter. Edit the
.env.local.example file and reorder the two variables so
NEXT_PUBLIC_STRIPE_ACCOUNT_ID is on the line above
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY (keeping any comments/values intact), then
save and re-run the linter.
In `@src/app/`[country]/[locale]/(storefront)/cart/page.tsx:
- Around line 16-22: The dynamic import for ExpressCheckoutButton lacks a
loading fallback which can cause layout shift and poor UX; update the
dynamic(...) call that defines ExpressCheckoutButton to include a loading option
(e.g., a small placeholder/spinner or skeleton) so users see visual feedback
while the client-only component loads, ensuring the fallback is lightweight and
matches expected dimensions of ExpressCheckoutButton to avoid layout jumps.
🪄 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: 61487baf-c339-4508-a154-0a9ca7e64ca2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.env.local.example.gitignoreREADME.mdnext.config.tspackage.jsonsrc/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/components/cart/CartDrawer.tsxsrc/components/checkout/ExpressCheckoutButton.tsxsrc/components/checkout/StripePaymentForm.tsxsrc/components/checkout/index.tssrc/lib/data/checkout.tssrc/lib/data/express-checkout-flow.tssrc/lib/utils/express-checkout.tssrc/lib/utils/stripe.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .gitignore
- src/lib/utils/stripe.ts
- package.json
- next.config.ts
- src/lib/utils/express-checkout.ts
- src/components/checkout/index.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (1)
40-46: Add a loading fallback for the client-only Express Checkout import.This area renders empty until the Stripe bundle loads, which creates a visible jump at the top of checkout. Add a small
loadingplaceholder to thedynamic()call. As per coding guidelines, "Use dynamic() import with loading fallback for heavy components like ProductReviews to enable code splitting."🤖 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 40 - 46, The dynamic import for ExpressCheckoutButton currently has no loading fallback, causing a visual jump; update the dynamic(...) call for ExpressCheckoutButton to include a loading option that renders a small placeholder (e.g., a minimal div/spinner/placeholder component) while the client-only Stripe bundle loads. Modify the dynamic invocation that imports "@/components/checkout/ExpressCheckoutButton" (the ExpressCheckoutButton symbol) to pass { ssr: false, loading: () => <YourSmallPlaceholder/> } or similar so the page shows the placeholder until the component is ready.src/app/[country]/[locale]/(storefront)/cart/page.tsx (1)
16-22: Add a loading fallback for the Stripe chunk.This leaves the primary CTA slot blank until the client-only bundle arrives, so the cart summary jumps. Give the
dynamic()import a lightweightloadingplaceholder. As per coding guidelines, "Use dynamic() import with loading fallback for heavy components like ProductReviews to enable code splitting."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/cart/page.tsx around lines 16 - 22, The dynamic import for ExpressCheckoutButton currently omits a loading fallback causing layout jumps; update the dynamic(...) call that imports ExpressCheckoutButton to include a loading option that returns a lightweight placeholder (e.g., a skeleton or disabled button-sized div with the same width/height and basic styling) so the primary CTA slot keeps its space until the Stripe chunk hydrates; target the ExpressCheckoutButton dynamic import and add the loading property to its options to render the small placeholder matching the CTA dimensions.
🤖 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]/(checkout)/checkout/[id]/page.tsx:
- Around line 560-577: The express checkout can be started while the regular
checkout is still processing because both flows share the same processing state;
create a separate expressProcessing state (e.g., expressProcessing) and use it
for the ExpressCheckoutButton’s onProcessingChange and enabled/disabled logic,
leaving the existing processing state for delivery-rate selection and card flow.
Only render/enable the ExpressCheckoutButton block when processing === false
(regular flow idle) and use expressProcessing to control the wallet/button UI so
express actions don’t conflict with ongoing regular mutations; keep loadOrder
usage the same but have ExpressCheckoutButton call setExpressProcessing (not
setProcessing) while preserving onComplete → loadOrder.
- Around line 579-649: The current UI only visually disables checkout sections
during express checkout (using processing and pointer-events-none) but leaves
AddressSection and PaymentSection mounted so their effects (see AddressSection
and PaymentSection) can still run; change the JSX to completely unmount these
interactive sections when processing is true (i.e., render them only when
!processing) so their useEffect/backend calls stop during the wallet flow, and
if you prefer keep them mounted then update the child components
(AddressSection, PaymentSection) to early-return their side-effect logic or gate
callbacks on the processing prop to prevent backend calls while processing.
In `@src/app/`[country]/[locale]/(storefront)/cart/page.tsx:
- Around line 184-189: The onComplete prop currently calls refreshCart() which
triggers getCart() and can clear the guest token during the redirect; change the
onComplete callback passed to ExpressCheckoutButton to be a no-op (remove
refreshCart) or instead call a local-only cart-clear helper that updates local
state without invoking CartContext.getCart(); update the JSX to pass
onComplete={() => {}} (or onComplete={clearLocalCartState}) and ensure any
helper used only clears local cart state rather than calling refreshCart/getCart
to avoid the race with ExpressCheckoutButton's own redirect.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 40-46: The dynamic import for ExpressCheckoutButton currently has
no loading fallback, causing a visual jump; update the dynamic(...) call for
ExpressCheckoutButton to include a loading option that renders a small
placeholder (e.g., a minimal div/spinner/placeholder component) while the
client-only Stripe bundle loads. Modify the dynamic invocation that imports
"@/components/checkout/ExpressCheckoutButton" (the ExpressCheckoutButton symbol)
to pass { ssr: false, loading: () => <YourSmallPlaceholder/> } or similar so the
page shows the placeholder until the component is ready.
In `@src/app/`[country]/[locale]/(storefront)/cart/page.tsx:
- Around line 16-22: The dynamic import for ExpressCheckoutButton currently
omits a loading fallback causing layout jumps; update the dynamic(...) call that
imports ExpressCheckoutButton to include a loading option that returns a
lightweight placeholder (e.g., a skeleton or disabled button-sized div with the
same width/height and basic styling) so the primary CTA slot keeps its space
until the Stripe chunk hydrates; target the ExpressCheckoutButton dynamic import
and add the loading property to its options to render the small placeholder
matching the CTA dimensions.
🪄 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: 8913881f-386e-4435-9cdb-7c4b28f58b3f
📒 Files selected for processing (2)
src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/cart/page.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/lib/data/express-checkout-flow.ts (1)
128-148:⚠️ Potential issue | 🔴 CriticalFinalization still has a charge-without-order recovery gap.
expressCheckoutFinalizestill commits payment first and then returns failure when order completion fails (non-403/422). That leaves a captured payment without a completed order and no durable reconciliation state from this action.Suggested direction
-export async function expressCheckoutFinalize( +export async function expressCheckoutFinalize( cartId: string, sessionId: string, -): Promise<{ success: true } | { success: false; error: string }> { +): Promise< + | { success: true; status: "completed" } + | { + success: true; + status: "payment_captured_pending_order"; + retryAfterSeconds: number; + error: string; + } + | { success: false; error: string } +> { return actionResult(async () => { const sessionResult = await completeCheckoutPaymentSession(cartId, sessionId); if (!sessionResult.success) { throw new Error(sessionResult.error); } - const orderResult = await completeCheckoutOrder(cartId); - if (!orderResult.success) { - throw new Error(orderResult.error); - } + // Attempt completion with bounded retries; if still failing, + // return a recoverable "pending_order" status for reconciliation/polling. + const orderResult = await completeCheckoutOrder(cartId); + if (!orderResult.success) { + return { + status: "payment_captured_pending_order" as const, + retryAfterSeconds: 5, + error: orderResult.error, + }; + } - return {}; + return { status: "completed" as const }; }, "Failed to finalize order"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/express-checkout-flow.ts` around lines 128 - 148, expressCheckoutFinalize currently captures payment via completeCheckoutPaymentSession and then tries to complete the order with completeCheckoutOrder, which can leave a captured payment without any durable reconciliation state if order completion fails; change the flow in expressCheckoutFinalize (or refactor into a new helper) to either 1) perform order completion before capturing payment (call completeCheckoutOrder first, then completeCheckoutPaymentSession) or 2) if you must capture first, on a non-403/422 failure from completeCheckoutOrder persist a durable recovery record (e.g. create a PendingOrderRecovery or PaymentWithoutOrder entry) containing cartId, sessionId, payment/session result, and failure reason so background reconciliation can fix it, and only then return the error; ensure this uses actionResult and updates/reuses the same error path so callers get a reliable state to reconcile.src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (2)
586-656:⚠️ Potential issue | 🟠 MajorVisual disable is insufficient; checkout sections remain mounted during express flow.
pointer-events-noneonly blocks clicks. Mounted child effects/callbacks can still execute backend updates while wallet checkout is mutating the order.🤖 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 586 - 656, The visual-disable (opacity + pointer-events-none) still leaves AddressSection, DeliveryMethodSection and PaymentSection mounted so their effects/callbacks can run during an express/wallet checkout; replace the single wrapper div rendering with conditional rendering so these child components are unmounted when doing express checkout. Detect the express-wallet flow (e.g., use an existing flag or add one such as isExpressCheckout or check the wallet payment state used in validateAndPay) and render either a disabled placeholder/skeleton or nothing when that flag is true (and processing is true), otherwise render AddressSection, DeliveryMethodSection and PaymentSection as now; ensure any refs like paymentRef and handlers (handleAutoSave, handleDeliveryRateSelect, handlePaymentComplete) are not invoked while unmounted.
567-584:⚠️ Potential issue | 🟠 MajorExpress checkout can still start while regular checkout work is in flight.
processingis shared with delivery/card mutations, but the express block remains active here, so users can trigger concurrent mutations on the same cart.🤖 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 567 - 584, The express checkout can be triggered while other checkout mutations are in flight because the UI renders ExpressCheckoutButton based only on isAuthenticated and cart.total; update the guard to also check the shared processing flag (the processing state used with setProcessing) or pass a disabled prop into ExpressCheckoutButton so it cannot start when processing is true. Specifically, modify the render condition around ExpressCheckoutButton (and/or add a disabled prop) to include !processing, and ensure the ExpressCheckoutButton click handler (or its onComplete/onProcessingChange usage) short-circuits if processing is true to prevent concurrent mutations on the same cart.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 586-656: The visual-disable (opacity + pointer-events-none) still
leaves AddressSection, DeliveryMethodSection and PaymentSection mounted so their
effects/callbacks can run during an express/wallet checkout; replace the single
wrapper div rendering with conditional rendering so these child components are
unmounted when doing express checkout. Detect the express-wallet flow (e.g., use
an existing flag or add one such as isExpressCheckout or check the wallet
payment state used in validateAndPay) and render either a disabled
placeholder/skeleton or nothing when that flag is true (and processing is true),
otherwise render AddressSection, DeliveryMethodSection and PaymentSection as
now; ensure any refs like paymentRef and handlers (handleAutoSave,
handleDeliveryRateSelect, handlePaymentComplete) are not invoked while
unmounted.
- Around line 567-584: The express checkout can be triggered while other
checkout mutations are in flight because the UI renders ExpressCheckoutButton
based only on isAuthenticated and cart.total; update the guard to also check the
shared processing flag (the processing state used with setProcessing) or pass a
disabled prop into ExpressCheckoutButton so it cannot start when processing is
true. Specifically, modify the render condition around ExpressCheckoutButton
(and/or add a disabled prop) to include !processing, and ensure the
ExpressCheckoutButton click handler (or its onComplete/onProcessingChange usage)
short-circuits if processing is true to prevent concurrent mutations on the same
cart.
In `@src/lib/data/express-checkout-flow.ts`:
- Around line 128-148: expressCheckoutFinalize currently captures payment via
completeCheckoutPaymentSession and then tries to complete the order with
completeCheckoutOrder, which can leave a captured payment without any durable
reconciliation state if order completion fails; change the flow in
expressCheckoutFinalize (or refactor into a new helper) to either 1) perform
order completion before capturing payment (call completeCheckoutOrder first,
then completeCheckoutPaymentSession) or 2) if you must capture first, on a
non-403/422 failure from completeCheckoutOrder persist a durable recovery record
(e.g. create a PendingOrderRecovery or PaymentWithoutOrder entry) containing
cartId, sessionId, payment/session result, and failure reason so background
reconciliation can fix it, and only then return the error; ensure this uses
actionResult and updates/reuses the same error path so callers get a reliable
state to reconcile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9a79157-77e5-439a-981a-ab9134ecca95
📒 Files selected for processing (5)
src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/components/checkout/ExpressCheckoutButton.tsxsrc/components/checkout/StripePaymentForm.tsxsrc/lib/data/express-checkout-flow.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/checkout/StripePaymentForm.tsx
- src/app/[country]/[locale]/(storefront)/cart/page.tsx
d7a6fc5 to
9cd6260
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Should work correct - make sure u have setuped ur card on https://wallet.google.com/wallet/home?utm_source=pgc&utm_medium=website&utm_campaign=redirect before test google pay |
Port Express Checkout from the old quick-checkout branch, adapted to the new SDK v0.10 (Cart-based API with cookie auth). Adds the ExpressCheckoutButton component to both the cart page and cart drawer, with server actions for the full flow: resolve shipping, select rates, prepare payment, create session, and finalize order. Also adds HTTPS dev server support (dev:https script + mkcert certs) required for Apple Pay / Google Pay testing, and centralizes Stripe initialization with Connect account support. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g line item - Use cart.item_total instead of cart.total + buffer for initial Stripe amount - Add "Shipping" line item to Apple Pay summary after address selection - Move ExpressCheckoutButton outside expressProcessing conditional to prevent unmounting during payment confirmation - Reorder cart drawer footer: Summary → Express Checkout → Checkout/View Cart Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename shipments → fulfillments, shipping_rates → delivery_rates, shipping_method_id → delivery_method_id, selectShippingRate → selectDeliveryRate to match the new SDK types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show Apple Pay / Google Pay / Link buttons side-by-side above the Contact section when the user is not logged in. Add maxColumns and showDivider props to ExpressCheckoutButton for layout flexibility. Also fix fulfillment fetching and shipping rate compatibility with older Spree API responses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document how to set up local HTTPS with mkcert for testing Express Checkout (Apple Pay, Google Pay) on non-localhost domains. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On /cart page, hide "Proceed to Checkout" and "Continue Shopping" buttons while express checkout is processing. On checkout page, dim and disable all form sections (address, shipping, payment, pay button) with opacity + pointer-events overlay during processing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oading UX - StripePaymentForm: check both stripe && elements before calling onReady so callers get a functional handle (not one that returns early errors) - ExpressCheckoutButton: use basePath prop instead of recomputing from window.location.pathname; remove extractBasePath import - ExpressCheckoutButton: remove 8s timeout that permanently hid express checkout if Stripe initialized slowly (no recovery path) - ExpressCheckoutButton: add smooth crossfade animation between loading spinner and payment buttons using shared positioning (no layout shift) - Cart page: change onComplete to no-op to prevent refreshCart() racing with navigation after express checkout completes - Cart/checkout pages: add loading spinner fallback to dynamic imports for ExpressCheckoutButton to prevent layout shift during chunk load - express-checkout-flow: add explicit return type annotations to all exported server action functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of syncing processing state to the parent via useEffect, call onProcessingChange directly alongside setProcessing through an updateProcessing helper. This follows React 19 best practices by avoiding useEffect for notifying parents of state changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After rebasing onto main (which bumped SDK to 0.15), update all express checkout code to use the new field names: shipping_address/billing_address, first_name/last_name, postal_code, and discount_total. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of abruptly mounting/unmounting the "Finalizing your payment..." indicator, use an absolute overlay that fades in over the button area, eliminating layout shift during express checkout processing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Express checkout skipped the confirm-payment intermediate page and navigated directly to order-placed without caching the completed order. When the cart token cookie was cleared, the page had no way to retrieve the order data. - Return order data from expressCheckoutFinalize so callers can cache it - Cache completed order in sessionStorage before navigating to order-placed - Fetch completed order in completeCheckoutOrder on 403/422 (already completed) - Fetch completed order in confirmPaymentAndCompleteCart when cart is gone - Reset expressProcessing state in CartDrawer on close to restore checkout UI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add onAvailabilityChange callback to ExpressCheckoutButton so parent components can react to Stripe payment method availability. The checkout page now hides the "Express checkout" heading when no methods (Apple Pay, Google Pay, Link) are available for the current domain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Locally-trusted certs with shop.lvh.me cannot pass Stripe's payment method domain verification (Stripe fetches the verification file from the public internet). Document cloudflared quick tunnel as the supported local HTTPS path for Apple Pay / Google Pay, and note that the Spree backend must also be publicly reachable.
mkcert + lvh.me is no longer the recommended local HTTPS path (see the README HTTPS Development section). Drop the unused dev:https npm script and the /certificates gitignore entry, and whitelist *.trycloudflare.com in allowedDevOrigins and images.remotePatterns so quick tunnels work out of the box.
The storefront does not use Stripe Connect, so drop the NEXT_PUBLIC_STRIPE_ACCOUNT_ID env var and the conditional stripeAccount option passed to loadStripe().
0ca92f8 to
38504e9
Compare
elements.update() must succeed before event.resolve() — Stripe validates that Elements amount >= sum(lineItems). The previous try/catch silently swallowed update failures, leaving the old amount (subtotal only) while lineItems included shipping.
…_total The Elements amount was initialized and synced using cart.item_total (subtotal only), but buildLineItems() also includes tax and discounts. When a cart had tax from a prior checkout attempt, the lineItems sum exceeded the Elements amount, causing Stripe IntegrationError. Use buildLineItems() total for both initial Elements options and the sync effect so the amount always covers all line items.
Summary
Closes #29
Test plan
/cart) → Express Checkout buttons work the same way🤖 Generated with Claude Code
Summary by CodeRabbit