feat: add i18n support for /pvt/account navigation#3400
Open
lemagnetic wants to merge 8 commits into
Open
Conversation
Contributor
WalkthroughAccount navigation now routes through the shared link resolver and router locale, while redirect destinations, proxy rewriting, and persisted session hydration all use locale-aware helpers instead of hardcoded paths or session-derived locale values. ChangesAccount navigation and locale display
Localized redirects and validation
Proxy, store, and session locale plumbing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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. |
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
🛡️ SDD Check — action requiredI couldn't detect an SDD in this PR. Please check one option below (requires write access to the repo):
|
Route My Account links through the core Link so they pick up the active locale's path prefix, and localize getServerSideProps/getStaticProps redirect destinations via the new localizeRedirectDestination helper. Remove the /pvt/ skip in withLocaleValidation now that account routes are locale-aware.
Pass locale={false} to NextLink so resolveLink stays the single source of
truth for the localized href. This prevents the server from prepending the
router locale on top of a custom-path href (e.g. /it-IT/europe/it/...),
which diverged from the client (/europe/it/...) and triggered a hydration
mismatch.
…ot binding When the default locale uses a root binding on the same hostname as path-prefixed locales (e.g. /pt-BR), rewriteSubdomainRequest was injecting /en-US on top of an already-resolved locale, producing broken paths like /pt-BR/en-US/... and 404s in dev and production. Skip the subdomain rewrite when Next has already assigned a different valid locale to the request (including _next/data routes).
… sync The session locale could reset to the default (e.g. en-US) when refocusing the window: the persisted middleware re-read a stale locale from IndexedDB on focus/visibilitychange and clobbered the correct in-memory value. The URL is the source of truth for locale/currency/salesChannel, but neither IDB nor validateSession (called with an empty search on path-based bindings) could heal it. Add an optional `reconcile` seam to the `persisted` middleware, applied both on hydration and on the cross-tab sync, threaded through `createStore` / `createSessionStore` via `CreateStoreOptions`. @faststore/core supplies the locale-aware policy (`reconcileSessionLocale`) that forces locale/currency/ salesChannel from the URL while preserving the rest of the payload, replacing the one-shot `installLocaleCorrector` (which only fixed memory and never covered the focus sync). The new params are optional and backward-compatible, but they alter public type signatures of @faststore/sdk.
The persisted middleware reconciled stale fields (e.g. URL-derived locale) into memory on hydration, but the IDB write subscriber was still gated off (`hydrated` flipped only after the hydration set), so the persisted payload stayed stale until the next write (a validateSession response or the focus sync) — which is non-deterministic under load churn. Flip `hydrated` right after the initial IDB read, before the hydration set, so the reconciled value is persisted immediately at load. The read-before-write protection is preserved (writes are still blocked until the initial read completes).
lemagnetic
force-pushed
the
feat/my-account-urls-localized
branch
from
e0432dd to
79ccfb1
Compare
lemagnetic
changed the title
…Link in OrganizationDrawerBody
lemagnetic
requested review from
eduardoformiga 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: 2
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/components/account/orders/ListOrders/SelectedTags/SelectedTags.tsx (1)
51-59: 🎯 Functional Correctness | 🟡 MinorDefault the router locale before formatting dates.
useRouter().localeis optional, whileformatFilterDaterequires a string. Fall back to the app’s default locale (or acceptstring | undefinedin the helper and handle it there) before calling it.🤖 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 51 - 59, The date formatting in SelectedTags uses useRouter().locale directly, but locale can be undefined while formatFilterDate expects a string. Update SelectedTags to default to the app’s fallback locale before formatting dateInitial and dateFinal, or adjust formatFilterDate to accept string | undefined and resolve the default internally. Use the existing useRouter, formatFilterDate, formattedDateInitial, and formattedDateFinal logic to keep the fix localized.Source: Path instructions
🧹 Nitpick comments (2)
packages/core/src/proxy.ts (2)
55-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFlatten the data-route conditional.
The nested
ifinside thedataMatchblock can collapse into a single guard, improving readability. While here,RegExp.exec()is the preferred (and slightly faster) form overString.match()for a non-global regex (per SonarQube).♻️ Flatten and switch to exec
- const { pathname } = request.nextUrl - const dataMatch = pathname.match(DATA_ROUTE_RE) - - if (dataMatch) { - const localeFromData = dataMatch[1] - if (validLocales.has(localeFromData)) { - return localeFromData - } - } + const { pathname } = request.nextUrl + const dataMatch = DATA_ROUTE_RE.exec(pathname) + + if (dataMatch && validLocales.has(dataMatch[1])) { + return dataMatch[1] + }As per path instructions: "Prefer flat conditionals over nested ifs."
🤖 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/proxy.ts` around lines 55 - 72, In getActiveLocaleFromRequest, flatten the nested dataMatch check and switch from pathname.match(DATA_ROUTE_RE) to DATA_ROUTE_RE.exec(pathname) for the data-route lookup. Use a single guard that verifies a match exists and that the captured locale is valid before returning it, keeping the existing locale fallback logic from request.nextUrl unchanged.Sources: Path instructions, Linters/SAST tools
92-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant locale validation in the guard.
getActiveLocaleFromRequestonly ever returns a locale already present invalidLocales(orundefined), so thevalidLocales.has(activeLocale)check here is dead weight. A truthyactiveLocaleis already valid.♻️ Simplify guard
- if ( - activeLocale && - validLocales.has(activeLocale) && - activeLocale !== locale - ) { - return null - } + if (activeLocale && activeLocale !== locale) { + return null + }🤖 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/proxy.ts` around lines 92 - 96, The guard in getActiveLocaleFromRequest handling is redundant because activeLocale is already guaranteed to be valid when defined. Simplify the conditional in proxy.ts by removing the validLocales.has(activeLocale) check and keeping only the truthy activeLocale comparison against locale, so the intent stays clear and the logic matches the return contract of getActiveLocaleFromRequest.
🤖 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/orders/ListOrders/ListOrdersTable/ListOrdersTable.tsx`:
- Line 111: The locale from useRouter() in ListOrdersTable may be undefined, but
formatOrderDate expects a string, so make the date formatting path type-safe.
Update the ListOrdersTable component to either provide a fallback locale before
calling formatOrderDate or adjust formatOrderDate to accept string | undefined,
and make sure all calls from ListOrdersTable use the safe value.
In `@packages/core/test/sdk/session/reconcileSessionLocale.browser.test.ts`:
- Line 86: The test suite currently targets the source module initialSession.ts
but the filename is keyed to the exported symbol reconcileSessionLocale; rename
the test file to match the source file name convention (for example,
initialSession.browser.test.ts) so it tracks the module under test. Update any
references or adjacent test organization to keep the suite aligned with the
source file and preserve predictable discovery for getInitialSession and
reconcileSessionLocale.
---
Outside diff comments:
In
`@packages/core/src/components/account/orders/ListOrders/SelectedTags/SelectedTags.tsx`:
- Around line 51-59: The date formatting in SelectedTags uses useRouter().locale
directly, but locale can be undefined while formatFilterDate expects a string.
Update SelectedTags to default to the app’s fallback locale before formatting
dateInitial and dateFinal, or adjust formatFilterDate to accept string |
undefined and resolve the default internally. Use the existing useRouter,
formatFilterDate, formattedDateInitial, and formattedDateFinal logic to keep the
fix localized.
---
Nitpick comments:
In `@packages/core/src/proxy.ts`:
- Around line 55-72: In getActiveLocaleFromRequest, flatten the nested dataMatch
check and switch from pathname.match(DATA_ROUTE_RE) to
DATA_ROUTE_RE.exec(pathname) for the data-route lookup. Use a single guard that
verifies a match exists and that the captured locale is valid before returning
it, keeping the existing locale fallback logic from request.nextUrl unchanged.
- Around line 92-96: The guard in getActiveLocaleFromRequest handling is
redundant because activeLocale is already guaranteed to be valid when defined.
Simplify the conditional in proxy.ts by removing the
validLocales.has(activeLocale) check and keeping only the truthy activeLocale
comparison against locale, so the intent stays clear and the logic matches the
return contract of getActiveLocaleFromRequest.
🪄 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: 2adfeeb6-f828-4c19-8f06-9415f4caedf5
📒 Files selected for processing (31)
packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerBody.tsxpackages/core/src/components/account/Menu/Menu.tsxpackages/core/src/components/account/orders/ListOrders/FilterSlider/FilterSlider.tsxpackages/core/src/components/account/orders/ListOrders/ListOrders.tsxpackages/core/src/components/account/orders/ListOrders/ListOrdersTable/ListOrdersTable.tsxpackages/core/src/components/account/orders/ListOrders/SelectedTags/SelectedTags.tsxpackages/core/src/components/account/orders/OrderDetails/OrderDetailsHeader.tsxpackages/core/src/components/account/utils/useFormatPrice.tspackages/core/src/components/ui/Link/Link.tsxpackages/core/src/experimental/index.tspackages/core/src/experimental/myAccountServerSideProps.tspackages/core/src/pages/pvt/account/404.tsxpackages/core/src/pages/pvt/account/[...unknown].tsxpackages/core/src/pages/pvt/account/index.tsxpackages/core/src/pages/pvt/account/profile.tsxpackages/core/src/pages/pvt/account/user-details.tsxpackages/core/src/proxy.tspackages/core/src/sdk/session/index.tspackages/core/src/sdk/session/initialSession.tspackages/core/src/utils/localization/localizeRedirectDestination.tspackages/core/src/utils/localization/withLocaleValidation.tspackages/core/test/proxy.localization.test.tspackages/core/test/sdk/session/installLocaleCorrector.browser.test.tspackages/core/test/sdk/session/reconcileSessionLocale.browser.test.tspackages/core/test/sdk/session/reconcileSessionLocale.disabled.browser.test.tspackages/core/test/utils/localization/localizeRedirectDestination.test.tspackages/sdk/src/index.tspackages/sdk/src/session/index.tspackages/sdk/src/store/composed.tspackages/sdk/src/store/persisted.tspackages/sdk/test/store/persisted.test.ts
💤 Files with no reviewable changes (2)
- packages/core/test/sdk/session/installLocaleCorrector.browser.test.ts
- packages/core/src/utils/localization/withLocaleValidation.ts
| }, | ||
| })) | ||
|
|
||
| import { reconcileSessionLocale } from '../../../src/sdk/session/initialSession' |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test filename should track its source file. This suite exercises initialSession.ts, but the file is named after the exported symbol (reconcileSessionLocale.browser.test.ts). The guideline expects the test filename to match the source file (e.g. initialSession.browser.test.ts), which also keeps test discovery predictable when both getInitialSession and reconcileSessionLocale live in the same module.
If the intent is per-symbol test files, confirm that's an accepted repo convention before merging.
As per coding guidelines: "Test files MUST match the source file name with .test. infix".
🤖 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/test/sdk/session/reconcileSessionLocale.browser.test.ts` at
line 86, The test suite currently targets the source module initialSession.ts
but the filename is keyed to the exported symbol reconcileSessionLocale; rename
the test file to match the source file name convention (for example,
initialSession.browser.test.ts) so it tracks the module under test. Update any
references or adjacent test organization to keep the suite aligned with the
source file and preserve predictable discovery for getInitialSession and
reconcileSessionLocale.
Source: Coding guidelines
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?
Localize My Account private routes so navigation and SSR redirects respect the active locale (e.g.
/pt-BR/pvt/account/ordersinstead of dropping to/pvt/account/...on the default binding).Jira: SFS-3228
How it works?
useLink/resolveLinkon account menu, orders list (pagination, search, filters), order details back link, and filter slider.localizeRedirectDestinationon My Account GSSPs (index,profile,user-details,404, catch-all)./pt-BR/en-US/...double-prefix).useRouter().localefor price/date formatting in My Account (instead ofuseSession().localeon SSR).reconcileSessionLocalevia SDKpersistedreconcile seam, keeps URL-derived locale on IDB hydrate and tab focus sync.How to test it?
pt-BR→/pt-BR).@faststore/corelocally and sign in./pt-BR/pvt/account/profile→ orders → order detail → back./pt-BRprefix.localization.enabled: falseunchanged.Video (localhost):
account: b2bfaststoredev
Gravacao.de.Tela.2026-06-25.as.21.58.28.mov
Summary by CodeRabbit
New Features
Bug Fixes