feat(core): localized product URLs, breadcrumbs and hreflang for PDP#3352
Merged
hellofanny merged 29 commits intoJun 19, 2026
Conversation
hellofanny
requested review from
lariciamota and
renatomaurovtex
and removed request for
a team
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds VTEX Catalog dataplane client with localized typings and getLocalizedProduct, request-scoped translations cache, GraphQL otherLocales and locale-aware breadcrumb/slug validation, frontend LocalizedProductContext/provider, hreflang injection on product pages, UI/hook changes for PDP-aware locale routing, and tests. ChangesMulti-locale Product Support
Sequence Diagram(s)sequenceDiagram
participant User
participant PDP_Page
participant GraphQL_Server
participant Catalog_Dataplane
participant Request_Cache
participant LocalizedProductContext
participant LocalizationButton
User->>PDP_Page: request product page
PDP_Page->>GraphQL_Server: query product + otherLocales
GraphQL_Server->>Request_Cache: lookup productId:locale
alt cache miss
GraphQL_Server->>Catalog_Dataplane: getLocalizedProduct(productId, locale)
Catalog_Dataplane-->>GraphQL_Server: LocalizedProductResponse
GraphQL_Server->>Request_Cache: store productId:locale ⟹ entry
else cache hit
Request_Cache-->>GraphQL_Server: return entry
end
GraphQL_Server-->>PDP_Page: product + otherLocales
PDP_Page->>LocalizedProductContext: provide otherLocales
User->>LocalizationButton: open locale selector
LocalizationButton->>LocalizedProductContext: read otherLocales
LocalizationButton->>User: redirect to /binding-url/{localized-slug}/p
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. |
![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: 3
🧹 Nitpick comments (4)
packages/api/src/platforms/vtex/resolvers/query.ts (1)
101-129: ⚖️ Poor tradeoffConsider extracting shared cache lookup/populate logic.
The cache lookup and populate pattern (lines 110-121) is duplicated across
query.ts,breadcrumbList, andotherLocalesresolvers. A shared helper would reduce duplication and ensure consistent behavior.♻️ Proposed helper signature
// In a shared utility file async function getLocalizedProductEntry( ctx: GraphqlContext, productId: string, locale: string ): Promise<LocalizedProductEntry | null> { const cacheKey = `${productId}:${locale}` let entry = ctx.storage.productTranslationsCache?.get(cacheKey) if (!entry) { try { const result = await ctx.clients.catalog.getLocalizedProduct(productId, locale) entry = { linkId: result.linkId, category: result.category } ctx.storage.productTranslationsCache ??= new Map() ctx.storage.productTranslationsCache.set(cacheKey, entry) } catch { return null } } return entry }This is lower priority since the current implementation works correctly and the duplication is contained within the localization feature.
🤖 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/api/src/platforms/vtex/resolvers/query.ts` around lines 101 - 129, Extract the duplicated cache lookup/populate logic into a shared helper (e.g., getLocalizedProductEntry) and replace the inline blocks in query.ts (the block around productGroupID/slugPrefix), breadcrumbList resolver, and otherLocales resolver to call it; the helper should accept (ctx, productId, locale), compute cacheKey `${productId}:${locale}`, read ctx.storage.productTranslationsCache, call ctx.clients.catalog.getLocalizedProduct(productId, locale) inside a try/catch to build and store { linkId, category } on miss, return the entry or null on error, and each caller should use the returned entry (null-safe) instead of duplicating the logic.packages/api/src/platforms/vtex/resolvers/product.ts (2)
97-99: ⚡ Quick winExtract duplicated localization check into a helper.
The pattern
(ctx.discoveryConfig as any)?.localization?.enabled === trueappears twice. Extract to improve readability and type safety.♻️ Proposed helper
Add a helper function at the top of the file or in a shared utility:
const isLocalizationEnabled = (ctx: GraphqlContext): boolean => (ctx.discoveryConfig as { localization?: { enabled?: boolean } })?.localization?.enabled === true const getConfiguredLocales = (ctx: GraphqlContext): string[] => Object.keys( (ctx.discoveryConfig as { localization?: { locales?: Record<string, unknown> } })?.localization?.locales ?? {} )Then use in both resolvers:
- const isLocalizationEnabled = - (ctx.discoveryConfig as any)?.localization?.enabled === true + const localizationEnabled = isLocalizationEnabled(ctx)This also narrows the
as anyassertions to more specific types.Also applies to: 248-249
🤖 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/api/src/platforms/vtex/resolvers/product.ts` around lines 97 - 99, Extract the duplicated localization check into a typed helper and replace the inline checks: add a helper like isLocalizationEnabled(ctx: GraphqlContext): boolean that returns (ctx.discoveryConfig as { localization?: { enabled?: boolean } })?.localization?.enabled === true and a helper getConfiguredLocales(ctx: GraphqlContext): string[] that returns Object.keys((ctx.discoveryConfig as { localization?: { locales?: Record<string, unknown> } })?.localization?.locales ?? {}); then replace the two inline occurrences of (ctx.discoveryConfig as any)?.localization?.enabled === true and any direct locale-keys access with calls to isLocalizationEnabled(ctx) and getConfiguredLocales(ctx) in the product resolver (and the other occurrence noted) to remove as any and improve readability/type-safety.
262-283: ⚖️ Poor tradeoffConcurrent API calls scale with locale count.
Promise.allfires one API call per configured locale simultaneously. For stores with many locales, this could cause rate limiting or latency spikes. The architecture note mentions a future all-locales endpoint optimization which would address this.Consider adding a note or TODO comment to track this optimization.
🤖 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/api/src/platforms/vtex/resolvers/product.ts` around lines 262 - 283, The Promise.all over configuredLocales launches one ctx.clients.catalog.getLocalizedProduct(productId, locale) call per locale which can cause rate limiting/latency spikes for many locales; add a clear TODO comment above the mapped Promise.all (referencing configuredLocales, getLocalizedProduct, productTranslationsCache and productId) noting to replace the per-locale calls with a single all-locales endpoint or batched/rate-limited requests in a future optimization and include expected behavior and ticket/issue ID if available.packages/api/src/platforms/vtex/clients/catalog/index.ts (1)
35-48: ⚡ Quick winURL-encode
productIdto prevent malformed URLs.If
productIdcontains unexpected characters, the URL could break or behave unexpectedly. Encoding ensures robustness.♻️ Proposed fix
getLocalizedProduct: ( productId: string, locale: string ): Promise<LocalizedProductResponse> => fetchAPI( - `${base}/api/catalog-dataplane/product/${productId}?an=${account}`, + `${base}/api/catalog-dataplane/product/${encodeURIComponent(productId)}?an=${encodeURIComponent(account)}`, { method: 'GET', headers: { 'Accept-Language': locale, - 'Content-Type': 'application/json', }, } ),Also removed
Content-Typeheader since GET requests have no body.🤖 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/api/src/platforms/vtex/clients/catalog/index.ts` around lines 35 - 48, The getLocalizedProduct implementation should URL-encode the productId before interpolating into the fetch URL and omit the unnecessary 'Content-Type' header for the GET request; update the call that constructs `${base}/api/catalog-dataplane/product/${productId}?an=${account}` to encode productId (e.g., via encodeURIComponent) and remove 'Content-Type' from the headers passed to fetchAPI so only 'Accept-Language' is sent.
🤖 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/api/src/platforms/vtex/clients/catalog/index.ts`:
- Around line 24-28: The CatalogDataplane function references the Options type
but it isn't imported, causing TypeScript to fail; add a type-only import for
Options (e.g., add "import type { Options } from
'<module-that-exports-Options>';" at the top of the file) so the signature
"Pick<Options, 'account' | 'environment'>" is valid, ensure you point the import
to the module that actually exports Options and run the TypeScript build to
confirm compilation succeeds.
In `@packages/core/src/pages/`[slug]/p.tsx:
- Around line 153-161: The x-default hreflang currently uses the global
baseStoreUrl/storeConfig value which can point to the wrong host for
multi-binding setups; update the href when pushing into hreflangLinks to use the
default locale's binding URL from defaultEntry (e.g., use the defaultEntry's
binding/storeUrl field) instead of baseStoreUrl, so the x-default points to the
actual URL bound to defaultLocale; change the href construction in the
hreflangLinks.push call that references defaultEntry to use that defaultEntry
binding URL plus `/${defaultEntry.slug}/p`.
In `@packages/core/src/sdk/localization/useBindingSelector.ts`:
- Around line 203-211: The current redirect preserves the PDP slug when no
localized `entry` exists, which can produce invalid product URLs in the target
binding; update the redirect logic in useBindingSelector (the block handling
`entry` and the fallback that calls `buildRedirectUrl`) so that when `entry` is
undefined and the current path matches a PDP pattern (e.g., contains a product
slug or `/p` segment), you strip the PDP path and redirect to the binding's base
URL (preserving only search and hash) instead of preserving the full page path;
adjust the call sites around `entry`, `binding`, `baseUrl`, and
`buildRedirectUrl` to detect PDP paths and use the binding root URL
(`binding.url` normalized) plus `window.location.search`/`hash` for the
redirect.
---
Nitpick comments:
In `@packages/api/src/platforms/vtex/clients/catalog/index.ts`:
- Around line 35-48: The getLocalizedProduct implementation should URL-encode
the productId before interpolating into the fetch URL and omit the unnecessary
'Content-Type' header for the GET request; update the call that constructs
`${base}/api/catalog-dataplane/product/${productId}?an=${account}` to encode
productId (e.g., via encodeURIComponent) and remove 'Content-Type' from the
headers passed to fetchAPI so only 'Accept-Language' is sent.
In `@packages/api/src/platforms/vtex/resolvers/product.ts`:
- Around line 97-99: Extract the duplicated localization check into a typed
helper and replace the inline checks: add a helper like
isLocalizationEnabled(ctx: GraphqlContext): boolean that returns
(ctx.discoveryConfig as { localization?: { enabled?: boolean }
})?.localization?.enabled === true and a helper getConfiguredLocales(ctx:
GraphqlContext): string[] that returns Object.keys((ctx.discoveryConfig as {
localization?: { locales?: Record<string, unknown> } })?.localization?.locales
?? {}); then replace the two inline occurrences of (ctx.discoveryConfig as
any)?.localization?.enabled === true and any direct locale-keys access with
calls to isLocalizationEnabled(ctx) and getConfiguredLocales(ctx) in the product
resolver (and the other occurrence noted) to remove as any and improve
readability/type-safety.
- Around line 262-283: The Promise.all over configuredLocales launches one
ctx.clients.catalog.getLocalizedProduct(productId, locale) call per locale which
can cause rate limiting/latency spikes for many locales; add a clear TODO
comment above the mapped Promise.all (referencing configuredLocales,
getLocalizedProduct, productTranslationsCache and productId) noting to replace
the per-locale calls with a single all-locales endpoint or batched/rate-limited
requests in a future optimization and include expected behavior and ticket/issue
ID if available.
In `@packages/api/src/platforms/vtex/resolvers/query.ts`:
- Around line 101-129: Extract the duplicated cache lookup/populate logic into a
shared helper (e.g., getLocalizedProductEntry) and replace the inline blocks in
query.ts (the block around productGroupID/slugPrefix), breadcrumbList resolver,
and otherLocales resolver to call it; the helper should accept (ctx, productId,
locale), compute cacheKey `${productId}:${locale}`, read
ctx.storage.productTranslationsCache, call
ctx.clients.catalog.getLocalizedProduct(productId, locale) inside a try/catch to
build and store { linkId, category } on miss, return the entry or null on error,
and each caller should use the returned entry (null-safe) instead of duplicating
the logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 384c81e1-2f4a-4063-8f0e-b634b7537049
📒 Files selected for processing (12)
packages/api/src/platforms/vtex/clients/catalog/index.tspackages/api/src/platforms/vtex/clients/index.tspackages/api/src/platforms/vtex/index.tspackages/api/src/platforms/vtex/resolvers/product.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/api/src/platforms/vtex/typeDefs/product.graphqlpackages/core/src/components/ui/Breadcrumb/Breadcrumb.tsxpackages/core/src/components/ui/LocalizationButton/LocalizationButton.tsxpackages/core/src/experimental/index.tspackages/core/src/pages/[slug]/p.tsxpackages/core/src/sdk/localization/LocalizedProductContext.tsxpackages/core/src/sdk/localization/useBindingSelector.ts
hellofanny
changed the title
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
hellofanny
commented
May 28, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/api/src/platforms/vtex/clients/catalog/index.ts (1)
32-32:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUnresolved TS type import blocker for
OptionsLine 32 references
Options, but this file does not import that type, so this still breaks TypeScript compilation.Suggested fix
import { fetchAPI } from '../fetch' +import type { Options } from '../..'#!/bin/bash # Verify whether Options is imported in this file and where it is exported. set -euo pipefail echo "1) Check usage/import in target file" rg -n -C2 '\bOptions\b|import type \{ Options \}' packages/api/src/platforms/vtex/clients/catalog/index.ts echo echo "2) Locate exported Options type under VTEX platform module" rg -n -C2 'export (type|interface) Options\b' packages/api/src/platforms/vtex echo echo "3) Confirm catalog client currently lacks Options import" rg -n -C2 '^import' packages/api/src/platforms/vtex/clients/catalog/index.tsAs 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/api/src/platforms/vtex/clients/catalog/index.ts` at line 32, The function signature for CatalogDataplane references the missing TypeScript type Options but this file lacks an import; locate the module that exports the VTEX Options type (search for "export type Options" or "export interface Options" under the vtex platform) and add an explicit type-only import (import type { Options } from '...') at the top of this file so CatalogDataplane({ account, environment }: Options) compiles correctly; ensure the import uses the exact exported name and update any tooling comments if necessary.
🧹 Nitpick comments (1)
packages/api/src/platforms/vtex/resolvers/product.ts (1)
129-135: ⚡ Quick winUse a
Setfor category ID membership in breadcrumb filteringLine 134 currently does repeated
includeslookups on an array. SwitchingmainTreeIdsto aSetis cleaner and faster in resolver hot paths.Proposed refactor
- const mainTreeIds = removeTrailingSlashes(categoriesIds[mainTreeIndex]) + const mainTreeIds = new Set( + removeTrailingSlashes(categoriesIds[mainTreeIndex]) .split('/') .filter(Boolean) + ) const localizedCategories = entry.categories - .filter((category) => mainTreeIds.includes(category.id.toString())) + .filter((category) => mainTreeIds.has(category.id.toString()))As per coding guidelines, “packages/api/src/**: Consider performance implications of resolvers”.
🤖 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/api/src/platforms/vtex/resolvers/product.ts` around lines 129 - 135, Replace the array membership checks on mainTreeIds with a Set to avoid repeated O(n) includes in the breadcrumb filter: after computing mainTreeIds (from removeTrailingSlashes(...).split('/').filter(Boolean)) create a new Set (e.g. mainTreeIdSet) and then use mainTreeIdSet.has(category.id.toString()) inside the localizedCategories filter over entry.categories; update references to mainTreeIds in this resolver (the localizedCategories filter and any subsequent uses) to use the Set for faster lookups.
🤖 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.
Duplicate comments:
In `@packages/api/src/platforms/vtex/clients/catalog/index.ts`:
- Line 32: The function signature for CatalogDataplane references the missing
TypeScript type Options but this file lacks an import; locate the module that
exports the VTEX Options type (search for "export type Options" or "export
interface Options" under the vtex platform) and add an explicit type-only import
(import type { Options } from '...') at the top of this file so
CatalogDataplane({ account, environment }: Options) compiles correctly; ensure
the import uses the exact exported name and update any tooling comments if
necessary.
---
Nitpick comments:
In `@packages/api/src/platforms/vtex/resolvers/product.ts`:
- Around line 129-135: Replace the array membership checks on mainTreeIds with a
Set to avoid repeated O(n) includes in the breadcrumb filter: after computing
mainTreeIds (from removeTrailingSlashes(...).split('/').filter(Boolean)) create
a new Set (e.g. mainTreeIdSet) and then use
mainTreeIdSet.has(category.id.toString()) inside the localizedCategories filter
over entry.categories; update references to mainTreeIds in this resolver (the
localizedCategories filter and any subsequent uses) to use the Set for faster
lookups.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 28fbec94-8dbb-4cf4-b096-1d134824d741
📒 Files selected for processing (4)
packages/api/src/platforms/vtex/clients/catalog/index.tspackages/api/src/platforms/vtex/resolvers/product.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/core/src/sdk/localization/useBindingSelector.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/api/src/platforms/vtex/resolvers/query.ts
LocalizationButton lives in the Navbar (global section) uses useSafePDP to have product (localized slugs) on non-PDP pages
instead of splitting fullPathUriName
hellofanny
force-pushed
the
feat/localized-product-links
branch
from
618aec5 to
f2eee11
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/api/src/platforms/vtex/resolvers/product.ts`:
- Around line 292-307: The resolver forwards ctx.storage.locale directly to
ctx.clients.catalog.getLocalizedProduct (used in the otherLocales flow), which
can be missing/invalid and cause a catch returning null; before calling
getLocalizedProduct validate/guard locale (use (ctx.storage.locale) &&
isSupportedLocale(...) or fallback to
(ctx.discoveryConfig?.localization?.defaultLocale)) and only call
getLocalizedProduct when the resolved locale is truthy/valid; ensure the
cacheKey creation and ctx.storage.productTranslationsCache access use the
sanitized locale value so you don't populate the cache with invalid keys and
preserve alternates for PDP responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06167fb8-8a61-43f1-b398-c5330a16dc60
⛔ Files ignored due to path filters (1)
packages/api/src/__generated__/schema.tsis excluded by!**/__generated__/**and included bypackages/**
📒 Files selected for processing (14)
packages/api/src/platforms/vtex/clients/catalog/index.tspackages/api/src/platforms/vtex/clients/index.tspackages/api/src/platforms/vtex/index.tspackages/api/src/platforms/vtex/resolvers/product.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/api/src/platforms/vtex/typeDefs/product.graphqlpackages/core/src/components/sections/EmptyState/EmptyState.tsxpackages/core/src/components/ui/Breadcrumb/Breadcrumb.tsxpackages/core/src/components/ui/LocalizationButton/LocalizationButton.tsxpackages/core/src/experimental/index.tspackages/core/src/pages/[slug]/p.tsxpackages/core/src/sdk/localization/LocalizedProductContext.tsxpackages/core/src/sdk/localization/useBindingSelector.tspackages/core/test/sdk/localization/useBindingSelector.test.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/core/src/experimental/index.ts
- packages/core/src/components/ui/LocalizationButton/LocalizationButton.tsx
- packages/api/src/platforms/vtex/clients/index.ts
- packages/api/src/platforms/vtex/index.ts
- packages/core/src/components/ui/Breadcrumb/Breadcrumb.tsx
- packages/api/src/platforms/vtex/typeDefs/product.graphql
- packages/api/src/platforms/vtex/resolvers/query.ts
- packages/api/src/platforms/vtex/clients/catalog/index.ts
- packages/core/src/sdk/localization/LocalizedProductContext.tsx
- packages/core/src/pages/[slug]/p.tsx
Comment on lines
+97
to
+166
| const isLocalizationEnabled = | ||
| (ctx.discoveryConfig as any)?.localization?.enabled === true | ||
| const locale = ctx.storage.locale | ||
|
|
||
| if (isLocalizationEnabled && locale) { | ||
| // productTranslationsCache is request-scoped and shared with the slug and otherLocales | ||
| // resolvers — if any of them already called getLocalizedProduct for this product+locale, | ||
| // we reuse the result here at zero extra cost. | ||
| const cacheKey = `${productId}:${locale}` | ||
| let entry = ctx.storage.productTranslationsCache?.get(cacheKey) | ||
|
|
||
| if (!entry) { | ||
| try { | ||
| const result = await ctx.clients.catalog.getLocalizedProduct( | ||
| productId, | ||
| locale | ||
| ) | ||
| // Store both linkId (for the product item URL) and the full categories array | ||
| // (for per-level localized slugs). We intentionally keep categories[] rather than | ||
| // just the leaf category so we never need to reconstruct the hierarchy via split('/'). | ||
| entry = { | ||
| linkId: result.linkId, | ||
| categories: result.categories ?? [], | ||
| availableLinkIds: result.availableLinkIds ?? {}, | ||
| } | ||
| ctx.storage.productTranslationsCache ??= new Map() | ||
| ctx.storage.productTranslationsCache.set(cacheKey, entry) | ||
| } catch { | ||
| // Catalog Dataplane API unavailable — fall through to IS-based behavior below | ||
| } | ||
| } | ||
|
|
||
| if (entry) { | ||
| // Extract the category IDs that belong to the main tree (same tree chosen from IS above). | ||
| // A product can be registered in multiple trees; Catalog Dataplane returns all of them | ||
| // in categories[], so we filter to only the ones matching this tree's IDs. | ||
| const mainTreeIds = removeTrailingSlashes(categoriesIds[mainTreeIndex]) | ||
| .split('/') | ||
| .filter(Boolean) | ||
|
|
||
| const localizedCategories = entry.categories | ||
| .filter((category) => mainTreeIds.includes(category.id.toString())) | ||
| .sort( | ||
| (a, b) => | ||
| a.fullPath.split('/').length - b.fullPath.split('/').length | ||
| ) | ||
|
|
||
| // Length guard: if Catalog Dataplane returns fewer categories than IS expects | ||
| // (e.g. data inconsistency or empty categories), fall through to the IS fallback. | ||
| if (localizedCategories.length === splittedCategories.length) { | ||
| return { | ||
| itemListElement: [ | ||
| // Category items: both name and slug come from Catalog Dataplane, ensuring | ||
| // they are always consistent with each other for the requested locale. | ||
| ...localizedCategories.map((category, index) => ({ | ||
| name: category.name, | ||
| item: `/${category.fullPathUriName}/`, | ||
| position: index + 1, | ||
| })), | ||
| { | ||
| name: productName, | ||
| item: getPath(entry.linkId, itemId), | ||
| position: splittedCategories.length + 1, | ||
| }, | ||
| ], | ||
| numberOfItems: splittedCategories.length, | ||
| } | ||
| } | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
Do you think it's worth moving this block of code into a new function to keep the resolver cleaner and more explicit in its intention?
Contributor
Author
There was a problem hiding this comment.
This one I think it was a little bit tricker and confusing while extracting it, likely reduce readability. (AI said is better to keep this way hahah)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/pages/[slug]/p.tsx (1)
47-52:⚠️ Potential issue | 🟡 MinorRemove unsafe type assertions on localization config.
The
as Record<string, any>andas | string | undefinedcasts drop type information from the SEO helper. ExtendStoreConfigto include the localization shape solocalesanddefaultLocaleflow with proper types.♻️ Type the localization config
type StoreConfig = typeof storeConfig & { experimental: { revalidate?: number enableClientOffer?: boolean } + localization?: { + enabled?: boolean + defaultLocale?: string + locales?: Record< + string, + { bindings?: Array<{ url?: string | null }> } + > + } } @@ - const locales = storeConfig.localization.locales as Record<string, any> + const locales = storeConfig.localization.locales const baseStoreUrl = storeConfig.storeUrl.replace(/\/$/, '') - const defaultLocale = storeConfig.localization.defaultLocale as - | string - | undefined + const defaultLocale = storeConfig.localization.defaultLocaleApplies to lines 96–100 where the casts are used. Per coding guidelines, 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/pages/`[slug]/p.tsx around lines 47 - 52, The StoreConfig type definition is missing type information for the localization configuration (locales and defaultLocale fields), which forces unsafe type assertions elsewhere in the code. Extend the StoreConfig type at its definition to include the localization shape with properly typed locales and defaultLocale properties, rather than leaving them untyped and requiring as Record<string, any> and as string | undefined assertions when these fields are used on lines 96-100. This will allow the localization config to flow through with proper type safety without needing unsafe type casts.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/api/src/platforms/vtex/resolvers/query.ts`:
- Around line 67-73: Add validation of the `locale` parameter against
configured/supported locales before the call to
`catalog.getLocalizedProduct(productGroupID, locale)` in this query resolver.
Implement a guard that checks if the locale value is in the list of supported
locales before proceeding with the platform call. If the locale is invalid,
either return an error or fall back to a default locale. This validation should
occur before or at the beginning of the try block to prevent unsupported user
input from triggering unnecessary platform calls to the Catalog Dataplane.
---
Outside diff comments:
In `@packages/core/src/pages/`[slug]/p.tsx:
- Around line 47-52: The StoreConfig type definition is missing type information
for the localization configuration (locales and defaultLocale fields), which
forces unsafe type assertions elsewhere in the code. Extend the StoreConfig type
at its definition to include the localization shape with properly typed locales
and defaultLocale properties, rather than leaving them untyped and requiring as
Record<string, any> and as string | undefined assertions when these fields are
used on lines 96-100. This will allow the localization config to flow through
with proper type safety without needing unsafe type casts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6803b9ef-0f56-4180-94ea-294cfe6cd6e9
⛔ Files ignored due to path filters (2)
packages/core/@generated/gql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (4)
packages/api/src/platforms/vtex/clients/catalog/index.tspackages/api/src/platforms/vtex/resolvers/product.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/core/src/pages/[slug]/p.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/api/src/platforms/vtex/resolvers/product.ts
- packages/api/src/platforms/vtex/clients/catalog/index.ts
hellofanny
commented
Jun 17, 2026
Member
There was a problem hiding this comment.
- After changing to the Italian version, if I focused out of the browser and then focused in, the selector went back to the other locale:
- hreflang - maybe I misconfigure it, but only brings 1 and the
<link rel="alternate" hreflang="x-default" href="https://homebrewqa.fast.store/roshe-tenis-80/p" data-next-head="">edit: the 2) looks good now.
9 tasks
Contributor
Author
The first 1 I couldn't reproduced it. Did you tried to to remove the browser data and tried again? |
eduardoformiga
approved these changes
Jun 19, 2026
hellofanny
force-pushed
the
feat/localized-product-links
branch
2 times, most recently
from
954989a to
1775602
Compare
|
Contributor
Author
I believe it's an existing bug, I will treat this bug later, ok? |
hellofanny
merged commit 92bc71f
into
feat/localized-slugs-links
19 of 21 checks passed
hellofanny
added a commit
that referenced
this pull request
Jun 19, 2026
…3352) ## Summary Implements localized product links for FastStore stores with localization.enabled: true (PDP only — PLP localized slugs are handled separately). **it-IT** <img width="1907" height="1001" alt="image" src="https://github.com/user-attachments/assets/b78831db-edbd-4db3-96bb-0c67b4b6c20b" /> **pt-BR** <img width="1918" height="992" alt="image" src="https://github.com/user-attachments/assets/f357c607-a37d-41e2-b13a-abf25de2629d" /> ### Problem FastStore stores with localization enabled had three major issues on the PDP: 1. Localized product URLs (e.g. `/it-IT/adidas-polo-uomo-65/p`) returned 404 because the `Query.product` resolver's `startsWith` guard rejected slugs whose prefix didn't match the Intelligent Search `linkText` (always in the default locale). 2. Breadcrumb items were rendered as non-clickable `<span>` elements and `BreadcrumbJsonLd` was suppressed entirely. 3. The `LocalizationSelector` had no localized product URL to navigate to when the shopper switched locale. 4. After landing on a 404 from a locale switch, the LocalizationSelector lost the product context and couldn't recover the correct localized URL — and the 404 page showed the bare asPath instead of the full locale-prefixed URL. ### Changes @faststore/api - New clients/catalog/ client — uses the public catalog-dataplane/product/{id} endpoint (Accept-Language header, no AppKey/AppToken required) to fetch localized product data (linkId, category.fullPathUriName) per locale - Query.product resolver: when startsWith fails and localization is enabled, validates the slug prefix against the - - Catalog Dataplane linkId for the current locale before rejecting - New StoreProduct.otherLocales: [StoreProductLocale!] field — fires Promise.all over all configured locales to build localized slug entries; zero overhead for non-localized stores - StoreProduct.breadcrumbList resolver: now async; uses category.fullPathUriName from the Catalog Dataplane for localized category URL paths and linkId for the product URL; falls back to slugify(IS name) when no translation exists - productTranslationsCache in GraphqlContext.storage — request-scoped cache keyed by productId:locale shared across all three resolvers to avoid duplicate API calls @faststore/core - New `LocalizedProductContext` — React context that provides otherLocales from the PDP GraphQL response to - any component in the tree (including global components like LocalizationButton) - `useLocalizedProduct_unstable` exported from experimental/index.ts for store customization - `LocalizationButton` / `useBindingSelector`: uses otherLocales to redirect to the correct localized product URL on locale switch; falls back to the defaultLocale slug when no translation exists for the target locale - `useBindingSelector`: persists otherLocales to sessionStorage keyed by SKU id (fs:otherLocales:{skuId}) so the locale selector can recover the correct product URL even after the user lands on a 404 (where the page no longer provides otherLocales) - `Breadcrumb.tsx`: removed localization.enabled workaround — always renders <Link> now that localized URLs are available -`p.tsx`: restored BreadcrumbJsonLd for all locales + added hreflang alternate tags via NextSeo.additionalLinkTags (including x-default pointing to the root store URL) - `EmptyState`: the fromUrl shown on the 404 page now includes the active Next.js locale prefix (e.g. /it-IT/...) so shoppers see the full URL they attempted to visit ### Architecture note The Catalog Dataplane is called once per product+locale combination. For otherLocales (all locales) and breadcrumbList (current locale only), results are shared via productTranslationsCache. A future optimization — replacing the Promise.all with a single "all-locales" endpoint from the Catalog team — would be a contained 2-file change with no behavior impact. The sessionStorage persistence in `useBindingSelector` uses a fs:otherLocales:{skuId} key and is intentionally session-scoped (cleared on tab close). It is a best-effort fallback; the locale selector always prefers live otherLocales from context when available. ## Test plan try using `brandless` acc - [ ] Visit a localized PDP (e.g. `/it-IT/{localized-slug}-{skuId}/p`) — page renders correctly eg. /adidas-mens-performance-polo-blast-blue-65/p -> switch to it-IT locale! You will be able to see the slug localized. - [ ] Visit a non-localized locale PDP (e.g. `/pt-BR/{default-slug}-{skuId}/p`) — page renders correctly (fallback path) eg. select en-CA - [ ] PDP breadcrumb items are clickable `<Link>` elements with localized category URLs - [ ] 2-level category hierarchy breadcrumb builds correct intermediate paths - [ ] `LocalizationSelector` navigates to the correct localized product URL for each locale - [ ] `LocalizationSelector` falls back to the default locale slug for locales without a translation - [ ] `hreflang` tags appear in `<head>` for all configured locales + `x-default` (lists only the `availableLinkIds`) <img width="1086" height="181" alt="Screenshot 2026-06-11 at 16 33 23" src="https://github.com/user-attachments/assets/e1dc0ed0-3332-492b-9a1d-84a33c3a1eb4" /> <img width="498" height="178" alt="Screenshot 2026-06-11 at 16 37 36" src="https://github.com/user-attachments/assets/6f52bb39-9211-4ece-8462-14e0841d6115" /> - [ ] `BreadcrumbJsonLd` appears in page source for all locales - [ ] Non-localized stores (`localization.enabled: false`) — no regression, no extra API calls - [ ] Switch locale on a PDP → land on a 404 → LocalizationSelector still shows/navigates to correct localized URLs for all locales - [ ] 404 page fromUrl displays the full locale-prefixed path (e.g. /it-IT/some-product/p) [Spec file for more information](vtex/faststore-dx-spec-kit#15) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit * **New Features** * Product pages now expose `otherLocales` via a dedicated localization context/provider. * Product pages generate multi-locale SEO `hreflang` alternate links. * Added localized VTEX catalog support to fetch locale-specific product/category details. * **Improvements** * Breadcrumbs now use localized category/product path segments when available and consistently render as links. * Locale-aware product slug validation and PDP redirects that use cached or persisted `otherLocales`. * Added request-scoped caching for localized product lookups. * **Tests** * Expanded coverage for PDP locale parsing plus `otherLocales` persistence/recovery behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.
Summary
Implements localized product links for FastStore stores with localization.enabled: true (PDP only — PLP localized slugs are handled separately).
it-IT
pt-BR
Problem
FastStore stores with localization enabled had three major issues on the PDP:
/it-IT/adidas-polo-uomo-65/p) returned 404 because theQuery.productresolver'sstartsWithguard rejected slugs whose prefix didn't match the Intelligent SearchlinkText(always in the default locale).<span>elements andBreadcrumbJsonLdwas suppressed entirely.LocalizationSelectorhad no localized product URL to navigate to when the shopper switched locale.Changes
@faststore/api
@faststore/core
LocalizedProductContext— React context that provides otherLocales from the PDP GraphQL response to - any component in the tree (including global components like LocalizationButton)useLocalizedProduct_unstableexported from experimental/index.ts for store customizationLocalizationButton/useBindingSelector: uses otherLocales to redirect to the correct localized product URL on locale switch; falls back to the defaultLocale slug when no translation exists for the target localeuseBindingSelector: persists otherLocales to sessionStorage keyed by SKU id (fs:otherLocales:{skuId}) so the locale selector can recover the correct product URL even after the user lands on a 404 (where the page no longer provides otherLocales)Breadcrumb.tsx: removed localization.enabled workaround — always renders now that localized URLs are available-
p.tsx: restored BreadcrumbJsonLd for all locales + added hreflang alternate tags via NextSeo.additionalLinkTags (including x-default pointing to the root store URL)EmptyState: the fromUrl shown on the 404 page now includes the active Next.js locale prefix (e.g. /it-IT/...) so shoppers see the full URL they attempted to visitArchitecture note
The Catalog Dataplane is called once per product+locale combination. For otherLocales (all locales) and breadcrumbList (current locale only), results are shared via productTranslationsCache. A future optimization — replacing the Promise.all with a single "all-locales" endpoint from the Catalog team — would be a contained 2-file change with no behavior impact.
The sessionStorage persistence in
useBindingSelectoruses a fs:otherLocales:{skuId} key and is intentionally session-scoped (cleared on tab close). It is a best-effort fallback; the locale selector always prefers live otherLocales from context when available.Test plan
try using
brandlessacc/it-IT/{localized-slug}-{skuId}/p) — page renders correctlyeg. /adidas-mens-performance-polo-blast-blue-65/p -> switch to it-IT locale! You will be able to see the slug localized.
/pt-BR/{default-slug}-{skuId}/p) — page renders correctly (fallback path)eg. select en-CA
<Link>elements with localized category URLsLocalizationSelectornavigates to the correct localized product URL for each localeLocalizationSelectorfalls back to the default locale slug for locales without a translationhreflangtags appear in<head>for all configured locales +x-default(lists only theavailableLinkIds)BreadcrumbJsonLdappears in page source for all localeslocalization.enabled: false) — no regression, no extra API callsSpec file for more information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
otherLocalesvia a dedicated localization context/provider.hreflangalternate links.Improvements
otherLocales.Tests
otherLocalespersistence/recovery behavior.