fix: prevent stale cart cache after navigation - #527
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Mutation cache cancellation libs/storefront-data/src/shared/cart-cache-sync.ts, libs/storefront-data/src/cart/hooks.ts |
cancelCartCaches cancels matching active-cart and detail queries in parallel. Mutation success handlers await cache synchronisation before callbacks or completion. |
Stale alias protection libs/storefront-data/src/cart/hooks.ts |
Cart aliases propagate only when the active source query still contains the returned cart object. |
Reactivity regression coverage libs/storefront-data/tests/cart.hooks.reactivity.test.tsx |
Tests cover mutation ordering, inactive observer restoration, stale reads, and active reads after failed mutations. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Sequence Diagram(s)
sequenceDiagram
participant MutationHandler
participant CartCacheSync
participant QueryClient
MutationHandler->>CartCacheSync: synchronise mutation cart
CartCacheSync->>QueryClient: cancel matching cart queries
QueryClient-->>CartCacheSync: cancellation complete
CartCacheSync-->>MutationHandler: synchronisation complete
MutationHandler->>MutationHandler: invoke success callback
Possibly related PRs
- TechsioCZ/new-engine#290: Extends the cart hook and cache synchronisation logic used by this change.
- TechsioCZ/new-engine#351: Shares the cart cache synchronisation and cancellation changes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly describes the main change: preventing stale cart cache data after navigation. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/cart-cache-navigation-race
✨ Simplify code
- Create PR with simplified code
- Commit simplified code in branch
fix/cart-cache-navigation-race
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 help to get the list of available commands.
Greptile SummaryThe PR coordinates cart-query cancellation and cache synchronization to preserve the newest cart state.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| libs/storefront-data/src/cart/hooks.ts | Reorders mutation cache synchronization and adds an ownership guard before cart data fans out to cache aliases. |
| libs/storefront-data/src/shared/cart-cache-sync.ts | Adds a shared helper that silently cancels active and detail queries for a cart. |
| libs/storefront-data/tests/cart.hooks.reactivity.test.tsx | Adds focused regression tests covering mutation ordering and stale cart-read synchronization. |
Sequence Diagram
sequenceDiagram
participant M as Cart mutation
participant Q as QueryClient
participant C as Cart consumers
M->>Q: Start cancelCartCaches
M->>Q: syncCartCaches(new cart)
Q-->>C: Publish current cart
M->>Q: Optionally invalidate caches
Q-->>M: Cancellation settles
Reviews (2): Last reviewed commit: "fix(storefront-data): prevent stale cart..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.github.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
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)
libs/storefront-data/src/cart/hooks.ts (1)
619-648: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winfix(cart): make
syncCartCachesafe forselectconsumers
syncCartCachecompares theselect-transformedcartwith the raw cached query data. A transformingselectcan make this reference check fail while the hook still owns the active cache, which skips cache fan-out and stale query cleanup. Track ownership by aselect-independent signal, such as stored query state metadata, and update the same check inuseSuspenseCart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/storefront-data/src/cart/hooks.ts` around lines 619 - 648, Update syncCartCache and the corresponding ownership check in useSuspenseCart to avoid comparing select-transformed cart references with raw query data; use select-independent query state metadata to confirm the hook still owns the active cache before fan-out and stale-query cleanup. Add or update coverage in libs/storefront-data/tests/cart.hooks.reactivity.test.tsx:309-369 for transformed select consumers, with no direct change required elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/storefront-data/src/cart/hooks.ts`:
- Around line 537-544: Update syncMutationCart to handle cancelCartCaches
failures independently: catch and contain errors from cache cancellation, then
continue running syncCartCaches, invalidateCart, and the caller’s onSuccess flow
even when cancellation rejects. Keep the mutation success result unaffected by
cache-sync failures.
---
Outside diff comments:
In `@libs/storefront-data/src/cart/hooks.ts`:
- Around line 619-648: Update syncCartCache and the corresponding ownership
check in useSuspenseCart to avoid comparing select-transformed cart references
with raw query data; use select-independent query state metadata to confirm the
hook still owns the active cache before fan-out and stale-query cleanup. Add or
update coverage in
libs/storefront-data/tests/cart.hooks.reactivity.test.tsx:309-369 for
transformed select consumers, with no direct change required elsewhere.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4d989c40-dec7-4e8b-846c-9c15b2e3dc58
📒 Files selected for processing (3)
libs/storefront-data/src/cart/hooks.tslibs/storefront-data/src/shared/cart-cache-sync.tslibs/storefront-data/tests/cart.hooks.reactivity.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: main
- GitHub Check: Greptile Review
⚠️ CI failures not shown inline (1)
GitHub Check: Kilo Code Review: Kilo Code Review failed
Conclusion: failure
Review failed: The message could not be delivered
🧰 Additional context used
📓 Path-based instructions (6)
libs/storefront-data/**/*.{ts,tsx}
📄 CodeRabbit inference engine (libs/storefront-data/AGENTS.md)
Do not use
anytype - use proper generics insteadMedusa storefront data layer should use TanStack Query for data fetching in libs/storefront-data
Files:
libs/storefront-data/tests/cart.hooks.reactivity.test.tsxlibs/storefront-data/src/shared/cart-cache-sync.tslibs/storefront-data/src/cart/hooks.ts
libs/**
📄 CodeRabbit inference engine (AGENTS.md)
Use RSLib for building libraries in the monorepo
Files:
libs/storefront-data/tests/cart.hooks.reactivity.test.tsxlibs/storefront-data/src/shared/cart-cache-sync.tslibs/storefront-data/src/cart/hooks.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for running tests in backend and UI library projects
Files:
libs/storefront-data/tests/cart.hooks.reactivity.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome linting and formatting only on changed files using 'bunx biome check --write path/to/file'
Files:
libs/storefront-data/tests/cart.hooks.reactivity.test.tsxlibs/storefront-data/src/shared/cart-cache-sync.tslibs/storefront-data/src/cart/hooks.ts
libs/storefront-data/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (libs/storefront-data/AGENTS.md)
libs/storefront-data/src/**/*.{ts,tsx}: Do not import from./dist/paths - use source files instead
Do not hardcode query keys - usecreateQueryKey()utility instead
Do not mix server and client code in the same file
Use cache strategies fromCacheConfig(static, semiStatic, realtime, userData) for query configuration
Use TanStack Query 5+ for data fetching, caching, and SSR hydration in@techsio/storefront-data
Files:
libs/storefront-data/src/shared/cart-cache-sync.tslibs/storefront-data/src/cart/hooks.ts
libs/storefront-data/src/**/hooks.ts
📄 CodeRabbit inference engine (libs/storefront-data/AGENTS.md)
Always use factory pattern for hooks (e.g.,
createProductHooks,createCollectionHooks, etc.)
Files:
libs/storefront-data/src/cart/hooks.ts
🧠 Learnings (1)
📚 Learning: 2025-12-16T19:45:17.746Z
Learnt from: BleedingDev
Repo: NMIT-WR/new-engine PR: 207
File: libs/ui/src/molecules/select.tsx:50-50
Timestamp: 2025-12-16T19:45:17.746Z
Learning: When reviewing Tailwind classes in TSX/TS files, prefer using square brackets for arbitrary CSS values and complex expressions. Specifically: - Do not use the parentheses syntax (z-(--z-index)) for anything beyond simple CSS variable references; this syntax auto-wraps in var() and cannot handle calc or complex functions. - Use the square brackets syntax (e.g., h-[calc(var(--available-height)-var(--spacing-content))]) for calc expressions, var with calc, and any complex CSS expressions. This rule applies broadly to Tailwind v4 usage in TSX code across the project.
Applied to files:
libs/storefront-data/tests/cart.hooks.reactivity.test.tsx
🔇 Additional comments (5)
libs/storefront-data/src/shared/cart-cache-sync.ts (1)
208-229: LGTM!libs/storefront-data/src/cart/hooks.ts (2)
15-19: LGTM!
762-764: LGTM!Also applies to: 790-791, 824-825, 872-873, 905-906, 934-935, 963-964
libs/storefront-data/tests/cart.hooks.reactivity.test.tsx (2)
371-447: LGTM!
449-519: LGTM!
|
🎉 This PR is included in version 0.30.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
Summary by CodeRabbit