feat: my account localization#3354
Open
lemagnetic wants to merge 7 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
💤 Files with no reviewable changes (20)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR migrates My Account pages to CMS-driven section rendering, adds label-based account and order UI components, introduces account page context and CMS fetch helpers, localizes order statuses, updates account page SSR flows, and adds My Account CMS schemas plus related documentation and template updates. ChangesMy Account CMS and runtime migration
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SSR as Account SSR page
participant CMS as fetchMyAccountPageContent
participant GQL as Account GraphQL
participant UI as Layout/RenderSectionsBase
Browser->>SSR: Request /pvt/account/...
SSR->>CMS: Fetch page content
SSR->>GQL: Fetch account data
CMS-->>SSR: Sections + navigation data
GQL-->>SSR: accountPageData
SSR->>UI: Build PageProvider props
UI-->>Browser: Render account sections
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
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)
Comment |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Add 14 Account section component schemas and 6 page content-types with English label defaults. Regenerate schema.json with base.jsonc and document the generate-schema command in README.
Rename Layout, Menu, Drawer, ListOrders, OrderDetails, and shared UI primitives. Update imports in app shell, navbar, and CLI template. Remove profile/i18n.ts; add label helpers and CMS label props on account components; decompose order details for section rendering.
Add accountPageContext, CMS fetch with default sections fallback, 14 Account section wrappers, and ACCOUNT_COMPONENTS map. Extend getMyAccountRoutes with optional navigation label overrides.
Fetch CMS sections in GSSP with locale and silent fallback defaults. Pass accountPageData and navigationLabels via PageProvider and render Account sections through RenderSectionsBase on all wired pages.
Add getLocalizedOrderStatusMap and wire CMS status label overrides through StatusBadge on orders list and order details header.
lemagnetic
force-pushed
the
feat/account-localization
branch
from
b6cd5cf to
b633106
Compare
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
Rename content-type keys to myAccountProfile, myAccountOrders, etc. to match landingPage and globalSections. Regenerate schema.json and update GSSP fetch calls and default sections map.
lemagnetic
requested review from
gabpaladino and
renatamottam
and removed request for
a team
![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: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/core/src/components/account/orders/OrderDetails/DeliveryOptionAccordion/DeliveryOptionAccordionDeliveryInfo.tsx (1)
35-37:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard
addressbefore rendering address fields to avoid null dereference.Current flow can reach the address container with
addressundefined, then crash onaddress.city/street/postalCode.Suggested fix
- <InfoContainer title={isPickup ? 'Store address' : 'Delivery address'}> - <span data-fs-delivery-option-accordion-info> - <strong>{address.city}</strong> - </span> - <span data-fs-delivery-option-accordion-info>{address.street}</span> - <span - data-fs-delivery-option-accordion-info - >{`${address.postalCode}${address.state ? `, ${address.state}` : ''}${address.country ? `, ${address.country}.` : ''}`}</span> - </InfoContainer> + {address && ( + <InfoContainer title={isPickup ? 'Store address' : 'Delivery address'}> + <span data-fs-delivery-option-accordion-info> + <strong>{address.city}</strong> + </span> + <span data-fs-delivery-option-accordion-info>{address.street}</span> + <span + data-fs-delivery-option-accordion-info + >{`${address.postalCode}${address.state ? `, ${address.state}` : ''}${address.country ? `, ${address.country}.` : ''}`}</span> + </InfoContainer> + )}Also applies to: 55-63
🤖 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 `@packages/core/src/components/account/orders/OrderDetails/DeliveryOptionAccordion/DeliveryOptionAccordionDeliveryInfo.tsx` around lines 35 - 37, The component DeliveryOptionAccordionDeliveryInfo can render address fields when address is undefined causing null dereferences; update the guard so address is checked before any address property access—either expand the existing early return (the if that currently checks !address && !contact && !deliveryOption.deliveryWindow) to return when address is missing for address-specific render paths, or wrap the JSX that accesses address.city/address.street/address.postalCode with a conditional (address && ...) so those fields only render when address is defined; apply the same fix to the other address-rendering block referenced around lines 55-63.packages/core/src/components/account/orders/OrderDetails/OrderActionModal/OrderActionModal.tsx (1)
31-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHardcoded default cancelText breaks localization.
The default value
"Not now"is a hardcoded English string. While the modal already receives localizedtitle,message, andconfirmTextvia props, thecancelTextdefault prevents full localization. Consider making this required or accepting labels from the parent component's label resolver.🌐 Recommended fix: Remove default or accept from parent labels
Option 1: Make
cancelTextrequired (since parentOrderActionsalready has label resolver):interface OrderActionModalProps { isOpen: boolean loading?: boolean title: string message: React.ReactNode confirmText: string - cancelText?: string + cancelText: string confirmVariant?: ButtonProps['variant'] danger?: boolean onClose: () => void onConfirm: () => void } export default function OrderActionModal({ isOpen = false, loading = false, title, message, confirmText, - cancelText = 'Not now', + cancelText, confirmVariant = 'primary',Option 2: If optional behavior is needed, use empty string:
- cancelText = 'Not now', + cancelText = '',Then ensure parent (OrderActions) always passes the localized string.
🤖 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 `@packages/core/src/components/account/orders/OrderDetails/OrderActionModal/OrderActionModal.tsx` at line 31, The default English cancelText ("Not now") in OrderActionModal is breaking localization; remove the hardcoded default and require the parent to pass a localized cancelText (or alternatively default to an empty string). Update the OrderActionModal props (cancelText) so it is non-optional or defaults to '' and adjust callers (e.g., OrderActions) to pass their label resolver value into cancelText; ensure references to cancelText in the OrderActionModal render/handlers use the passed-in prop.packages/core/src/components/account/orders/OrderDetails/DeliveryOptionAccordion/DeliveryOptionAccordionProducts.tsx (1)
46-54:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftHardcoded UI strings break localization pattern.
Lines 46, 50, and 54 contain hardcoded English strings (
"Each","Taxes","Total") that cannot be localized via CMS. This violates the PR's core objective of CMS-backed localization for My Account pages. Other components in this PR (e.g.,OrderActions) correctly implement alabelsprop with resolved defaults.🌐 Recommended fix: Add labels prop with resolver pattern
Add a labels interface and resolver following the established pattern:
// In orderDetailsLabels.ts or similar interface DeliveryOptionAccordionProductLabels { priceLabel?: string taxLabel?: string totalLabel?: string } function resolveDeliveryOptionAccordionProductLabels( labels?: DeliveryOptionAccordionProductLabels ): Required<DeliveryOptionAccordionProductLabels> { return { priceLabel: labels?.priceLabel ?? 'Each', taxLabel: labels?.taxLabel ?? 'Taxes', totalLabel: labels?.totalLabel ?? 'Total', } }Then update the component props and usage:
interface DeliveryOptionAccordionProductProps { image: string quantity: number name: string field?: { name: string value: string refId?: string } price: string tax: string total: string + labels?: DeliveryOptionAccordionProductLabels } function DeliveryOptionAccordionProduct({ image, quantity, name, field, price, tax, total, + labels: labelsProp, }: DeliveryOptionAccordionProductProps) { + const labels = resolveDeliveryOptionAccordionProductLabels(labelsProp) + return ( <div data-fs-delivery-option-accordion-product> ... <div data-fs-delivery-option-accordion-product-price> <p>{price}</p> - <span>Each</span> + <span>{labels.priceLabel}</span> </div> <div data-fs-delivery-option-accordion-product-tax> <p>{tax}</p> - <span>Taxes</span> + <span>{labels.taxLabel}</span> </div> <div data-fs-delivery-option-accordion-product-total> <p>{total}</p> - <span>Total</span> + <span>{labels.totalLabel}</span> </div>🤖 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 `@packages/core/src/components/account/orders/OrderDetails/DeliveryOptionAccordion/DeliveryOptionAccordionProducts.tsx` around lines 46 - 54, DeliveryOptionAccordionProducts currently uses hardcoded strings "Each", "Taxes", and "Total"; add a labels prop and resolver following the project's pattern (e.g., define DeliveryOptionAccordionProductLabels interface with priceLabel, taxLabel, totalLabel and a resolveDeliveryOptionAccordionProductLabels function that returns defaults) then update the DeliveryOptionAccordionProducts props to accept labels, call the resolver inside the component, and replace the hardcoded span text with the resolved labels (priceLabel, taxLabel, totalLabel) so the strings can be provided from CMS-backed labels.packages/core/src/components/account/orders/OrderDetails/PaymentCard/PaymentCard.tsx (2)
152-156:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
relto the bank invoice link opened in a new tab.
target="_blank"should includerel="noopener noreferrer"to prevent tabnabbing.Suggested fix
<Link data-fs-payment-invoice-link href={bankInvoiceUrl} target="_blank" + rel="noopener noreferrer" >🤖 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 `@packages/core/src/components/account/orders/OrderDetails/PaymentCard/PaymentCard.tsx` around lines 152 - 156, The Link in the PaymentCard component opens bankInvoiceUrl with target="_blank" and is missing the rel attribute; update the Link (in PaymentCard) that uses href={bankInvoiceUrl} to include rel="noopener noreferrer" (e.g., add rel to the Link element) to prevent tabnabbing while keeping target="_blank".
67-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle missing transactions before iterating bank invoice URLs.
paymentDatais optional, butgetBankInvoiceUrlassumes an array and can throw on undefined. Make the helper acceptundefinedand early-returnnull.Suggested fix
-const getBankInvoiceUrl = (transactions: OrderPaymentDataTransaction[]) => { +const getBankInvoiceUrl = (transactions?: OrderPaymentDataTransaction[]) => { + if (!transactions?.length) { + return null + } + for (const transaction of transactions) {Also applies to: 88-88
🤖 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 `@packages/core/src/components/account/orders/OrderDetails/PaymentCard/PaymentCard.tsx` around lines 67 - 76, getBankInvoiceUrl currently assumes transactions is an array and will throw if undefined; change its signature to accept transactions?: OrderPaymentDataTransaction[] and add an early guard that returns null when transactions is falsy before the for-loops, then keep the existing nested iteration and replacement logic unchanged; update any call sites (the usage around line 88) to allow passing undefined without extra checks.
🟡 Minor comments (5)
packages/core/src/components/account/orders/ListOrders/SelectedTags/SelectedTags.tsx-49-50 (1)
49-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse localized labels for date-range prefixes.
Lines 65-68 still hardcode
to/from, so CMS-drivenfromLabelandtoLabelare ignored in the selected-date tag.💡 Proposed fix
- {(dateInitial && dateFinal - ? `${formattedDateInitial} to ${formattedDateFinal}` - : formattedDateInitial - ? `from ${formattedDateInitial}` - : `to ${formattedDateFinal}`) || ''} + {(dateInitial && dateFinal + ? `${formattedDateInitial} ${labels.toLabel} ${formattedDateFinal}` + : formattedDateInitial + ? `${labels.fromLabel} ${formattedDateInitial}` + : `${labels.toLabel} ${formattedDateFinal}`) || ''}Also applies to: 64-68
🤖 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 `@packages/core/src/components/account/orders/ListOrders/SelectedTags/SelectedTags.tsx` around lines 49 - 50, The selected-date tag is still using hardcoded "from"/"to" strings; update the logic that builds the date-range label (where resolveListOrdersLabels and getStatusDisplayLabels are used and the selected-date tag is constructed around lines 64-68) to use the localized values from the resolved labels (e.g., labels.fromLabel and labels.toLabel or the equivalent properties returned by resolveListOrdersLabels) instead of literal "from"/"to", so CMS-driven fromLabel/toLabel are applied to the displayed tag.packages/core/src/components/account/security/securityLabels.ts-1-20 (1)
1-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete the labels contract for fully localized/security-accessible copy.
SecuritySectionLabelsis missing keys forhidePasswordAriaand toast titles, soSecurityDrawerstill hardcodes some English UI text and cannot expose accurate show/hide action labels.Proposed contract extension
export type SecuritySectionLabels = { @@ showPasswordAria?: string + hidePasswordAria?: string @@ passwordUpdatedToast?: string passwordFailedToast?: string + passwordUpdatedToastTitle?: string + passwordFailedToastTitle?: string } export const defaultSecurityLabels: Required<SecuritySectionLabels> = { @@ showPasswordAria: 'Show Password', + hidePasswordAria: 'Hide Password', @@ passwordUpdatedToast: 'Password updated successfully', passwordFailedToast: 'Failed to set password.', + passwordUpdatedToastTitle: 'Success setting password', + passwordFailedToastTitle: 'Error setting password', }Also applies to: 22-41
🤖 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 `@packages/core/src/components/account/security/securityLabels.ts` around lines 1 - 20, SecuritySectionLabels is missing localization keys for the password hide aria label and separate toast titles, causing hardcoded English in SecurityDrawer; update the SecuritySectionLabels type to include hidePasswordAria, passwordUpdatedToastTitle, and passwordFailedToastTitle (in addition to the existing passwordUpdatedToast and passwordFailedToast), then update any usages in SecurityDrawer and related components to consume these new keys (use hidePasswordAria for the hide action aria-label and the new toast title keys for toast titles).packages/core/src/components/account/security/SecurityDrawer.tsx-158-163 (1)
158-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse state-aware aria-labels on password visibility toggles.
Lines 161 and 189 always announce the same label, but the action changes by state. Use distinct labels for show/hide to keep assistive text accurate.
Proposed fix
- aria-label={labels.showPasswordAria} + aria-label={ + showCurrentPassword + ? labels.hidePasswordAria + : labels.showPasswordAria + } ... - aria-label={labels.showPasswordAria} + aria-label={ + showNewPassword + ? labels.hidePasswordAria + : labels.showPasswordAria + }As per coding guidelines, component code should “use semantic HTML and proper ARIA labels for accessibility”.
Also applies to: 186-190
🤖 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 `@packages/core/src/components/account/security/SecurityDrawer.tsx` around lines 158 - 163, The password visibility IconButton uses a static aria-label; update it to be state-aware so it announces the correct action: change the aria-label on the current-password toggle (IconButton that calls setShowCurrentPassword and reads showCurrentPassword) to use labels.showPasswordAria when the field is hidden and labels.hidePasswordAria when visible, and do the same for the new-password toggle (the IconButton that toggles setShowNewPassword / showNewPassword) so both buttons dynamically choose the show/hide label based on their respective boolean state.Source: Coding guidelines
packages/core/src/components/account/profile/profile.tsx-57-63 (1)
57-63:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard nullable email before rendering the
mailto:link.
UserProfile.emailis nullable, but Line 62 always builds a mailto URL. This can render invalid links (mailto:null/mailto:undefined) when profile data is incomplete.Proposed fix
{ heading: emailLabel, - data: ( - <Link href={`mailto:${profile?.email}`}>{profile?.email}</Link> - ), + data: profile?.email ? ( + <Link href={`mailto:${profile.email}`}>{profile.email}</Link> + ) : ( + '-' + ), },🤖 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 `@packages/core/src/components/account/profile/profile.tsx` around lines 57 - 63, Guard the nullable email before constructing the mailto link in the rows array: inside the Profile component (where rows are built with nameLabel/emailLabel and profile?.email), check profile?.email truthiness and only render <Link href={`mailto:${profile?.email}`}>…</Link> when an actual email exists; otherwise render a safe fallback (plain text, empty string, or a placeholder like "—"). Update the email row so it uses this conditional rendering instead of always building the mailto URL with profile?.email.packages/core/src/components/sections/Account/AccountUnauthorized.tsx-26-26 (1)
26-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the icon alt text localizable.
Line 26 hardcodes
'Shopping Cart'in English, which bypasses CMS-driven localization for this section. Move this text into labels/props with defaults, like the other strings.🤖 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 `@packages/core/src/components/sections/Account/AccountUnauthorized.tsx` at line 26, The icon alt text is hardcoded in the AccountUnauthorized component as titleIcon={{ icon: 'ShoppingCart', alt: 'Shopping Cart' }}; make this localizable by adding a prop/label (e.g., shoppingCartAlt or labels.shoppingCartAlt) with a sensible default and use that value for titleIcon.alt instead of the string literal; follow the pattern used for other strings in this component (add the prop to the component props/defaults and consume it where titleIcon is constructed) so CMS-driven localization can override it.
🧹 Nitpick comments (4)
packages/core/src/components/account/orders/OrderDetails/OrderDetails.tsx (1)
41-43: ⚡ Quick winAvoid forcing GraphQL data with
ascasts in render path.These casts suppress type errors instead of narrowing data and can hide runtime shape mismatches. Prefer explicit guards (or typed mappers) before rendering
DeliveryCard,StatusCard, andDeliveryOptionAccordion.As per coding guidelines, TypeScript files should ensure type safety and avoid type assertions when possible.
Also applies to: 50-52, 67-71
🤖 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 `@packages/core/src/components/account/orders/OrderDetails/OrderDetails.tsx` around lines 41 - 43, The render currently force-casts order.deliveryOptionsData to UserOrderDeliveryOptionsData (used in DeliveryCard, StatusCard, DeliveryOptionAccordion) which hides shape mismatches; replace those inline "as" assertions by validating the data before render — add a type-guard or mapper function (e.g., isUserOrderDeliveryOptionsData or mapToDeliveryOptionsData) that checks required fields on order.deliveryOptionsData and returns a properly typed object or undefined/defaults, then pass the validated/normalized value to DeliveryCard, StatusCard, and DeliveryOptionAccordion (or conditionally render those components only when the guard returns true) to ensure safe, explicit typing instead of using "as".Source: Coding guidelines
packages/core/src/components/sections/Account/AccountOrderDelivery.tsx (1)
30-43: ⚡ Quick winRemove type assertions by narrowing
deliveryOptionsDataand mapped options.
as UserOrderDeliveryOptionsDataandas UserOrderDeliveryOptionweaken type guarantees. Narrow once and pass strongly typed values to keep GraphQL nullability checks enforceable.Suggested refactor
- <DeliveryCard - deliveryOptionsData={ - order.deliveryOptionsData as UserOrderDeliveryOptionsData - } + <DeliveryCard + deliveryOptionsData={order.deliveryOptionsData} fields={ order.customFields?.find((field) => field.type === 'address') ?.fields || [] } title={labels.deliveryTitle} /> - {order.deliveryOptionsData.deliveryOptions.map((option) => ( + {order.deliveryOptionsData.deliveryOptions + .filter( + (option): option is UserOrderDeliveryOption => option != null + ) + .map((option) => ( <DeliveryOptionAccordion key={option.friendlyDeliveryOptionName} - deliveryOption={option as UserOrderDeliveryOption} + deliveryOption={option} contact={order.deliveryOptionsData.contact} currencyCode={order.storePreferencesData.currencyCode} customFields={order.customFields.filter( (field) => field.type === 'item' )} labels={labels} /> - ))} + ))}As per coding guidelines: "TypeScript files: Ensure type safety and avoid type assertions when possible."
🤖 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 `@packages/core/src/components/sections/Account/AccountOrderDelivery.tsx` around lines 30 - 43, Narrow order.deliveryOptionsData once before using it and remove the inline type assertions: add a runtime/compile-time guard (e.g., if (!order.deliveryOptionsData) return null or throw) then assign a local const like deliveryOptionsData = order.deliveryOptionsData and use that variable for the props (deliveryOptionsData, deliveryOptionsData.contact, deliveryOptionsData.deliveryOptions). When mapping, pass each option directly (deliveryOption={option}) instead of using "as UserOrderDeliveryOption" so TypeScript/GraphQL nullability is preserved; update DeliveryOptionAccordion usages to consume the strongly-typed deliveryOptionsData and options.Source: Coding guidelines
packages/core/src/components/sections/Account/AccountUnauthorized.tsx (1)
1-1: ⚡ Quick winAlias
@faststore/uiimports with theUIprefix in component code.Please rename
LinkButtonimport/usage toUILinkButtonto match the component import convention and avoid future local name collisions.As per coding guidelines, imports from
@faststore/uiinpackages/core/src/components/**/*.{ts,tsx}should be aliased with theUIprefix.Suggested diff
-import { LinkButton } from '`@faststore/ui`' +import { LinkButton as UILinkButton } from '`@faststore/ui`' ... - <LinkButton variant="secondary" href="/pvt/account"> + <UILinkButton variant="secondary" href="/pvt/account"> {backLabel} - </LinkButton> + </UILinkButton>Also applies to: 30-32
🤖 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 `@packages/core/src/components/sections/Account/AccountUnauthorized.tsx` at line 1, Rename the imported UI component LinkButton to UILinkButton and update all usages accordingly in the AccountUnauthorized component: change the import statement that currently imports LinkButton from '`@faststore/ui`' to import UILinkButton, then replace any JSX occurrences of <LinkButton ...> with <UILinkButton ...> (and corresponding closing tags or references). Ensure the renamed symbol (UILinkButton) is used consistently in AccountUnauthorized and any nearby exports or helper functions to avoid local name collisions.Source: Coding guidelines
packages/core/src/server/cms/myAccountDefaultSections.ts (1)
54-73: ⚡ Quick winType assertions assume data shapes without validation.
Lines 62 and 70 cast
datafields toRecord<string, string>andRecord<string, unknown>without runtime checks. While the?? {}fallback provides safety when data is missing, if CMS returns data with unexpected types (e.g., nested objects, arrays), these assertions will succeed but cause type mismatches in consuming code.Consider adding runtime validation or using type guards. As per coding guidelines, avoid type assertions when possible.
♻️ Safer approach with validation helper
+function isStringRecord(value: unknown): value is Record<string, string> { + if (typeof value !== 'object' || value === null) return false + return Object.values(value).every(v => typeof v === 'string') +} + export function extractAccountNavigationData< T extends { $componentKey?: string; name: string; data?: unknown }, >(sections: T[]) { const navigationSection = sections.find( (section) => (section.$componentKey ?? section.name) === 'AccountNavigation' ) + const navData = navigationSection?.data ?? {} + const navigationData = isStringRecord(navData) ? navData : {} + return { - navigationData: (navigationSection?.data ?? {}) as Record<string, string>, + navigationData, pageSections: sections .filter( (section) => (section.$componentKey ?? section.name) !== 'AccountNavigation' ) .map((section) => ({ ...section, data: (section.data ?? {}) as Record<string, unknown>, })), } }🤖 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 `@packages/core/src/server/cms/myAccountDefaultSections.ts` around lines 54 - 73, The code in extractAccountNavigationData unguardedly asserts navigationSection?.data to Record<string,string> and section.data to Record<string,unknown>, which can hide unexpected shapes from the CMS; replace these blind type assertions with runtime validation via a small type-guard/validator called from extractAccountNavigationData (e.g. isRecordOfStringToString for navigationData and isRecordOfStringToUnknown for pageSections) that checks typeof keys and values (and falls back to {} when validation fails), and use those guards before assigning to navigationData and before mapping pageSections so consuming code only receives validated objects.Source: Coding guidelines
🤖 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 `@packages/core/src/components/account/components/Accordion/index.ts`:
- Around line 1-4: Change the barrel so the main compound component Accordion is
the default export and subcomponents are named exports: export default Accordion
and export { AccordionButton, AccordionItem, AccordionPanel } (from
'./Accordion', './AccordionButton', './AccordionItem', './AccordionPanel'
respectively); additionally ensure each component file exports its Props types
(add export type { AccordionButtonProps } in AccordionButton.tsx and similarly
AccordionItemProps and AccordionPanelProps) and then re-export those types from
the barrel as named exports (export type { AccordionButtonProps,
AccordionItemProps, AccordionPanelProps }) so all Props types are available from
this index.
In `@packages/core/src/components/account/components/StatusBadge/StatusBadge.tsx`:
- Around line 9-30: Update getStatusVariant to remove the redundant cast and use
a nullish fallback: replace the current casted access with statusMap[status as
OrderStatusKey]?.variant ?? 'neutral' inside getStatusVariant. Then extract the
inline props type for StatusBadge into an exported interface named
StatusBadgeProps and update the StatusBadge function signature to use
StatusBadgeProps. Finally, re-export StatusBadgeProps from the component barrel
(packages/core/src/components/account/components/StatusBadge/index.ts) so
consumers can import the type.
In
`@packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerBody.tsx`:
- Around line 18-24: When computing menuItems, preserve representative-only
entries by always running menuRoutes through getExtraMyAccountRoutes before
passing into getMyAccountRoutes; update the logic around
navigationLabels/menuItems so you call getMyAccountRoutes with routes:
getExtraMyAccountRoutes(menuRoutes) in both cases and only conditionally supply
navigationLabels to getMyAccountRoutes (i.e., do not fall back to raw menuRoutes
when navigationLabels is falsy), referencing the existing symbols menuItems,
navigationLabels, getMyAccountRoutes, getExtraMyAccountRoutes, and menuRoutes.
In `@packages/core/src/components/account/Layout/index.ts`:
- Line 1: The barrel currently only re-exports the default component; you must
also re-export the props type so consumers can access LayoutProps—add a
re-export for the type declared in Layout.tsx (e.g., export type { LayoutProps }
from './Layout') alongside the existing default export to comply with the rule
that all Props and variant types are re-exported from the component barrel.
In `@packages/core/src/components/account/Menu/index.ts`:
- Line 1: The barrel export misses the MenuProps type, so update the export in
index.ts to re-export the component and its props; specifically, add a named
export for MenuProps (the type exported from Menu.tsx) alongside the default
export of Menu so consumers can import { MenuProps } from the barrel; reference
the symbols Menu (default) and MenuProps (type) in your change and ensure the
index.ts re-exports both.
In
`@packages/core/src/components/account/orders/ListOrders/FilterSlider/FilterFacetDateRange/FilterFacetDateRange.tsx`:
- Around line 82-88: The clear() handler resets local state but doesn’t
re-enable the parent apply control; update the clear function (in
FilterFacetDateRange) to call setDisabled(false) after clearing errors and date
values so Apply is no longer stuck disabled — i.e., inside clear() alongside
setInputFromError(undefined), setInputToError(undefined), setDateRange({ from:
'', to: '' }) and clearing inputFromRef/inputToRef, invoke setDisabled(false) to
reset the parent disabled state.
In
`@packages/core/src/components/account/orders/ListOrders/FilterSlider/index.ts`:
- Line 1: The barrel currently only re-exports the default export; add a named
re-export for the props type so consumers can import FilterSliderProps from the
barrel. Update the index to also export the props type from './FilterSlider'
(reference symbol: FilterSliderProps, module: './FilterSlider', existing default
export line) so both the default component and its props type are available from
this barrel.
In
`@packages/core/src/components/account/orders/OrderDetails/BuyingPolicyAlert/BuyingPolicyAlert.tsx`:
- Around line 15-18: The rejection modal still uses hardcoded strings instead of
the passed-in labels, so update the BuyingPolicyAlert component to localize the
modal by extending/using the existing labels contract: add keys for the modal
text (e.g., rejectModalTitle, rejectModalMessage, rejectConfirmText) to the
labels prop interface and replace hardcoded title/message/confirmText in the
reject flow (the modal shown by the handleReject / openRejectModal code path and
the JSX rendering around the reject confirmation) to read from labels with
sensible fallbacks to existing hardcoded strings.
In
`@packages/core/src/components/account/orders/OrderDetails/DeliveryCard/DeliveryCard.tsx`:
- Around line 5-8: The prop deliveryOptionsData on DeliveryCardProps is optional
but is dereferenced with deliveryOptionsData.deliveryOptions.map causing a
runtime crash when absent; update DeliveryCard to either require
deliveryOptionsData in DeliveryCardProps or add a guard/default: check for
deliveryOptionsData (or use optional chaining/default empty array) before
mapping (locations: the DeliveryCard component where
deliveryOptionsData.deliveryOptions.map is called) so the component safely
handles missing data.
In
`@packages/core/src/components/account/orders/OrderDetails/MoreInformationCard/MoreInformationCard.tsx`:
- Line 14: The hardcoded default title in MoreInformationCard prevents
localization; change the component to accept a labels?:
MoreInformationCardLabels prop and use a resolver (e.g.,
resolveMoreInformationCardLabels) to obtain a
Required<MoreInformationCardLabels> and read the title from that instead of the
literal 'More information'; add the MoreInformationCardLabels interface and
resolveMoreInformationCardLabels(labels?) function in the orderDetailsLabels
module returning title: labels?.title ?? 'More information' so callers/CMS can
override the label.
In
`@packages/core/src/components/account/orders/OrderDetails/OrderDetailsHeader.tsx`:
- Around line 32-40: The back control currently nests UIIconButton inside an
anchor which creates invalid interactive nesting; remove the outer <a> and
render a single interactive element by making UIIconButton the anchor (or
equivalent) — e.g., use UIIconButton's href/component/as prop to produce an
anchor with icon={<UIIcon name="ArrowLeft" />}, preserve
data-fs-order-details-header-back-button and aria-label={labels.goBackLabel},
and when rendering as an anchor omit button-only props like type to ensure
correct semantics and keyboard behavior.
In
`@packages/core/src/components/account/orders/OrderDetails/OrderedByCard/index.ts`:
- Line 1: The barrel currently only re-exports the default component; add a
props type re-export so callers can import the prop type from the barrel. Export
the interface/type named OrderedByCardProps from the component file
OrderedByCard.tsx (ensure it is exported, e.g. export type/interface
OrderedByCardProps = ...), then update the barrel index.ts to re-export the
type: export type { OrderedByCardProps } from './OrderedByCard' alongside the
existing default export.
In
`@packages/core/src/components/account/orders/OrderDetails/PaymentCard/index.ts`:
- Line 1: The barrel currently only re-exports the default component; export the
props type as well by adding an exported type in PaymentCard.tsx (e.g., export
type PaymentCardProps = ...) if not already exported, and update the barrel
index.ts to re-export that type (e.g., export { default } from './PaymentCard'
becomes export { default, type PaymentCardProps } from './PaymentCard' or add
export type { PaymentCardProps } from './PaymentCard'); reference symbols:
PaymentCard.tsx and PaymentCardProps and index.ts to ensure the Props type is
re-exported from the component barrel.
In
`@packages/core/src/components/account/orders/OrderDetails/StatusCard/index.ts`:
- Line 1: The barrel currently only re-exports the default component which
prevents consumers from importing the props type; update the barrel (index.ts)
to also export the StatusCardProps type and ensure the props interface is
exported from StatusCard.tsx (export interface StatusCardProps or export type
StatusCardProps) so consumers can import { StatusCard, StatusCardProps } from
the folder entrypoint; keep the default export of the component intact and
export the props type by name from both files.
In
`@packages/core/src/components/account/orders/OrderDetails/SummaryCard/index.ts`:
- Line 1: The barrel currently only re-exports the default component; add an
export for the props type by exporting SummaryCardProps from this index and
ensure the SummaryCard.tsx file exports the props interface (e.g., export
interface SummaryCardProps { ... } or export type SummaryCardProps = ...) so
consumers can import { SummaryCard, SummaryCardProps } from the barrel; update
the index.ts to include SummaryCardProps alongside the default export.
In `@packages/core/src/components/account/UserDetails/UserDetails.tsx`:
- Around line 35-36: The table in UserDetails is using hardcoded strings
("Username", "Phone number"); update the component to use the localized label
keys usernameLabel and phoneLabel from the labels contract/resolver instead: add
those keys to the labels contract/resolver, ensure the resolver returns
usernameLabel and phoneLabel, and replace the literals in the UserDetails
component (where username and phone are used) with the resolved labels (e.g.,
labels.usernameLabel and labels.phoneLabel or via the existing labels hook/prop)
so the rows become { heading: labels.usernameLabel, data: username ? username :
'–' } and { heading: labels.phoneLabel, data: phone ? phone : '–' }.
In `@packages/core/src/pages/pvt/account/403.tsx`:
- Around line 95-123: The 403 page fires CMS and GraphQL requests before running
the redirect guard; move and call getMyAccountRedirect(contentContext, context)
(the same redirect guard used by the other private account pages) before
creating/awaiting the Promise.all that invokes getGlobalSectionsData,
fetchMyAccountPageContent('myAccountUnauthorized', ...), and execute(...), and
if getMyAccountRedirect returns a redirect short-circuit immediately (return the
redirect response) so the CMS/account fetches are not started when FastStore My
Account is disabled.
In `@packages/core/src/pages/pvt/account/security.tsx`:
- Around line 180-184: The code reads security.data?.userDetails.email which
throws if userDetails is nullish; update the userEmail assignment in the
accountPageData object to guard userDetails (e.g., use
security.data?.userDetails?.email ?? '' or an equivalent null-safe check) so
that missing userDetails falls back to an empty string; change the assignment
referenced as accountPageData.userEmail and keep other fields (accountName,
navigationLabels) unchanged.
In `@packages/core/src/sdk/account/accountPageContext.ts`:
- Around line 68-73: The current useAccountPageData function unsafely casts
ctx.accountPageData to T which can hide runtime mismatches; update
useAccountPageData to accept an optional type guard/validator parameter (e.g.,
validator?: (d: AccountPageData) => d is T) and if provided run
validator(ctx.accountPageData) and throw a clear Error when it fails, otherwise
return the data; reference the existing symbols useAccountPageData,
AccountPageContext, and accountPageData and ensure callers are documented to
pass a validator when using a narrower generic type.
In `@packages/core/src/server/cms/myAccountDefaultSections.ts`:
- Around line 44-52: The function withDefaultMyAccountSections currently
force-casts getDefaultMyAccountSections(contentType) to T[] (unsafe) — change
the API to avoid the double cast by tightening the types: either make the
generic T extend DefaultMyAccountSection (e.g., T extends
DefaultMyAccountSection & { ... }) so returned defaults are type-compatible, or
change the function signature to accept/return DefaultMyAccountSection[] (i.e.,
sections: DefaultMyAccountSection[] | undefined | null and return
DefaultMyAccountSection[]) and remove the as unknown as T[] cast; update any
callers that relied on the looser generic to use the concrete
DefaultMyAccountSection type or a compatible constrained generic so the unsafe
assertion is eliminated.
---
Outside diff comments:
In
`@packages/core/src/components/account/orders/OrderDetails/DeliveryOptionAccordion/DeliveryOptionAccordionDeliveryInfo.tsx`:
- Around line 35-37: The component DeliveryOptionAccordionDeliveryInfo can
render address fields when address is undefined causing null dereferences;
update the guard so address is checked before any address property access—either
expand the existing early return (the if that currently checks !address &&
!contact && !deliveryOption.deliveryWindow) to return when address is missing
for address-specific render paths, or wrap the JSX that accesses
address.city/address.street/address.postalCode with a conditional (address &&
...) so those fields only render when address is defined; apply the same fix to
the other address-rendering block referenced around lines 55-63.
In
`@packages/core/src/components/account/orders/OrderDetails/DeliveryOptionAccordion/DeliveryOptionAccordionProducts.tsx`:
- Around line 46-54: DeliveryOptionAccordionProducts currently uses hardcoded
strings "Each", "Taxes", and "Total"; add a labels prop and resolver following
the project's pattern (e.g., define DeliveryOptionAccordionProductLabels
interface with priceLabel, taxLabel, totalLabel and a
resolveDeliveryOptionAccordionProductLabels function that returns defaults) then
update the DeliveryOptionAccordionProducts props to accept labels, call the
resolver inside the component, and replace the hardcoded span text with the
resolved labels (priceLabel, taxLabel, totalLabel) so the strings can be
provided from CMS-backed labels.
In
`@packages/core/src/components/account/orders/OrderDetails/OrderActionModal/OrderActionModal.tsx`:
- Line 31: The default English cancelText ("Not now") in OrderActionModal is
breaking localization; remove the hardcoded default and require the parent to
pass a localized cancelText (or alternatively default to an empty string).
Update the OrderActionModal props (cancelText) so it is non-optional or defaults
to '' and adjust callers (e.g., OrderActions) to pass their label resolver value
into cancelText; ensure references to cancelText in the OrderActionModal
render/handlers use the passed-in prop.
In
`@packages/core/src/components/account/orders/OrderDetails/PaymentCard/PaymentCard.tsx`:
- Around line 152-156: The Link in the PaymentCard component opens
bankInvoiceUrl with target="_blank" and is missing the rel attribute; update the
Link (in PaymentCard) that uses href={bankInvoiceUrl} to include rel="noopener
noreferrer" (e.g., add rel to the Link element) to prevent tabnabbing while
keeping target="_blank".
- Around line 67-76: getBankInvoiceUrl currently assumes transactions is an
array and will throw if undefined; change its signature to accept transactions?:
OrderPaymentDataTransaction[] and add an early guard that returns null when
transactions is falsy before the for-loops, then keep the existing nested
iteration and replacement logic unchanged; update any call sites (the usage
around line 88) to allow passing undefined without extra checks.
---
Minor comments:
In
`@packages/core/src/components/account/orders/ListOrders/SelectedTags/SelectedTags.tsx`:
- Around line 49-50: The selected-date tag is still using hardcoded "from"/"to"
strings; update the logic that builds the date-range label (where
resolveListOrdersLabels and getStatusDisplayLabels are used and the
selected-date tag is constructed around lines 64-68) to use the localized values
from the resolved labels (e.g., labels.fromLabel and labels.toLabel or the
equivalent properties returned by resolveListOrdersLabels) instead of literal
"from"/"to", so CMS-driven fromLabel/toLabel are applied to the displayed tag.
In `@packages/core/src/components/account/profile/profile.tsx`:
- Around line 57-63: Guard the nullable email before constructing the mailto
link in the rows array: inside the Profile component (where rows are built with
nameLabel/emailLabel and profile?.email), check profile?.email truthiness and
only render <Link href={`mailto:${profile?.email}`}>…</Link> when an actual
email exists; otherwise render a safe fallback (plain text, empty string, or a
placeholder like "—"). Update the email row so it uses this conditional
rendering instead of always building the mailto URL with profile?.email.
In `@packages/core/src/components/account/security/SecurityDrawer.tsx`:
- Around line 158-163: The password visibility IconButton uses a static
aria-label; update it to be state-aware so it announces the correct action:
change the aria-label on the current-password toggle (IconButton that calls
setShowCurrentPassword and reads showCurrentPassword) to use
labels.showPasswordAria when the field is hidden and labels.hidePasswordAria
when visible, and do the same for the new-password toggle (the IconButton that
toggles setShowNewPassword / showNewPassword) so both buttons dynamically choose
the show/hide label based on their respective boolean state.
In `@packages/core/src/components/account/security/securityLabels.ts`:
- Around line 1-20: SecuritySectionLabels is missing localization keys for the
password hide aria label and separate toast titles, causing hardcoded English in
SecurityDrawer; update the SecuritySectionLabels type to include
hidePasswordAria, passwordUpdatedToastTitle, and passwordFailedToastTitle (in
addition to the existing passwordUpdatedToast and passwordFailedToast), then
update any usages in SecurityDrawer and related components to consume these new
keys (use hidePasswordAria for the hide action aria-label and the new toast
title keys for toast titles).
In `@packages/core/src/components/sections/Account/AccountUnauthorized.tsx`:
- Line 26: The icon alt text is hardcoded in the AccountUnauthorized component
as titleIcon={{ icon: 'ShoppingCart', alt: 'Shopping Cart' }}; make this
localizable by adding a prop/label (e.g., shoppingCartAlt or
labels.shoppingCartAlt) with a sensible default and use that value for
titleIcon.alt instead of the string literal; follow the pattern used for other
strings in this component (add the prop to the component props/defaults and
consume it where titleIcon is constructed) so CMS-driven localization can
override it.
---
Nitpick comments:
In `@packages/core/src/components/account/orders/OrderDetails/OrderDetails.tsx`:
- Around line 41-43: The render currently force-casts order.deliveryOptionsData
to UserOrderDeliveryOptionsData (used in DeliveryCard, StatusCard,
DeliveryOptionAccordion) which hides shape mismatches; replace those inline "as"
assertions by validating the data before render — add a type-guard or mapper
function (e.g., isUserOrderDeliveryOptionsData or mapToDeliveryOptionsData) that
checks required fields on order.deliveryOptionsData and returns a properly typed
object or undefined/defaults, then pass the validated/normalized value to
DeliveryCard, StatusCard, and DeliveryOptionAccordion (or conditionally render
those components only when the guard returns true) to ensure safe, explicit
typing instead of using "as".
In `@packages/core/src/components/sections/Account/AccountOrderDelivery.tsx`:
- Around line 30-43: Narrow order.deliveryOptionsData once before using it and
remove the inline type assertions: add a runtime/compile-time guard (e.g., if
(!order.deliveryOptionsData) return null or throw) then assign a local const
like deliveryOptionsData = order.deliveryOptionsData and use that variable for
the props (deliveryOptionsData, deliveryOptionsData.contact,
deliveryOptionsData.deliveryOptions). When mapping, pass each option directly
(deliveryOption={option}) instead of using "as UserOrderDeliveryOption" so
TypeScript/GraphQL nullability is preserved; update DeliveryOptionAccordion
usages to consume the strongly-typed deliveryOptionsData and options.
In `@packages/core/src/components/sections/Account/AccountUnauthorized.tsx`:
- Line 1: Rename the imported UI component LinkButton to UILinkButton and update
all usages accordingly in the AccountUnauthorized component: change the import
statement that currently imports LinkButton from '`@faststore/ui`' to import
UILinkButton, then replace any JSX occurrences of <LinkButton ...> with
<UILinkButton ...> (and corresponding closing tags or references). Ensure the
renamed symbol (UILinkButton) is used consistently in AccountUnauthorized and
any nearby exports or helper functions to avoid local name collisions.
In `@packages/core/src/server/cms/myAccountDefaultSections.ts`:
- Around line 54-73: The code in extractAccountNavigationData unguardedly
asserts navigationSection?.data to Record<string,string> and section.data to
Record<string,unknown>, which can hide unexpected shapes from the CMS; replace
these blind type assertions with runtime validation via a small
type-guard/validator called from extractAccountNavigationData (e.g.
isRecordOfStringToString for navigationData and isRecordOfStringToUnknown for
pageSections) that checks typeof keys and values (and falls back to {} when
validation fails), and use those guards before assigning to navigationData and
before mapping pageSections so consuming code only receives validated objects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+1
to
+4
| export { default as Accordion } from './Accordion' | ||
| export { default as AccordionButton } from './AccordionButton' | ||
| export { default as AccordionItem } from './AccordionItem' | ||
| export { default as AccordionPanel } from './AccordionPanel' |
Contributor
There was a problem hiding this comment.
Compound component barrel exports violate guidelines.
Two issues:
- The main component
Accordionmust be the default export from the barrel, not a named export - All Props types (
AccordionButtonProps,AccordionItemProps,AccordionPanelProps) must be re-exported from the barrel
As per coding guidelines for compound components: "The main component is the default export. Sub-components and types are named exports" and "All Props and variant types MUST be re-exported from the barrel index.ts."
🔧 Proposed fix
-export { default as Accordion } from './Accordion'
+export { default } from './Accordion'
+export { default as Accordion } from './Accordion'
export { default as AccordionButton } from './AccordionButton'
export { default as AccordionItem } from './AccordionItem'
export { default as AccordionPanel } from './AccordionPanel'
+export type { AccordionButtonProps } from './AccordionButton'
+export type { AccordionItemProps } from './AccordionItem'
+export type { AccordionPanelProps } from './AccordionPanel'Note: First export the Props types from each component file, then add the type re-exports here. Example for AccordionButton.tsx:
// In AccordionButton.tsx, after the interface definition
export type { AccordionButtonProps }Apply similarly to AccordionItem.tsx and AccordionPanel.tsx.
🤖 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 `@packages/core/src/components/account/components/Accordion/index.ts` around
lines 1 - 4, Change the barrel so the main compound component Accordion is the
default export and subcomponents are named exports: export default Accordion and
export { AccordionButton, AccordionItem, AccordionPanel } (from './Accordion',
'./AccordionButton', './AccordionItem', './AccordionPanel' respectively);
additionally ensure each component file exports its Props types (add export type
{ AccordionButtonProps } in AccordionButton.tsx and similarly AccordionItemProps
and AccordionPanelProps) and then re-export those types from the barrel as named
exports (export type { AccordionButtonProps, AccordionItemProps,
AccordionPanelProps }) so all Props types are available from this index.
Source: Coding guidelines
Comment on lines
+9
to
+30
| function getStatusVariant({ | ||
| status, | ||
| statusMap, | ||
| }: { | ||
| status: string | ||
| statusMap: Record<OrderStatusKey, OrderStatusMapValue> | ||
| }) { | ||
| return ( | ||
| (statusMap[status as OrderStatusKey] as OrderStatusMapValue)?.variant || | ||
| 'neutral' | ||
| ) | ||
| } | ||
|
|
||
| function StatusBadge({ | ||
| status, | ||
| statusFallback, | ||
| statusLabels, | ||
| }: { | ||
| status: string | ||
| statusFallback?: string | ||
| statusLabels?: OrderStatusCmsLabels | ||
| }) { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo_root="$(pwd)"
target_file="packages/core/src/components/account/components/StatusBadge/StatusBadge.tsx"
target_index="packages/core/src/components/account/components/StatusBadge/index.ts"
echo "=== File: $target_file ==="
if [ -f "$target_file" ]; then
nl -ba "$target_file" | sed -n '1,120p'
else
echo "Missing: $target_file"
fi
echo
echo "=== File: $target_index ==="
if [ -f "$target_index" ]; then
nl -ba "$target_index" | sed -n '1,120p'
else
echo "Missing: $target_index"
fi
echo
echo "=== Search for StatusBadgeProps usage ==="
rg -n "StatusBadgeProps" packages/core/src/components/account/components/StatusBadge || true
echo
echo "=== Search for getStatusVariant implementation context ==="
rg -n "function getStatusVariant" packages/core/src/components/account/components/StatusBadge/StatusBadge.tsx || trueRepository: vtex/faststore
Length of output: 1950
Improve TypeScript safety and props typing in StatusBadge
- In
getStatusVariant, the(statusMap[status as OrderStatusKey] as OrderStatusMapValue)cast is redundant; remove theas OrderStatusMapValueand use a nullish fallback (e.g.,statusMap[status as OrderStatusKey]?.variant ?? 'neutral'). - Replace the inline props object with an exported
StatusBadgePropsinterface and ensureStatusBadgePropsis re-exported frompackages/core/src/components/account/components/StatusBadge/index.ts.
🤖 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 `@packages/core/src/components/account/components/StatusBadge/StatusBadge.tsx`
around lines 9 - 30, Update getStatusVariant to remove the redundant cast and
use a nullish fallback: replace the current casted access with statusMap[status
as OrderStatusKey]?.variant ?? 'neutral' inside getStatusVariant. Then extract
the inline props type for StatusBadge into an exported interface named
StatusBadgeProps and update the StatusBadge function signature to use
StatusBadgeProps. Finally, re-export StatusBadgeProps from the component barrel
(packages/core/src/components/account/components/StatusBadge/index.ts) so
consumers can import the type.
Source: Coding guidelines
Comment on lines
+18
to
+24
| const menuItems = navigationLabels | ||
| ? getMyAccountRoutes({ | ||
| routes: getExtraMyAccountRoutes(menuRoutes), | ||
| labels: navigationLabels, | ||
| }) | ||
| : menuRoutes | ||
|
|
Contributor
There was a problem hiding this comment.
Preserve representative routes in the no-label fallback path.
When navigationLabels is falsy, menuItems falls back to menuRoutes, which bypasses getExtraMyAccountRoutes(...). Representative-only entries can disappear in fallback scenarios.
Proposed fix
- const menuItems = navigationLabels
- ? getMyAccountRoutes({
- routes: getExtraMyAccountRoutes(menuRoutes),
- labels: navigationLabels,
- })
- : menuRoutes
+ const baseRoutes = getExtraMyAccountRoutes(menuRoutes)
+
+ const menuItems = navigationLabels
+ ? getMyAccountRoutes({
+ routes: baseRoutes,
+ labels: navigationLabels,
+ })
+ : baseRoutes🤖 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
`@packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerBody.tsx`
around lines 18 - 24, When computing menuItems, preserve representative-only
entries by always running menuRoutes through getExtraMyAccountRoutes before
passing into getMyAccountRoutes; update the logic around
navigationLabels/menuItems so you call getMyAccountRoutes with routes:
getExtraMyAccountRoutes(menuRoutes) in both cases and only conditionally supply
navigationLabels to getMyAccountRoutes (i.e., do not fall back to raw menuRoutes
when navigationLabels is falsy), referencing the existing symbols menuItems,
navigationLabels, getMyAccountRoutes, getExtraMyAccountRoutes, and menuRoutes.
| @@ -0,0 +1 @@ | |||
| export { default } from './Layout' | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Re-export LayoutProps from this barrel
LayoutProps is declared in Layout.tsx but not exported here, so consumers importing from the component barrel lose access to its public props type.
Suggested fix
export { default } from './Layout'
+export type { LayoutProps } from './Layout'As per coding guidelines, "All Props and variant types MUST be re-exported from the barrel index.ts."
🤖 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 `@packages/core/src/components/account/Layout/index.ts` at line 1, The barrel
currently only re-exports the default component; you must also re-export the
props type so consumers can access LayoutProps—add a re-export for the type
declared in Layout.tsx (e.g., export type { LayoutProps } from './Layout')
alongside the existing default export to comply with the rule that all Props and
variant types are re-exported from the component barrel.
Source: Coding guidelines
| @@ -0,0 +1 @@ | |||
| export { default } from './Menu' | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Re-export MenuProps from this barrel
MenuProps is public in Menu.tsx but omitted here, which makes barrel imports incomplete.
Suggested fix
export { default } from './Menu'
+export type { MenuProps } from './Menu'As per coding guidelines, "All Props and variant types MUST be re-exported from the barrel index.ts."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export { default } from './Menu' | |
| export { default } from './Menu' | |
| export type { MenuProps } from './Menu' |
🤖 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 `@packages/core/src/components/account/Menu/index.ts` at line 1, The barrel
export misses the MenuProps type, so update the export in index.ts to re-export
the component and its props; specifically, add a named export for MenuProps (the
type exported from Menu.tsx) alongside the default export of Menu so consumers
can import { MenuProps } from the barrel; reference the symbols Menu (default)
and MenuProps (type) in your change and ensure the index.ts re-exports both.
Source: Coding guidelines
Comment on lines
35
to
36
| { heading: 'Username', data: username ? username : '–' }, | ||
| { heading: 'Phone number', data: phone ? phone : '–' }, |
Contributor
There was a problem hiding this comment.
Unlocalized labels still present in the table
Username and Phone number are still hardcoded, so this section won’t be fully CMS/localized in non-English stores. Add usernameLabel and phoneLabel to the labels contract/resolver and render those values here.
Suggested local render update
- { heading: 'Username', data: username ? username : '–' },
- { heading: 'Phone number', data: phone ? phone : '–' },
+ { heading: labels.usernameLabel, data: username ? username : '–' },
+ { heading: labels.phoneLabel, data: phone ? phone : '–' },🤖 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 `@packages/core/src/components/account/UserDetails/UserDetails.tsx` around
lines 35 - 36, The table in UserDetails is using hardcoded strings ("Username",
"Phone number"); update the component to use the localized label keys
usernameLabel and phoneLabel from the labels contract/resolver instead: add
those keys to the labels contract/resolver, ensure the resolver returns
usernameLabel and phoneLabel, and replace the literals in the UserDetails
component (where username and phone are used) with the resolved labels (e.g.,
labels.usernameLabel and labels.phoneLabel or via the existing labels hook/prop)
so the rows become { heading: labels.usernameLabel, data: username ? username :
'–' } and { heading: labels.phoneLabel, data: phone ? phone : '–' }.
Comment on lines
95
to
+123
| const [ | ||
| globalSectionsPromise, | ||
| globalSectionsHeaderPromise, | ||
| globalSectionsFooterPromise, | ||
| ] = getGlobalSectionsData(contentContext) | ||
|
|
||
| const [account, globalSections, globalSectionsHeader, globalSectionsFooter] = | ||
| await Promise.all([ | ||
| execute< | ||
| ServerAccountPageQueryQueryVariables, | ||
| ServerAccountPageQueryQuery | ||
| >( | ||
| { | ||
| variables: {}, | ||
| operation: query, | ||
| }, | ||
| { headers: { ...context.req.headers } } | ||
| ), | ||
| globalSectionsPromise, | ||
| globalSectionsHeaderPromise, | ||
| globalSectionsFooterPromise, | ||
| ]) | ||
| const [ | ||
| pageContent, | ||
| account, | ||
| globalSections, | ||
| globalSectionsHeader, | ||
| globalSectionsFooter, | ||
| ] = await Promise.all([ | ||
| fetchMyAccountPageContent( | ||
| 'myAccountUnauthorized', | ||
| contentContext, | ||
| '/pvt/account/403' | ||
| ), | ||
| execute<ServerAccountPageQueryQueryVariables, ServerAccountPageQueryQuery>( | ||
| { | ||
| variables: {}, | ||
| operation: query, | ||
| }, | ||
| { headers: { ...context.req.headers } } | ||
| ), | ||
| globalSectionsPromise, | ||
| globalSectionsHeaderPromise, | ||
| globalSectionsFooterPromise, | ||
| ]) |
Contributor
There was a problem hiding this comment.
Gate the 403 page before firing CMS and GraphQL requests.
getMyAccountRedirect(...) runs after the Promise.all(...) here, unlike the other private account pages in this PR. When FastStore My Account is disabled, this page still fetches CMS/account data and can render the new 403 flow instead of short-circuiting to the configured redirect.
Suggested fix
const contentContext = {
previewData: context.previewData,
locale: context.locale,
}
+ const { isFaststoreMyAccountEnabled, redirect } = getMyAccountRedirect({
+ query: context.query,
+ })
+
+ if (!isFaststoreMyAccountEnabled) {
+ return { redirect }
+ }
+
const [
globalSectionsPromise,
globalSectionsHeaderPromise,
globalSectionsFooterPromise,
] = getGlobalSectionsData(contentContext)
@@
- const { isFaststoreMyAccountEnabled, redirect } = getMyAccountRedirect({
- query: context.query,
- })
-
- if (!isFaststoreMyAccountEnabled) {
- return { redirect }
- }
-
return {Based on the other account pages in this PR, the redirect guard is intended to short-circuit before CMS/account fetching.
Also applies to: 154-160
🤖 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 `@packages/core/src/pages/pvt/account/403.tsx` around lines 95 - 123, The 403
page fires CMS and GraphQL requests before running the redirect guard; move and
call getMyAccountRedirect(contentContext, context) (the same redirect guard used
by the other private account pages) before creating/awaiting the Promise.all
that invokes getGlobalSectionsData,
fetchMyAccountPageContent('myAccountUnauthorized', ...), and execute(...), and
if getMyAccountRedirect returns a redirect short-circuit immediately (return the
redirect response) so the CMS/account fetches are not started when FastStore My
Account is disabled.
Comment on lines
+180
to
+184
| accountName: security.data.accountProfile.name ?? '', | ||
| navigationLabels: navigationData as AccountNavigationLabels, | ||
| accountPageData: { | ||
| userEmail: security.data?.userDetails.email || '', | ||
| }, |
Contributor
There was a problem hiding this comment.
Guard userDetails before reading email.
security.data?.userDetails.email || '' still throws when userDetails is nullish, so a partial GraphQL payload turns into a 500 during SSR instead of falling back to ''.
Suggested fix
accountPageData: {
- userEmail: security.data?.userDetails.email || '',
+ userEmail: security.data?.userDetails?.email ?? '',
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| accountName: security.data.accountProfile.name ?? '', | |
| navigationLabels: navigationData as AccountNavigationLabels, | |
| accountPageData: { | |
| userEmail: security.data?.userDetails.email || '', | |
| }, | |
| accountName: security.data.accountProfile.name ?? '', | |
| navigationLabels: navigationData as AccountNavigationLabels, | |
| accountPageData: { | |
| userEmail: security.data?.userDetails?.email ?? '', | |
| }, |
🤖 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 `@packages/core/src/pages/pvt/account/security.tsx` around lines 180 - 184, The
code reads security.data?.userDetails.email which throws if userDetails is
nullish; update the userEmail assignment in the accountPageData object to guard
userDetails (e.g., use security.data?.userDetails?.email ?? '' or an equivalent
null-safe check) so that missing userDetails falls back to an empty string;
change the assignment referenced as accountPageData.userEmail and keep other
fields (accountName, navigationLabels) unchanged.
Comment on lines
+68
to
+73
| export function useAccountPageData< | ||
| T extends AccountPageData = AccountPageData, | ||
| >(): T { | ||
| const ctx = usePage<AccountPageContext>() | ||
| return ctx.accountPageData as T | ||
| } |
Contributor
There was a problem hiding this comment.
Type assertion bypasses runtime safety.
The as T cast assumes the caller's generic parameter matches the actual accountPageData shape, but there's no runtime validation. If a component calls useAccountPageData<AccountOrderDetailsPageData>() on a profile page, TypeScript won't catch the mismatch, leading to potential undefined-field access.
Consider adding a type guard or at least documenting that callers must ensure the correct type is used. As per coding guidelines, avoid type assertions when possible.
♻️ Safer alternative with documentation
+/**
+ * Returns account page data with optional type narrowing.
+ * ⚠️ The generic parameter T is not validated at runtime.
+ * Ensure the correct type is used based on the current page context.
+ */
export function useAccountPageData<
T extends AccountPageData = AccountPageData,
>(): T {
const ctx = usePage<AccountPageContext>()
return ctx.accountPageData as T
}Or for a more defensive approach, you could add a runtime check (higher effort):
export function useAccountPageData<
T extends AccountPageData = AccountPageData,
>(validator?: (data: AccountPageData) => data is T): T {
const ctx = usePage<AccountPageContext>()
if (validator && !validator(ctx.accountPageData)) {
throw new Error('Account page data does not match expected type')
}
return ctx.accountPageData as T
}🤖 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 `@packages/core/src/sdk/account/accountPageContext.ts` around lines 68 - 73,
The current useAccountPageData function unsafely casts ctx.accountPageData to T
which can hide runtime mismatches; update useAccountPageData to accept an
optional type guard/validator parameter (e.g., validator?: (d: AccountPageData)
=> d is T) and if provided run validator(ctx.accountPageData) and throw a clear
Error when it fails, otherwise return the data; reference the existing symbols
useAccountPageData, AccountPageContext, and accountPageData and ensure callers
are documented to pass a validator when using a narrower generic type.
Source: Coding guidelines
Comment on lines
+44
to
+52
| export function withDefaultMyAccountSections< | ||
| T extends { $componentKey?: string; name: string; data?: unknown }, | ||
| >(contentType: MyAccountContentType, sections: T[] | undefined | null): T[] { | ||
| if (sections && sections.length > 0) { | ||
| return sections | ||
| } | ||
|
|
||
| return getDefaultMyAccountSections(contentType) as unknown as T[] | ||
| } |
Contributor
There was a problem hiding this comment.
Unsafe double type cast bypasses all safety checks.
Line 51 uses as unknown as T[] to force-cast DefaultMyAccountSection[] to T[]. This assumes the default sections match the caller's expected type T, but there's no guarantee. If T has additional required properties or a different data shape, this will cause runtime errors.
Consider tightening the generic constraint or returning a more specific type. As per coding guidelines, avoid type assertions when possible.
♻️ Potential improvements
Option 1 - Return the base type explicitly:
export function withDefaultMyAccountSections<
T extends { $componentKey?: string; name: string; data?: unknown },
->(contentType: MyAccountContentType, sections: T[] | undefined | null): T[] {
+>(
+ contentType: MyAccountContentType,
+ sections: T[] | undefined | null
+): T[] | DefaultMyAccountSection[] {
if (sections && sections.length > 0) {
return sections
}
- return getDefaultMyAccountSections(contentType) as unknown as T[]
+ return getDefaultMyAccountSections(contentType)
}Option 2 - Use a stricter return type:
export function withDefaultMyAccountSections(
contentType: MyAccountContentType,
sections: DefaultMyAccountSection[] | undefined | null
): DefaultMyAccountSection[] {
if (sections && sections.length > 0) {
return sections
}
return getDefaultMyAccountSections(contentType)
}🤖 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 `@packages/core/src/server/cms/myAccountDefaultSections.ts` around lines 44 -
52, The function withDefaultMyAccountSections currently force-casts
getDefaultMyAccountSections(contentType) to T[] (unsafe) — change the API to
avoid the double cast by tightening the types: either make the generic T extend
DefaultMyAccountSection (e.g., T extends DefaultMyAccountSection & { ... }) so
returned defaults are type-compatible, or change the function signature to
accept/return DefaultMyAccountSection[] (i.e., sections:
DefaultMyAccountSection[] | undefined | null and return
DefaultMyAccountSection[]) and remove the as unknown as T[] cast; update any
callers that relied on the looser generic to use the concrete
DefaultMyAccountSection type or a compatible constrained generic so the unsafe
assertion is eliminated.
Source: Coding guidelines
This was referenced Jun 11, 2026
Move the 14 My Account component schemas and 6 content-types out of
cms/faststore/{components,pages} into cms/faststore/my-account/ — a sibling
folder that does not scan — and regenerate
schema.json without them.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's the purpose of this pull request?
My Account pages were hardcoded in English with no way for merchants to customize UI copy or adapt labels per locale through the Content Platform. This PR introduces CMS-backed content-types and section components for all My Account routes, so merchants can localize labels, reorder sections, and extend pages without code changes — while keeping a safe fallback when no CMS content is published.
Jira: SFS-3227
How it works?
CMS layer
myAccountProfile,myAccountOrders,myAccountOrderDetails,myAccountUserDetails,myAccountSecurity,myAccountUnauthorized.packages/core/cms/faststore/components/(e.g.AccountNavigation,AccountProfile,AccountOrdersList, order-details sections, etc.).schema.jsonand page JSONC definitions for Content Platform upload.Storefront integration
fetchMyAccountPageContent, with automatic fallback to default sections when CMS is empty or unavailable (myAccountDefaultSections.ts).packages/core/src/components/sections/Account/map CMSsection.datato UI label props; TypeScript defaults apply when fields are empty.AccountNavigationlabels are extracted from CMS and passed throughPageProvider/accountPageContextto the sidebar, drawer, and menu.accountPageData— CMS only controls UI copy and section composition.getLocalizedOrderStatusMap/StatusBadge.Component cleanup
MyAccount*components to shorter names (Layout,ListOrders, etc.) for consistency with the new section architecture.packages/core/src/components/cms/account/Components.tsforRenderSectionsBase.Fallback behavior (no Admin setup required)
If content-types or sections are not created/published in Admin, pages still render with hardcoded default sections and English label defaults. CMS customization is opt-in.
How to test it?
Preview: https://brandless.fast.store/
/pvt/account/profile/pvt/account/orders/pvt/account/orders/{orderId}(open an existing order)/pvt/account/user-details/pvt/account/securitybrandless.faststore@<version>via@vtex/cli-plugin-content.pageTitle) and confirm it reflects on the storefront./pvt/account/403renders withAccountUnauthorizedlabels.Starters Deploy Preview
TBD — add CodeSandbox / starter.store deploy preview link from the PR checks once available.
References
Summary by CodeRabbit
Release Notes
New Features
Improvements