feat: add event bus and cart header badge#835
Merged
tomaszpacior merged 4 commits intomainfrom Mar 24, 2026
Merged
Conversation
- @o2s/ui: mitt event bus (cart:changed), mitt dependency - @o2s/frontend: getCurrentCart SDK, CartInfo badge + session cache - blocks: emit/sync cart:changed on add (list, details) and cart mutations Made-with: Cursor
Contributor
WalkthroughAdds a typed mitt-based event bus at Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProductComponent as Product Component
participant SDK as SDK / API
participant EventBus as Event Bus
participant CartHeader as Cart Header
User->>ProductComponent: Click "Add to Cart"
ProductComponent->>SDK: sdk.cart.addCartItem(...)
SDK-->>ProductComponent: returns updated cart
ProductComponent->>EventBus: emit('cart:changed', { cart: updated })
EventBus-->>CartHeader: delivers 'cart:changed' payload
CartHeader->>CartHeader: update itemCount state and render badge
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 1
🧹 Nitpick comments (3)
packages/ui/src/event-bus/useEventBus.ts (1)
5-12: Consider documenting the handler memoization requirement.The
handlerin the dependency array means consumers must memoize their handlers (viauseCallback) to avoid subscribe/unsubscribe cycles on every render. This is a valid design choice, but it's easy to misuse.Consider either:
- Adding a JSDoc comment documenting this requirement, or
- Wrapping the handler internally with
useRefto maintain a stable reference📝 Option: Internal stable reference pattern
import { useEffect } from 'react'; +import { useRef } from 'react'; import { O2SEventMap, eventBus } from './event-bus'; export function useEventBus<K extends keyof O2SEventMap>(event: K, handler: (payload: O2SEventMap[K]) => void) { + const handlerRef = useRef(handler); + handlerRef.current = handler; + useEffect(() => { - eventBus.on(event, handler); + const stableHandler = (payload: O2SEventMap[K]) => handlerRef.current(payload); + eventBus.on(event, stableHandler); return () => { - eventBus.off(event, handler); + eventBus.off(event, stableHandler); }; - }, [event, handler]); + }, [event]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/event-bus/useEventBus.ts` around lines 5 - 12, The useEventBus hook currently requires consumers to memoize the handler because the handler is included in the useEffect dependency array causing subscribe/unsubscribe on every render; update useEventBus by either adding a JSDoc above the function documenting that the handler must be stable (e.g., "Consumers should wrap handler with useCallback to avoid resubscribe cycles") or implement an internal stable reference pattern: keep a ref for the latest handler (e.g., handlerRef) and use a stable wrapper callback for eventBus.on/off while calling handlerRef.current inside the wrapper; reference the useEventBus function, the handler argument, eventBus.on/eventBus.off, and the useEffect dependency list when applying the change.apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx (1)
18-19: Module-level state: acceptable trade-off, but document the constraint.The
lastKnownCartItemCountvariable prevents badge flicker during SPA navigations, which is a good UX choice. However, this pattern relies on only oneCartInfoinstance being active at a time. If multiple instances ever rendered simultaneously (e.g., during page transitions), they could race on this shared variable.The current
Header.tsxusage withuseMemoensures a single instance, but consider adding a brief comment noting this constraint for future maintainers.📝 Suggested documentation
-/** Last known line-item count for this browser session; survives header remounts on navigation (avoids badge flicker). */ +/** + * Last known line-item count for this browser session; survives header remounts + * on navigation (avoids badge flicker). + * + * Note: This assumes only one CartInfo instance is active at a time. + */ let lastKnownCartItemCount = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx` around lines 18 - 19, Add a brief comment near the module-level variable lastKnownCartItemCount in CartInfo.tsx documenting that this shared state is intentional to prevent badge flicker but assumes only one CartInfo instance is active at a time; mention that Header.tsx currently enforces this via useMemo and call out the potential race if multiple instances are rendered (e.g., during page transitions) so future maintainers know the constraint and can refactor to instance-scoped state if needed.apps/frontend/src/api/modules/cart.ts (1)
9-22: Consider flattening the nestedcart.cartstructure.The current structure requires
cart(internalSdk).cart.getCurrentCart(...), with a redundant.cart.cartchain. Other modules in this codebase use a different namespace (e.g.,page(internalSdk).modules.getPage). For consistency, consider either:
- Using a different namespace like
carts:- Or flattening if only cart-related methods exist here.
♻️ Proposed refactor
export const cart = (sdk: Sdk) => ({ - cart: { - getCurrentCart: (headers: AppHeaders, authorization?: string): Promise<Carts.Model.Cart> => + carts: { + getCurrent: (headers: AppHeaders, authorization?: string): Promise<Carts.Model.Cart> => sdk.makeRequest({ method: 'get', url: `${CARTS_API_URL}/current`, headers: { ...getApiHeaders(), ...headers, ...(authorization ? { Authorization: `Bearer ${authorization}` } : {}), }, }), }, });Then update
sdk.ts:-) => cart(internalSdk).cart.getCurrentCart(headers, authorization); +) => cart(internalSdk).carts.getCurrent(headers, authorization);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/api/modules/cart.ts` around lines 9 - 22, The exported factory function currently creates a nested cart.cart shape; change the API to return methods directly (or rename the factory to carts) so callers use carts(internalSdk).getCurrentCart(...) instead of cart(internalSdk).cart.getCurrentCart(...). Specifically, update the exported symbol (export const cart) to either export const carts = (sdk) => ({ getCurrentCart: (...) => sdk.makeRequest({...}) }) or keep the name and return the methods at the top level (export const cart = (sdk) => ({ getCurrentCart: ... })); then update all call sites and the SDK registry (the SDK construction in sdk.ts) to import/register the new symbol name (carts or flattened cart) so the module namespace is consistent with other modules like page(...).modules.getPage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/blocks/products/product-list/src/frontend/ProductList.client.tsx`:
- Line 76: The call to eventBus.emit in ProductList.client.tsx uses
result.items.data.length without defensive checks; update the payload
computation (the value passed to eventBus.emit('cart:changed', ...)) to use
optional chaining on the response object (e.g., derive itemCount from
result?.items?.data?.length with a fallback like 0) so that eventBus.emit is
always called with a safe numeric itemCount even if result or its nested
properties are undefined.
---
Nitpick comments:
In `@apps/frontend/src/api/modules/cart.ts`:
- Around line 9-22: The exported factory function currently creates a nested
cart.cart shape; change the API to return methods directly (or rename the
factory to carts) so callers use carts(internalSdk).getCurrentCart(...) instead
of cart(internalSdk).cart.getCurrentCart(...). Specifically, update the exported
symbol (export const cart) to either export const carts = (sdk) => ({
getCurrentCart: (...) => sdk.makeRequest({...}) }) or keep the name and return
the methods at the top level (export const cart = (sdk) => ({ getCurrentCart:
... })); then update all call sites and the SDK registry (the SDK construction
in sdk.ts) to import/register the new symbol name (carts or flattened cart) so
the module namespace is consistent with other modules like
page(...).modules.getPage.
In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx`:
- Around line 18-19: Add a brief comment near the module-level variable
lastKnownCartItemCount in CartInfo.tsx documenting that this shared state is
intentional to prevent badge flicker but assumes only one CartInfo instance is
active at a time; mention that Header.tsx currently enforces this via useMemo
and call out the potential race if multiple instances are rendered (e.g., during
page transitions) so future maintainers know the constraint and can refactor to
instance-scoped state if needed.
In `@packages/ui/src/event-bus/useEventBus.ts`:
- Around line 5-12: The useEventBus hook currently requires consumers to memoize
the handler because the handler is included in the useEffect dependency array
causing subscribe/unsubscribe on every render; update useEventBus by either
adding a JSDoc above the function documenting that the handler must be stable
(e.g., "Consumers should wrap handler with useCallback to avoid resubscribe
cycles") or implement an internal stable reference pattern: keep a ref for the
latest handler (e.g., handlerRef) and use a stable wrapper callback for
eventBus.on/off while calling handlerRef.current inside the wrapper; reference
the useEventBus function, the handler argument, eventBus.on/eventBus.off, and
the useEffect dependency list when applying the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45ff9522-868f-4bdd-9463-47f2e16a5f3c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.changeset/bright-icons-nudge.mdapps/frontend/src/api/modules/cart.tsapps/frontend/src/api/sdk.tsapps/frontend/src/containers/Header/CartInfo/CartInfo.tsxpackages/blocks/checkout/cart/src/frontend/Cart.client.tsxpackages/blocks/products/product-details/src/frontend/ProductDetails.client.tsxpackages/blocks/products/product-list/src/frontend/ProductList.client.tsxpackages/ui/package.jsonpackages/ui/src/event-bus/event-bus.tspackages/ui/src/event-bus/index.tspackages/ui/src/event-bus/useEventBus.tspackages/ui/tsconfig.json
Contributor
Coverage Report for packages/configs/vitest-config
File CoverageNo changed files found. |
Replace itemCount with Carts.Model.Cart so consumers can reuse line items and totals. CartInfo badge still uses cart.items.data.length. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx (2)
38-47: Consider defensive property access for API response.The property chain
cart.items.data.lengthassumes the API always returns the expected structure. While TypeScript types may guarantee this at compile time, runtime data from external APIs can diverge. Optional chaining provides a safer fallback.♻️ Suggested defensive access
try { const cart = await sdk.cart.getCurrentCart({ 'x-locale': locale }, token); if (!cancelled) { - const next = cart.items.data.length; + const next = cart?.items?.data?.length ?? 0; lastKnownCartItemCount = next; setItemCount(next); } } catch { // Do not reset to 0 — avoids flicker on client navigations or transient API errors. }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx` around lines 38 - 47, The code assumes cart.items.data.length always exists when calling sdk.cart.getCurrentCart in CartInfo.tsx; change the access to be defensive (e.g., use optional chaining/null coalescing) when reading the count from cart so runtime-missing properties don't throw — locate the call to sdk.cart.getCurrentCart and replace the direct property chain used to compute next (currently cart.items.data.length) with a safe read that falls back to 0 or the previous lastKnownCartItemCount before assigning lastKnownCartItemCount and calling setItemCount.
55-59: Apply the same defensive access for event payload.The event callback accesses the same deep property chain. For consistency and robustness, apply optional chaining here as well.
♻️ Suggested defensive access
const onCartChanged = useCallback((payload: O2SEventMap['cart:changed']) => { - const next = payload.cart.items.data.length; + const next = payload?.cart?.items?.data?.length ?? 0; lastKnownCartItemCount = next; setItemCount(next); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx` around lines 55 - 59, The onCartChanged callback reads a deep property chain without guards: change its access to use defensive optional chaining (e.g., read payload.cart?.items?.data?.length) and default to 0 if missing, then assign that value to lastKnownCartItemCount and pass it to setItemCount; update the function named onCartChanged that handles O2SEventMap['cart:changed'] to compute next safely before updating state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx`:
- Around line 38-47: The code assumes cart.items.data.length always exists when
calling sdk.cart.getCurrentCart in CartInfo.tsx; change the access to be
defensive (e.g., use optional chaining/null coalescing) when reading the count
from cart so runtime-missing properties don't throw — locate the call to
sdk.cart.getCurrentCart and replace the direct property chain used to compute
next (currently cart.items.data.length) with a safe read that falls back to 0 or
the previous lastKnownCartItemCount before assigning lastKnownCartItemCount and
calling setItemCount.
- Around line 55-59: The onCartChanged callback reads a deep property chain
without guards: change its access to use defensive optional chaining (e.g., read
payload.cart?.items?.data?.length) and default to 0 if missing, then assign that
value to lastKnownCartItemCount and pass it to setItemCount; update the function
named onCartChanged that handles O2SEventMap['cart:changed'] to compute next
safely before updating state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd17f52f-1ad2-41f3-a6c2-a87d8094527b
📒 Files selected for processing (5)
apps/frontend/src/containers/Header/CartInfo/CartInfo.tsxpackages/blocks/checkout/cart/src/frontend/Cart.client.tsxpackages/blocks/products/product-details/src/frontend/ProductDetails.client.tsxpackages/blocks/products/product-list/src/frontend/ProductList.client.tsxpackages/ui/src/event-bus/event-bus.ts
✅ Files skipped from review due to trivial changes (1)
- packages/ui/src/event-bus/event-bus.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/blocks/products/product-details/src/frontend/ProductDetails.client.tsx
- packages/blocks/products/product-list/src/frontend/ProductList.client.tsx
- packages/blocks/checkout/cart/src/frontend/Cart.client.tsx
…mmended products - Add GET /carts/:id client helper and expose sdk.cart.getCart for guests (localStorage cartId) - CartInfo: fetch current cart with token or guest cart by id; keep cart:changed subscription - RecommendedProducts: emit cart:changed after successful add-to-cart - Update changeset to include @o2s/blocks.recommended-products and list all emitting blocks Made-with: Cursor
marcinkrasowski
approved these changes
Mar 24, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx (1)
37-39: Useitems.totalfor the badge count.
data.lengthonly counts the loaded array. The cart model already exposesitems.total, which is the safer field for a header badge if the API ever returns a partial items collection (packages/framework/src/modules/carts/carts.model.ts:38-49).♻️ Proposed change
- const next = cart.items.data.length; + const next = cart.items.total; ... - const next = cart.items.data.length; + const next = cart.items.total; ... - const next = payload.cart.items.data.length; + const next = payload.cart.items.total;Also applies to: 59-61, 73-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx` around lines 37 - 39, The badge count uses cart.items.data.length which only reflects the currently loaded page; instead read the overall count from cart.items.total and use that to update lastKnownCartItemCount and call setItemCount. Locate occurrences in CartInfo where cart.items.data.length is used (e.g., the blocks updating lastKnownCartItemCount and setItemCount around the references) and replace with cart.items.total so the header badge shows the full cart total even when only a partial items array is returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/frontend/src/api/modules/cart.ts`:
- Around line 22-25: The getCart call interpolates a client-controlled cartId
directly into the URL, allowing characters like "/", "?", or "#" to alter the
request path; fix this by URL-encoding cartId before using it in the path
(update the getCart implementation to encode cartId with a proper URL-encoding
function such as encodeURIComponent) so the request URL is constructed as
`${CARTS_API_URL}/${encodedCartId}` and the same encoded variable is used for
all places referencing cartId in that function.
In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx`:
- Around line 41-43: The current catch blocks in CartInfo.tsx leave
lastKnownCartItemCount and itemCount unchanged for all errors which can
permanently show a stale badge; update the error handling in both catch sites
(the one that reads the guest CART_ID_KEY and the one at lines 63-65) to
distinguish retryable (network/5xx) errors from terminal errors (404/missing
cart, 401/authorization), and on terminal failures clear lastKnownCartItemCount
and itemCount and, when the failing cart is a guest cart, remove CART_ID_KEY
from storage; for retryable errors retain the previous counts but still
surface/log the error so transient issues can be retried.
---
Nitpick comments:
In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx`:
- Around line 37-39: The badge count uses cart.items.data.length which only
reflects the currently loaded page; instead read the overall count from
cart.items.total and use that to update lastKnownCartItemCount and call
setItemCount. Locate occurrences in CartInfo where cart.items.data.length is
used (e.g., the blocks updating lastKnownCartItemCount and setItemCount around
the references) and replace with cart.items.total so the header badge shows the
full cart total even when only a partial items array is returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06be0278-832a-4024-9220-4c0674d9d646
📒 Files selected for processing (5)
.changeset/bright-icons-nudge.mdapps/frontend/src/api/modules/cart.tsapps/frontend/src/api/sdk.tsapps/frontend/src/containers/Header/CartInfo/CartInfo.tsxpackages/blocks/products/recommended-products/src/frontend/RecommendedProducts.client.tsx
✅ Files skipped from review due to trivial changes (1)
- .changeset/bright-icons-nudge.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/frontend/src/api/sdk.ts
…st cart id in localStorage - Read key from env only (no in-code fallback); set in apps/frontend/.env.development - Use same pattern in CartInfo, product blocks, checkout blocks, and Storybook - Declare variable in turbo.json globalEnv for cache hashing and eslint-plugin-turbo - Extend Storybook main env typing; update changeset notes Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx (1)
41-43:⚠️ Potential issue | 🟠 MajorClear stale badge state on terminal cart fetch failures.
Both catch blocks currently preserve the old count for every failure. If the guest cart id is stale, or the authenticated fetch returns a terminal 401/404, the header can keep showing a positive badge indefinitely. Keep the current behavior for retryable failures, but clear
lastKnownCartItemCount/itemCounton terminal ones and drop the guest cart id from storage when the guest-cart lookup is definitively gone.💡 Suggested fix
- } catch { - // Do not reset to 0 — avoids flicker on client navigations or transient API errors. + } catch (error) { + const err = error as { status?: number; response?: { status?: number } }; + const status = err.status ?? err.response?.status; + if (!cancelled && (status === 401 || status === 404)) { + lastKnownCartItemCount = 0; + setItemCount(0); + } } @@ - } catch { - // Do not reset to 0 — avoids flicker on client navigations or transient API errors. + } catch (error) { + const err = error as { status?: number; response?: { status?: number } }; + const status = err.status ?? err.response?.status; + if (!cancelled && (status === 401 || status === 404)) { + localStorage.removeItem(cartIdLocalStorageKey); + lastKnownCartItemCount = 0; + setItemCount(0); + } }Also applies to: 63-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx` around lines 41 - 43, The catch blocks currently preserve the old cart badge on all failures; update the error handling in the catch after the cart fetches (both the guest-cart lookup and the authenticated fetch) so that on terminal errors (HTTP 401 or 404) you clear lastKnownCartItemCount and itemCount and remove the stored guest cart id (the storage key used for the guest cart id) from storage, while keeping the current behavior for retryable errors—detect the HTTP status on the thrown error (or the fetch response) and only perform the state clears and storage removal when status === 401 || status === 404; apply the same fix to the second catch block referenced (lines 63-65) so both guest and auth failure paths handle terminal errors the same way.
🧹 Nitpick comments (1)
packages/blocks/checkout/checkout-summary/src/frontend/CheckoutSummary.client.tsx (1)
24-24: Validate the cart-storage key once, instead of trimming it inline.
!.trim()still allows whitespace-only config, which would silently route every guest-cart read/write throughlocalStorage['']. Please guard the trimmed value once and reuse that validated constant/helper across the other cart-aware client components touched in this PR.💡 Suggested fix
-const cartIdLocalStorageKey = process.env.NEXT_PUBLIC_CART_ID_LOCAL_STORAGE_KEY!.trim(); +const cartIdLocalStorageKey = process.env.NEXT_PUBLIC_CART_ID_LOCAL_STORAGE_KEY?.trim(); + +if (!cartIdLocalStorageKey) { + throw new Error('NEXT_PUBLIC_CART_ID_LOCAL_STORAGE_KEY must be set to a non-empty value'); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blocks/checkout/checkout-summary/src/frontend/CheckoutSummary.client.tsx` at line 24, The cart-storage key is being trimmed inline which still allows whitespace-only values; instead, validate and normalize the env var once and reuse it: create a validated constant/helper (e.g., cartIdLocalStorageKey) that reads process.env.NEXT_PUBLIC_CART_ID_LOCAL_STORAGE_KEY, trims it, and throws or fails-fast if the trimmed value is empty or missing, then replace the inline usage in CheckoutSummary.client.tsx (and other cart-aware client components touched in this PR) to import/use that validated constant so no component ever reads/writes localStorage[''].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.storybook/preview.tsx:
- Line 23: Replace the unsafe non-null assertion on
process.env.NEXT_PUBLIC_CART_ID_LOCAL_STORAGE_KEY before calling .trim(): check
that process.env.NEXT_PUBLIC_CART_ID_LOCAL_STORAGE_KEY is defined and non-empty,
and if not throw or log a clear error (with the env var name) so Storybook fails
fast with an actionable message; then call .trim() and assign to
cartIdLocalStorageKey. Ensure you remove the `!` non-null assertion and perform
the guard in the top-level initialization where cartIdLocalStorageKey is
defined.
---
Duplicate comments:
In `@apps/frontend/src/containers/Header/CartInfo/CartInfo.tsx`:
- Around line 41-43: The catch blocks currently preserve the old cart badge on
all failures; update the error handling in the catch after the cart fetches
(both the guest-cart lookup and the authenticated fetch) so that on terminal
errors (HTTP 401 or 404) you clear lastKnownCartItemCount and itemCount and
remove the stored guest cart id (the storage key used for the guest cart id)
from storage, while keeping the current behavior for retryable errors—detect the
HTTP status on the thrown error (or the fetch response) and only perform the
state clears and storage removal when status === 401 || status === 404; apply
the same fix to the second catch block referenced (lines 63-65) so both guest
and auth failure paths handle terminal errors the same way.
---
Nitpick comments:
In
`@packages/blocks/checkout/checkout-summary/src/frontend/CheckoutSummary.client.tsx`:
- Line 24: The cart-storage key is being trimmed inline which still allows
whitespace-only values; instead, validate and normalize the env var once and
reuse it: create a validated constant/helper (e.g., cartIdLocalStorageKey) that
reads process.env.NEXT_PUBLIC_CART_ID_LOCAL_STORAGE_KEY, trims it, and throws or
fails-fast if the trimmed value is empty or missing, then replace the inline
usage in CheckoutSummary.client.tsx (and other cart-aware client components
touched in this PR) to import/use that validated constant so no component ever
reads/writes localStorage[''].
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db68f8e1-b966-4177-992f-d90b34e0aefa
📒 Files selected for processing (15)
.changeset/bright-icons-nudge.md.storybook/main.ts.storybook/preview.tsxapps/frontend/.env.developmentapps/frontend/src/containers/Header/CartInfo/CartInfo.tsxpackages/blocks/checkout/cart/src/frontend/Cart.client.stories.tsxpackages/blocks/checkout/cart/src/frontend/Cart.client.tsxpackages/blocks/checkout/checkout-billing-payment/src/frontend/CheckoutBillingPayment.client.tsxpackages/blocks/checkout/checkout-company-data/src/frontend/CheckoutCompanyData.client.tsxpackages/blocks/checkout/checkout-shipping-address/src/frontend/CheckoutShippingAddress.client.tsxpackages/blocks/checkout/checkout-summary/src/frontend/CheckoutSummary.client.tsxpackages/blocks/products/product-details/src/frontend/ProductDetails.client.tsxpackages/blocks/products/product-list/src/frontend/ProductList.client.tsxpackages/blocks/products/recommended-products/src/frontend/RecommendedProducts.client.tsxturbo.json
✅ Files skipped from review due to trivial changes (5)
- apps/frontend/.env.development
- turbo.json
- .storybook/main.ts
- packages/blocks/products/recommended-products/src/frontend/RecommendedProducts.client.tsx
- .changeset/bright-icons-nudge.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/blocks/products/product-list/src/frontend/ProductList.client.tsx
- packages/blocks/checkout/cart/src/frontend/Cart.client.tsx
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
2 participants
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.
@o2s/ui: mitt event bus (cart:changed), mitt dependency
@o2s/frontend: getCurrentCart SDK, CartInfo badge + session cache
blocks: emit/sync cart:changed on add (list, details) and cart mutations
Summary by CodeRabbit
New Features
Chores