fix: optimistic cart — custom zustand-based cart sync replacing Hydrogen's useOptimisticCart

[paul-phan]

Summary

Fixes cart data inconsistency issues: stale quantities, visual "drop-back" flashes, phantom qty:0, and $numCartLines variable errors during cart mutations.

Problems

Hydrogen's built-in useOptimisticCart has three design flaws that cause cart UI inconsistency:

1. Double-counting: Processes ALL fetchers with formData including completed (idle) ones. When the mutation result is already reflected in the baseline cart, the hook re-applies the mutation, showing qty N+2 instead of N+1.

2. Stale root loader overwrite: The deferred cart.get() promise in the root loader is captured before mutations. When it resolves, it overwrites the fresh post-mutation cart data with stale pre-mutation state.

3. $numCartLines undeclared: CART_MUTATION_FRAGMENT copied lines(first: $numCartLines) from the query fragment, but mutation operations don't declare this variable — causing GraphQL errors on add-to-cart.

Solution

Custom cart sync architecture (app/components/cart/store.ts)

• findFreshestFetcherCart() — Scans completed (idle) fetchers for the freshest cart data to use as baseline, ensuring useOptimisticCart-style optimistic mutations start from the correct state.

• applyOptimisticMutations() — Custom optimistic cart that ONLY processes pending (non-idle) fetchers. This prevents double-counting that Hydrogen's useOptimisticCart causes.

• useCart() — Single source of truth hook. Picks the freshest baseline (fetcher data > zustand serverCart), applies pending optimistic mutations, and syncs fetcher data to zustand via useEffect for persistence after fetcher cleanup.

• CartStoreSync() — Syncs deferred root loader cart to zustand using updatedAt timestamp comparison. Skips stale root loader data when a fetcher has already synced fresher post-mutation state.

GraphQL fragment fix (app/graphql/fragments.ts)

• CART_MUTATION_FRAGMENT now replaces $numCartLines with hardcoded 250, preventing the undeclared variable error.

Consumer simplification

• CartDrawer — Removed Suspense/Await wrapper and nested CartDrawerContent component. Now directly uses useCart() hook.
• CartRoute — Uses useCart() instead of useOptimisticCart with loader data.
• AddToCartButton / CartLineItem — Updated import from useCartDrawerStore → useCartStore.

Files Changed

File	Change
app/components/cart/store.ts	New custom cart sync (zustand + render-time fetcher sync)
app/graphql/fragments.ts	$numCartLines → 250 in mutation fragment
app/components/cart/cart-drawer.tsx	Simplified: useCart() replaces Suspense/Await/useOptimisticCart
app/routes/cart/cart-page.tsx	useCart() replaces useOptimisticCart
app/components/cart/cart-main.tsx	Minor type adjustment
app/components/cart/cart-line-item.tsx	Import rename, removed dead code
app/components/product/add-to-cart-button.tsx	Import rename
app/.server/context.ts	Uses CART_MUTATION_FRAGMENT for cart config
app/root.tsx	Added <CartStoreSync /> to root layout

Testing

Manually tested with rapid consecutive add-to-cart clicks. Cart quantity updates correctly without drop-back flashes or phantom zeroing.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
pilot	Error		Apr 15, 2026 5:37am

[paul-phan]

Code Review: Custom Zustand-Based Cart Sync

Commit: c69ee1ec — Replace Hydrogen useOptimisticCart with custom zustand-based cart store

Summary

This PR replaces Hydrogen's built-in useOptimisticCart with a custom implementation that solves real, confirmed bugs: stale deferred root loader data overwriting fresher mutation results, double-counting from Hydrogen's useOptimisticCart not filtering by fetcher state, and visual "drop-back" flashes between optimistic UI ending and server data arriving. The architecture — zustand store as single source of truth, render-time fetcher sync, timestamp-guarded root loader updates — is sound and well-motivated.

The unrelated XShareButton → TwitterShareButton fix in blog-post.tsx and the mutateFragment addition in context.ts are both correct.

---

Issues

1. findFreshestFetcherCart doesn't actually find the freshest — it finds the last in array order

app/components/cart/store.ts:31-45 · Confidence: 85%

function findFreshestFetcherCart(fetchers) {
  let freshest = null;
  for (const fetcher of fetchers) {
    if (fetcher.state === "idle" && fetcher.data?.cart?.id && fetcher.data?.cart?.lines) {
      freshest = fetcher.data.cart;
    }
  }
  return freshest;
}

The name says "freshest" but the logic returns whichever idle-with-cart-data fetcher appears last in the useFetchers() array. React Router doesn't guarantee ordering by completion time. If two mutations complete in the same render (e.g., rapid add-then-remove), this may pick the wrong one.

Since CartStoreSync already uses updatedAt timestamps for root loader data, consider comparing updatedAt here too for consistency:

if (fetcher.state === "idle" && fetcher.data?.cart?.id && fetcher.data?.cart?.lines) {
  const candidate = fetcher.data.cart as CartApiQueryFragment;
  if (!freshest || new Date(candidate.updatedAt).getTime() > new Date(freshest.updatedAt).getTime()) {
    freshest = candidate;
  }
}

---

2. CART_MUTATION_FRAGMENT string replacement only targets first occurrence

app/graphql/fragments.ts:360-363 · Confidence: 80%

export const CART_MUTATION_FRAGMENT = CART_QUERY_FRAGMENT.replace(
  "fragment CartApiQuery on Cart",
  "fragment CartApiMutation on Cart",
).replace("$numCartLines", "250");

String.replace() without /g flag only replaces the first match. Currently there's exactly one $numCartLines in the fragment, so this works. But if anyone adds a second reference to $numCartLines in CART_QUERY_FRAGMENT, the second occurrence silently survives as an undeclared variable — the exact bug this PR fixes.

Safer:

).replaceAll("$numCartLines", "250");

A comment explaining why the variable substitution exists would help future maintainers.

---

3. structuredClone on every render with pending fetchers

app/components/cart/store.ts:56 · Confidence: 90% (correctness is fine, perf suggestion)

const cart = structuredClone(baseline) as CartWithOptimistic & { ... };

applyOptimisticMutations runs on every render of any component using useCart() when there are pending fetchers. structuredClone deep-copies the entire cart object including all line items, images, cost objects, etc. Since only lines.nodes, totalQuantity, and isOptimistic are mutated, a shallow copy would be sufficient and measurably faster:

const nodes = [...baseline.lines.nodes];
const cart = {
  ...baseline,
  lines: { ...baseline.lines, nodes },
  totalQuantity: baseline.totalQuantity,
  isOptimistic: false,
} as CartWithOptimistic & { lines: { nodes: OptimisticLineNode[] }; totalQuantity: number };

Not blocking, but worth doing since this is a hot path.

---

4. Pre-existing: NoteUpdate can leave result uninitialized

app/routes/cart/cart-page.tsx:48-53 · Not part of this diff

case CartForm.ACTIONS.NoteUpdate: {
  const cartNote = inputs.cartNote as string;
  if (cartNote) {
    result = await cart.updateNote(cartNote);
  }
  break;
}

If cartNote is falsy (empty string to clear the note), result is never assigned. Line 98 handles it with result || {}, so no crash — but a "clear note" action silently does nothing. Pre-existing, noting for awareness.

---

Verdict

Approve with suggestions. The core architecture is correct and solves confirmed bugs that Hydrogen's built-in useOptimisticCart doesn't handle. Issues #1 and #2 are worth addressing before merge; #3 is optional cleanup.

[Copilot]

Pull request overview

Replaces Hydrogen’s useOptimisticCart usage with a custom zustand-backed cart synchronization layer to prevent cart UI inconsistencies during mutations, and fixes the cart mutation GraphQL fragment so mutations no longer reference an undeclared $numCartLines variable.

Changes:

• Added a zustand-based cart store + useCart() hook that chooses a “freshest” baseline cart and applies only pending optimistic mutations.
• Wired cart consumers (drawer + cart route + add-to-cart interactions) to use the new useCart() / useCartStore() APIs.
• Introduced a dedicated cart mutation fragment (CART_MUTATION_FRAGMENT) that hardcodes cart line count to avoid $numCartLines GraphQL errors.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
app/components/cart/store.ts	New zustand cart state + fetcher/root-loader syncing + custom optimistic overlay logic
app/components/cart/cart-drawer.tsx	Removes <Suspense>/<Await> cart consumption; uses useCart() + useCartStore()
app/routes/cart/cart-page.tsx	Switches cart page to useCart() instead of loader cart
app/components/cart/cart-main.tsx	Updates prop typing to accept an optimistic/nullable cart and removes useOptimisticCart usage
app/components/cart/cart-line-item.tsx	Store import rename + simplifies optimistic-state handling and remove-button logic
app/components/product/add-to-cart-button.tsx	Store import rename (useCartDrawerStore → useCartStore)
app/graphql/fragments.ts	Adds CART_MUTATION_FRAGMENT by transforming the query fragment and replacing $numCartLines
app/.server/context.ts	Configures Hydrogen cart context to use both query and mutation fragments
app/root.tsx	Adds <CartStoreSync /> to sync deferred root cart into zustand
app/sections/blog-post.tsx	Replaces XShareButton with TwitterShareButton from react-share
.weaverse/specs/2026-04-13-optimistic-cart-split-design.md	New spec doc (currently not aligned with the implemented approach)
.weaverse/specs/2026-04-10-optimistic-cart-fix*.md	New spec/design docs related to optimistic cart work

Comments suppressed due to low confidence (1)

app/routes/cart/cart-page.tsx:122

• useCart() replaces the loader-provided cart here, which means /cart can initially render as empty (Cart (0) + empty-state UI) until CartStoreSync's effect resolves the deferred root cart promise. Since this route's loader already awaits cart.get(), consider using that loader cart as the immediate baseline (e.g., pass originalCart into useCart(originalCart) or seed the zustand serverCart from loader data) to avoid a flash of incorrect empty cart state.

export default function CartRoute() {
  const { featuredProducts } = useLoaderData<typeof loader>();
  const cart = useCart();

  return (
    <>
      <Section width="fixed" verticalPadding="medium">
        <h1 className="h3 mb-8 text-center md:mb-16">
          Cart ({cart?.totalQuantity || 0})
        </h1>
        <CartMain layout="page" cart={cart} />

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[hta218]

Sometimes it works, sometimes it's not, please see this video: https://www.loom.com/share/d97c8ea3e93c47fd9ec12beb0395d4ce

[paul-phan]

Code Review: Custom Zustand-Based Cart Sync

Architecture

The approach is sound. Hydrogen's useOptimisticCart has real design flaws (double-counting from idle fetchers, no stale root loader protection), and replacing it with a custom zustand-based system is the right call. The updatedAt timestamp comparison in CartStoreSync is an elegant solution to the stale root loader overwrite problem.

---

Critical: findFreshestFetcherCart via useFetchers() is unreliable

app/components/cart/store.ts:32-44 — findFreshestFetcherCart scans useFetchers() for idle fetchers with cart data. This has two issues:

1. React Router cleans up idle fetchers synchronously. useFetchers() reads from state.fetchers — React Router deletes idle fetchers from this map on the same synchronous tick as completion. In practice, this function rarely finds completed fetchers, making the fetcher-to-zustand sync in useCart() unreliable.

2. Returns last in array order, not freshest. The function name implies timestamp comparison but simply picks whichever idle fetcher appears last in the list. If two fetchers complete in the same render cycle, the order is arbitrary.

Fix (already implemented locally, not yet committed): Replace findFreshestFetcherCart with useCartFetcherSync(fetcher) — a hook that takes a singular useFetcher() instance. The singular useFetcher() preserves fetcher.data via an internal fetcherData ref that survives cleanup, unlike the plural useFetchers() map. Each component that uses CartForm calls useCartFetcherSync(fetcher) to reliably sync its mutation response to zustand.

Severity: Critical — Without this fix, serverCart in zustand may never update from mutation responses, causing the same stale baseline problem this PR was designed to fix.

---

Minor: structuredClone on every render with pending fetchers

app/components/cart/store.ts:58 — applyOptimisticMutations uses structuredClone(baseline) on every render cycle when pending fetchers exist. Since only lines.nodes, totalQuantity, and isOptimistic are mutated, a shallow copy is sufficient and cheaper:

const nodes = [...baseline.lines.nodes] as OptimisticLineNode[];
const cart = {
  ...baseline,
  lines: { ...baseline.lines, nodes },
  totalQuantity: baseline.totalQuantity,
  isOptimistic: false,
} as CartWithOptimistic & { ... };

Severity: Low — negligible for typical cart sizes, but easy win.

---

Minor: CART_MUTATION_FRAGMENT string replacement fragility

app/graphql/fragments.ts:360-363 — String.replace() without /g only replaces the first occurrence. Currently there's only one $numCartLines, but if anyone adds another reference to the query fragment, the second occurrence silently survives as an unbound variable. Consider adding a comment explaining the intentional hardcoding.

Severity: Low — works today, maintenance risk.

---

Note: Suspense fallback removal changes cart badge UX

CartDrawer previously used <Suspense>/<Await> with a fallback. Now useCart() returns null until CartStoreSync's effect resolves the deferred promise, causing a brief flash of no badge → badge with count on initial page load. Not a bug, but a behavior change worth noting.

---

Summary

The core architecture (zustand store, custom optimistic mutations, timestamp-based staleness detection) is solid. The critical issue is the useFetchers() reliability problem — the fix for this is already implemented locally and should be committed before merging.

[paul-phan]

Code Review — PR #375

Summary

Solid fix for a real Hydrogen framework bug (useOptimisticCart double-counting + stale root loader overwrite). The custom zustand-based cart sync is well-architected and the updatedAt timestamp guard is the right approach. The fix commit (273fdaa3) addressed the initial review round well.

5 issues found (1 bug, 2 code quality, 2 maintenance).

---

🔴 Bug: CartStoreSync clears fresher fetcher data when root loader resolves null

File: app/components/cart/store.ts — CartStoreSync effect

When the root loader's deferred cart promise resolves to null (fresh session, no cart), it unconditionally sets serverCart: null — bypassing the updatedAt timestamp guard that protects the non-null path.

Race condition scenario:

1. User lands on site (fresh session). Root loader defers cart.get() → will resolve null
2. User clicks "Add to Cart" before promise resolves. Fetcher completes → useCartFetcherSync sets serverCart to the newly-created cart
3. Root loader's deferred promise resolves null (initiated pre-mutation)
4. CartStoreSync sees !resolved → sets serverCart: null → cart disappears

Fix: Guard the null path the same way:

if (!resolved) {
  // Don't overwrite a fresher fetcher-synced cart with null
  const current = useCartStore.getState().serverCart;
  if (!current) {
    useCartStore.setState({ serverCart: null });
  }
  return;
}

Confidence: 95% — The updatedAt guard exists precisely for this class of bug but only covers the truthy branch.

---

🟡 AGENTS.md: any type used in 5 places across 3 files

Rule: "Avoid any type when possible, use unknown if escape hatch needed"

File	Location	Code
store.ts	useCartFetcherSync param	Fetcher<any>
cart-line-item.tsx	ItemRemoveButton render prop	FetcherWithComponents<any>
cart-line-item.tsx	ItemRemoveButtonInner prop	FetcherWithComponents<any>
cart-line-qty-adjust.tsx	UpdateCartButton render prop	FetcherWithComponents<any>
cart-line-qty-adjust.tsx	UpdateCartButtonInner prop	FetcherWithComponents<any>

Fix: Replace with Fetcher<unknown> / FetcherWithComponents<unknown> or ideally Fetcher<{ cart: CartApiQueryFragment; userErrors: unknown[]; errors: unknown[] }>.

Confidence: 90%

---

🟡 Missing optimistic state for non-line mutations

applyOptimisticMutations only handles LinesAdd, LinesRemove, LinesUpdate. Other CartForm actions (DiscountCodesUpdate, GiftCardCodesAdd/Remove, NoteUpdate, BuyerIdentityUpdate) won't set isOptimistic: true while the fetcher is pending.

This means discount code application, gift card usage, and note updates won't show loading/disabled states during mutation — users can double-submit.

Fix: Add a catch-all at the end of the action switch:

default:
  cart.isOptimistic = true;
  break;

Confidence: 80%

---

🟢 Stale spec documents

The 3 files in .weaverse/specs/ describe the design exploration that led to this implementation, but 2026-04-13-optimistic-cart-split-design.md describes an abandoned <Suspense><Await> + useOptimisticCart approach that doesn't match the final implementation. Future maintainers may be confused.

Fix: Add a note at the top of the split-design spec: > **Note:** This design was superseded by the zustand-based approach in store.ts.

Confidence: 85%

---

🟢 No automated tests for regression scenarios

The complex race conditions that motivated this PR (rapid consecutive ATC, stale root loader overwrite, double-counting) remain untested. Given this is now custom cart logic diverging from Hydrogen's built-in approach, regression coverage would be valuable.

Confidence: 85%

---

Review generated by code review agents. 5 parallel agents examined: AGENTS.md compliance, bug scan, git history, prior PR comments, and code comment compliance.

[paul-phan]

@hta218 please review again

[paul-phan]

@hta218 friendly nudge — could you take another pass when you have a moment?

Since your Apr 15 review, Paul shipped several fixes addressing the trash→reappear flash from your Loom:

• 273fdaa — per-fetcher useCartFetcherSync + replaceAll for $numCartLines
• 375f994 — type safety, restored workaround comments
• 252955a → dfbfcc0 → a40bb6f — tombstone registry, then moved out of render to fix Rules of Hooks
• a2a3d22 — final fix: module-level freshestFetcherCartRef that persists across component unmounts, so removed items stay removed even when the remove button unmounts before the fetcher completes (the exact scenario in your video)
• 987ea20 — debug log cleanup + sync stabilization

PR is MERGEABLE, CI green, but blocked on your CHANGES_REQUESTED. If the flash-back is resolved on your end, a re-approval would unblock the merge 🙏

A few minor non-blocking nits I noticed if you want to address in a follow-up:

1. CartStoreSync null-resolution path in store.ts doesn't guard with updatedAt — small race window where a null root-loader resolution could clear a fresher fetcher-synced cart.
2. applyOptimisticMutations only sets isOptimistic: true for Lines* actions — DiscountCodesUpdate, NoteUpdate, etc. won't show loading state, leaving room for double-submit.
3. .weaverse/specs/2026-04-13-optimistic-cart-split-design.md describes the abandoned Suspense/Await approach — worth marking as superseded for future maintainers.

— posted on behalf of @paul-phan via Samantha