feat: centralize locale detection and add web↔docs cookie redirect#1690
Merged
Conversation
Three services implemented i18n with overlapping but inconsistent logic:
web/docs used path-based locales but never redirected new visitors based
on Accept-Language and never set a cookie; platform parsed Accept-Language
with utilities only it could use; the React-side hook, provider, and
i18next bootstrap were duplicated across web/docs/platform.
This consolidates everything behind two shared packages:
- new packages/i18n/ — framework-free locale primitives used by every
service and by the web/docs Bun servers: SUPPORTED_LOCALES,
isUrlPrefixedLocale/isCookieLocale, parseAcceptLanguage, resolveLocale,
isValidLocale, narrowBcp47, localizedPath, cookie helpers
(LOCALE_COOKIE_NAME='tale_locale', serializeLocaleCookie,
readLocaleCookie), and negotiatePathLocale (the redirect decision tree).
- @tale/ui/i18n/* — React/i18next glue used by all three services:
defaultLocale, detectInitialLocale, resolveRegionalLocale, useT,
useCurrentLocale, I18nProvider, initI18n, collectRegionalBundles, and
LocaleSync (single owner of i18n.changeLanguage + <html lang>). The
React i18n previously lived in @tale/webui/i18n/* but moved to
@tale/ui/i18n/* so platform — which doesn't depend on @tale/webui —
could use it.
Behavior changes for the marketing/docs sites:
- a German visitor hitting / now gets a 302 to /de and a tale_locale=de
cookie (Domain=.tale.dev in prod via LOCALE_COOKIE_DOMAIN, unset in
dev). On the next visit they land on /de directly without re-detection.
Cross-subdomain cookie sharing means tale.dev and docs.tale.dev honor
the same preference.
- locale-neutral paths (/api/*, /sitemap.xml, /robots.txt, /llms*.txt,
*.md) skip negotiation entirely.
- docs Dockerfile now bundles server.ts → server.js so the @tale/i18n
import resolves at runtime; mirrors what web already did.
Per-service lib/i18n/ folders are now identical (same file shape across
web, docs, platform, plus the plop react-service template). Platform
adopts the shared useT/I18nProvider/initI18n; useLocale stays
platform-specific (localStorage + dayjs) but no longer calls
i18n.changeLanguage directly — LocaleSync owns that.
Cleanup along the way:
- moved platform's parse-accept-language/resolve-locale/is-valid-locale
into @tale/i18n; deleted the originals.
- moved narrow-bcp47 from platform into @tale/i18n.
- SUPPORTED_AGENT_LOCALES re-exports SUPPORTED_LOCALES.
- removed services/web/lib/i18n/localized-paths.ts: localizedHref → the
shared localizedPath, and the route table moved inline into
LocalizedLink (its only consumer).
- removed legacy unused notFound: { ... } namespace from docs's en/de/fr
bundles.
- docs gained the shared defineMessagesParityTests +
defineMessagesUsageTests (web and platform already had them).
Verification: bun run check — 38/38 turbo tasks green
(45 i18n tests, 3025 platform tests, 26 docs tests, all
linting/typechecking/format). Smoke-tested locally with curl on the web
server: DE Accept-Language → 302 /de + cookie; FR cookie + /pricing →
302 /fr/pricing; EN visitor → 200 + en cookie; /sitemap.xml and
/api/health skip negotiation.
CodeRabbit review applied: URIError guard around decodeURIComponent in
both servers; explicit strict: true in packages/i18n/tsconfig.json;
case-insensitive .md skip; resolve-locale doc comment matches
implementation.
The platform Dockerfile didn't COPY packages/i18n into its workspace stage, so `bun install` in CI failed with "Workspace not found packages/i18n" and broke Build platform / Smoke test / Validate images. Web and docs were already updated; mirror the same change here. Knip flagged four pure-shim files that no service code actually imports: - services/web/lib/i18n/config.ts - services/docs/lib/i18n/config.ts - services/docs/lib/i18n/use-current-locale.ts - services/platform/lib/i18n/locales.ts These were added in the prior commit "for parity" with platform's config.ts (which IS used by 3 consumers). Deleting them keeps each service's lib/i18n/ honest about what it actually consumes — web/docs still have locales.ts + use-current-locale.ts (URL-based), platform still has config.ts (consumed by number formatter, agents config, org queries). Plop template trimmed to the universal subset. Trim re-exports in lib/i18n/locales.ts to drop unused symbols (localizedPath/Locale/RegionalLocale/UrlPrefixedLocale in docs; ALL_LOCALES/Locale/RegionalLocale/UrlPrefixedLocale in web). Drop unused dependencies surfaced by knip: - root: react-i18next (lives in @tale/ui now) - platform: i18next-icu, intl-messageformat (only used by @tale/ui's initI18n)
Contributor
📝 WalkthroughWalkthroughThis PR establishes a shared, standardized i18n infrastructure across the Tale monorepo by creating a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
87-87:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove or justify unused
react-i18nextroot dependency.Knip reports
react-i18nextas unused at Line 87. If usage has moved fully behind@tale/ui/i18n, this should be removed to keep dependency hygiene clean.🤖 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 `@package.json` at line 87, Knip flags the root package.json dependency "react-i18next" as unused; either remove the "react-i18next": "17.0.2" entry from package.json or add a short justification (e.g., comment in PR description or move to devDependencies) if it’s intentionally kept for consumers; if usage is now provided by the scoped package "@tale/ui/i18n", delete the dependency line and run a reinstall and linting/knip check to confirm nothing breaks, or document why the root-level dependency must remain.services/platform/package.json (1)
99-100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove now-unused i18n dependencies flagged by knip.
Knip flags
i18next-icu(line 99) andintl-messageformat(line 100) as unused in services/platform. Both have no direct imports anywhere in platform source code;i18next-icuis pulled in transitively via@tale/ui, andintl-messageformatappears to be an orphaned dependency from prior code. Remove both to resolve the lint failure.🔧 Cleanup
"i18next": "26.0.4", - "i18next-icu": "2.4.3", - "intl-messageformat": "11.2.0", "jexl": "2.3.0",🤖 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 `@services/platform/package.json` around lines 99 - 100, Remove the now-unused i18n dependencies by deleting the "i18next-icu" and "intl-messageformat" entries from services/platform package.json dependencies, run the package manager to update lockfile (npm/yarn/pnpm install), and re-run the knip/lint pipeline and tests to confirm no imports break; note that "i18next-icu" is transitive via `@tale/ui` so removing the direct entry is safe.
🤖 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/i18n/src/locales.ts`:
- Around line 53-59: The function localizedPath currently returns an empty
string for localizedPath('en','') because the 'en' fast path runs before
normalizing root; move or add a root normalization check in localizedPath to
handle pathname === '' or pathname === '/' first: if root and locale === 'en'
return '/' otherwise return `/${locale}`; this ensures all locales including
'en' produce a valid root URL while preserving existing behavior for non-root
paths.
In `@packages/ui/package.json`:
- Around line 55-63: The package `@tale/ui` exposes React hooks/components that
rely on i18next, react-i18next, and i18next-icu at runtime, so move "i18next",
"react-i18next", and "i18next-icu" out of the dependencies block in
packages/ui/package.json and declare them under peerDependencies (keeping the
same version constraints) to require the host app to provide matching versions;
leave "@tale/i18n" as a normal dependency and optionally add those three
packages to devDependencies for local testing/builds if needed.
In `@packages/ui/src/i18n/locales.ts`:
- Around line 26-31: The function resolveRegionalLocale currently declares a
return type of string which loses type safety; change its signature to return
the precise locale union (e.g., Locale or SupportedLocale | RegionalLocale) and
import the appropriate type(s) into this module, update any related
constants/types such as REGIONAL_OVERRIDES if necessary so the candidate
(`${base}-${region}`) is recognized as a RegionalLocale, and ensure callers
still type-check with the new return type; reference the resolveRegionalLocale
function, the SupportedLocale/RegionalLocale/Locale types, and
REGIONAL_OVERRIDES when making these changes.
In `@packages/ui/src/i18n/use-current-locale.ts`:
- Around line 13-16: Remove the unsafe type assertion on the useParams call in
use-current-locale.ts and instead use TanStack Router's select option to pick
the lang param with proper typing; specifically, replace the cast around
useParams(...) as { lang?: string } by calling useParams({ strict: false,
select: p => ({ lang: p.lang }) }) (or select: p => p.lang depending on how you
consume it) so TypeScript infers lang as string | undefined without using `as`,
and update any code that reads `params.lang` to match the new selected shape.
In `@services/docs/app/entry-server.tsx`:
- Around line 19-20: Extract the duplicated expression that computes basepath
((import.meta.env.BASE_URL ?? '/').replace(/\/$/, '') || undefined) into a
single helper exported from a new module named basepath (e.g., export const
getBasepath or export const basepath) and replace the inline expression in both
router.tsx and entry-server.tsx with an import from that helper; ensure the
helper encapsulates the exact logic and preserves the undefined fallback so
existing callers (router and entry-server) behave identically.
In `@services/docs/lib/i18n/config.ts`:
- Line 1: Remove the unused shim file services/docs/lib/i18n/config.ts which
only re-exports defaultLocale from `@tale/ui/i18n/config`; delete the file and
update any imports if present to use `@tale/ui/i18n/config` directly (look for the
export "defaultLocale" in the file to locate the shim).
In `@services/docs/lib/i18n/keys-dynamic.txt`:
- Around line 10-14: The file comment is stale: update the explanatory comment
that references where `languageSwitcher` and `themeSwitcher` live; change the
path mention from "packages/webui/src/layout/" to the new location under
`@tale/ui/i18n` (or the appropriate `@tale/ui` package path that now contains
the React/i18next glue), so anyone auditing the allowlist knows
`languageSwitcher` and `themeSwitcher` are consumed from `@tale/ui/i18n` rather
than `@tale/webui`; keep the rest of the comment and listed keys
(`languageSwitcher`, `themeSwitcher`) unchanged.
In `@services/docs/lib/i18n/locales.ts`:
- Around line 5-13: The shim currently re-exports unused symbols localizedPath,
RegionalLocale, and UrlPrefixedLocale which breaks Knip; remove those three from
the re-export lists in this module so only consumed types/symbols remain (keep
resolveRegionalLocale, Locale, SupportedLocale if they are used). Update the two
export blocks (the named export list and the exported type list) to drop
localizedPath, RegionalLocale and UrlPrefixedLocale and run the Knip/lint to
confirm the pipeline passes.
In `@services/docs/lib/i18n/use-current-locale.ts`:
- Line 1: This re-export file only exposes useCurrentLocale (export {
useCurrentLocale } from '@tale/ui/i18n/use-current-locale') but isn't consumed
by the docs service; either delete this unused shim (remove
services/docs/lib/i18n/use-current-locale.ts) or wire it through by importing
useCurrentLocale in the docs app (e.g., add an import of useCurrentLocale in the
docs entry or a file under services/docs that uses i18n client functionality) so
knip sees it as used; update any barrel/index exports if needed to keep public
API consistent.
In `@services/docs/server.ts`:
- Around line 62-65: The catch block that swallows errors when decoding the
request path (the decodeURIComponent(pathname) call that assigns to rel) should
log the decoding error before returning the SPA fallback; update the catch to
capture the thrown error and emit a warning (e.g., console.warn or
console.error) including the pathname and the error, then return new
Response(file(join(DIST, 'index.html'))); keep the SPA fallback behavior intact
and reference the same variables/expressions (decodeURIComponent, pathname, rel,
Response(file(join(DIST, 'index.html')))) so the only change is adding the
warning log.
In `@services/platform/app/hooks/use-locale.ts`:
- Around line 40-53: The setLocale function can race with async dayjs locale
loading causing applyLocale/formatDate to use the old locale; change setLocale
(the function that currently calls setLocaleState(resolved) and
localStorage.setItem(STORAGE_KEY, resolved)) to first await
loadDayjsLocale(resolved) (handling errors) and only after the import succeeds
update setLocaleState(resolved) and persist to localStorage, or alternately gate
the locale state on a "localeLoaded" flag so components render only after
loadDayjsLocale completes; reference loadDayjsLocale, setLocale, setLocaleState,
applyLocale/formatDate, STORAGE_KEY, isValidLocale, and defaultLocale when
making this change.
In `@services/platform/lib/i18n/locales.ts`:
- Around line 1-13: This module exports and re-exports symbols (ALL_LOCALES,
detectInitialLocale, isUrlPrefixedLocale, localizedPath, resolveRegionalLocale
and types Locale, RegionalLocale, SupportedLocale, UrlPrefixedLocale) but is
unused; either remove this barrel file services/platform/lib/i18n/locales.ts to
eliminate dead code, or update callers to import from it — if you choose to
remove, delete the file and run the build/knip checks; if you choose to keep it,
add/modify imports in the consumers to reference this barrel (e.g., replace
direct imports from '@tale/ui/i18n/locales' with imports from
'services/platform/lib/i18n/locales.ts' where appropriate) so the exported
symbols are actually consumed.
In `@services/web/Dockerfile`:
- Around line 49-53: Remove the build-stage ARG/ENV declarations for
LOCALE_COOKIE_DOMAIN in the Dockerfile to avoid invalidating the build cache;
LOCALE_COOKIE_DOMAIN is only read at runtime by services/web/server.ts, so
delete the ARG LOCALE_COOKIE_DOMAIN and the ENV LOCALE_COOKIE_DOMAIN entries
from the builder stage (the block containing ARG LOCALE_COOKIE_DOMAIN and ENV
NODE_ENV... LOCALE_COOKIE_DOMAIN) and ensure the runner stage still defines
LOCALE_COOKIE_DOMAIN where it's actually consumed.
In `@services/web/lib/i18n/config.ts`:
- Line 1: The file contains an unused re-export shim: "export { defaultLocale }
from '@tale/ui/i18n/config'"; remove this shim if nothing imports this module,
or update the consumer(s) to import defaultLocale from this path instead of
importing directly from '@tale/ui/i18n/config'. Specifically, either delete the
re-export statement (cleanup) or ensure any intended consumers change their
imports to reference this re-exported symbol "defaultLocale" so the shim is
actually used.
In `@services/web/lib/i18n/locales.ts`:
- Line 14: Remove the unused re-exports flagged by knip: delete the export
entries for ALL_LOCALES and the type re-exports Locale, RegionalLocale, and
UrlPrefixedLocale from the locales barrel (the export list that currently
re-exports these symbols). Ensure you only remove the export statements (not the
original type or value definitions if they live elsewhere); run the build/tests
to confirm nothing breaks and add them back later when a concrete consumer
requires them.
In `@services/web/server.ts`:
- Around line 171-174: The bare catch around decodeURIComponent(pathname)
swallows malformed URL decode errors; update the catch to log the error and the
offending pathname instead of being empty so failures are visible. Specifically,
in the block that sets rel = decodeURIComponent(pathname).replace(/^\/+/, '');
catch the thrown error and call console.warn or console.error with a short
message including pathname and the error (or re-throw if desired), then return
the same fallback Response(file(join(DIST, 'index.html'))).
- Around line 211-236: The locale negotiation is incorrectly handling static
asset requests; update isLocaleNeutralPath (in packages/i18n/src/negotiate.ts)
so negotiatePathLocale() returns skip: true for asset directory patterns (e.g.,
paths under /assets/) and for common static file extensions (.js, .css, .png,
.jpg, .svg, .woff, .woff2, .ico, .map, etc.), ensuring negotiatePathLocale()
never sets redirectTo for those paths; verify both services calling
negotiatePathLocale (services/web where serveStatic and
applyLocaleResponseHeaders are used, and services/docs) will bypass
cookie/Accept-Language redirects for these requests.
In `@tools/plop/templates/react-service/lib/i18n/locales.ts`:
- Around line 1-13: Collapse the two named export blocks into wildcard
re-exports: replace the explicit exports of ALL_LOCALES, detectInitialLocale,
isUrlPrefixedLocale, localizedPath, resolveRegionalLocale and the explicit type
exports Locale, RegionalLocale, SupportedLocale, UrlPrefixedLocale with a single
export * from '@tale/ui/i18n/locales' and a single export type * from
'@tale/ui/i18n/locales' (so future additions from that module are automatically
exported).
---
Outside diff comments:
In `@package.json`:
- Line 87: Knip flags the root package.json dependency "react-i18next" as
unused; either remove the "react-i18next": "17.0.2" entry from package.json or
add a short justification (e.g., comment in PR description or move to
devDependencies) if it’s intentionally kept for consumers; if usage is now
provided by the scoped package "@tale/ui/i18n", delete the dependency line and
run a reinstall and linting/knip check to confirm nothing breaks, or document
why the root-level dependency must remain.
In `@services/platform/package.json`:
- Around line 99-100: Remove the now-unused i18n dependencies by deleting the
"i18next-icu" and "intl-messageformat" entries from services/platform
package.json dependencies, run the package manager to update lockfile
(npm/yarn/pnpm install), and re-run the knip/lint pipeline and tests to confirm
no imports break; note that "i18next-icu" is transitive via `@tale/ui` so removing
the direct entry is safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50a35478-7bb9-4183-84b2-12a6efa62df6
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (87)
package.jsonpackages/i18n/package.jsonpackages/i18n/src/accept-language.test.tspackages/i18n/src/accept-language.tspackages/i18n/src/cookie.test.tspackages/i18n/src/cookie.tspackages/i18n/src/is-valid-locale.tspackages/i18n/src/locales.tspackages/i18n/src/narrow-bcp47.tspackages/i18n/src/negotiate.test.tspackages/i18n/src/negotiate.tspackages/i18n/src/resolve-locale.test.tspackages/i18n/src/resolve-locale.tspackages/i18n/tsconfig.jsonpackages/i18n/vitest.config.tspackages/ui/package.jsonpackages/ui/src/i18n/client.tsxpackages/ui/src/i18n/config.tspackages/ui/src/i18n/init.tspackages/ui/src/i18n/locales.tspackages/ui/src/i18n/provider.tsxpackages/ui/src/i18n/regional-bundles.tspackages/ui/src/i18n/sync.tsxpackages/ui/src/i18n/use-current-locale.tspackages/webui/package.jsonpackages/webui/src/i18n/locales.tspackages/webui/src/index.tspackages/webui/src/layout/language-switcher.tsxpackages/webui/src/layout/theme-switcher.tsxpackages/webui/src/seo/build-sitemap.tspackages/webui/src/seo/document-meta.tsservices/docs/Dockerfileservices/docs/app/entry-server.tsxservices/docs/app/router.tsxservices/docs/app/routes/__root.tsxservices/docs/docker-entrypoint.shservices/docs/index.htmlservices/docs/lib/i18n/client.tsxservices/docs/lib/i18n/config.tsservices/docs/lib/i18n/i18n-provider.tsxservices/docs/lib/i18n/i18n.tsservices/docs/lib/i18n/keys-dynamic.txtservices/docs/lib/i18n/locales.tsservices/docs/lib/i18n/messages-usage.test.tsservices/docs/lib/i18n/messages.test.tsservices/docs/lib/i18n/use-current-locale.tsservices/docs/messages/de.jsonservices/docs/messages/en.jsonservices/docs/messages/fr.jsonservices/docs/package.jsonservices/docs/server.tsservices/docs/vitest.config.tsservices/platform/app/hooks/use-locale.tsservices/platform/convex/agent_tools/delegation/create_delegation_tool.tsservices/platform/lib/i18n/client.tsxservices/platform/lib/i18n/config.tsservices/platform/lib/i18n/i18n-provider.tsxservices/platform/lib/i18n/i18n.tsservices/platform/lib/i18n/locales.tsservices/platform/lib/shared/constants/agents.tsservices/platform/lib/shared/utils/resolve-agent-locale.tsservices/platform/package.jsonservices/web/Dockerfileservices/web/app/components/layout/localized-link.tsxservices/web/app/components/layout/site-footer.tsxservices/web/app/components/layout/site-header.tsxservices/web/app/pages/contact-page.tsxservices/web/app/pages/hardware-pricing-page.tsxservices/web/app/pages/home-page.tsxservices/web/app/pages/pricing-page.tsxservices/web/app/pages/request-demo-page.tsxservices/web/app/routes/__root.tsxservices/web/lib/i18n/client.tsxservices/web/lib/i18n/config.tsservices/web/lib/i18n/i18n-provider.tsxservices/web/lib/i18n/i18n.tsservices/web/lib/i18n/locales.tsservices/web/lib/i18n/localized-paths.tsservices/web/lib/i18n/use-current-locale.tsservices/web/package.jsonservices/web/server.tstools/plop/generators/react-service.tstools/plop/templates/react-service/lib/i18n/client.tsxtools/plop/templates/react-service/lib/i18n/config.tstools/plop/templates/react-service/lib/i18n/i18n-provider.tsxtools/plop/templates/react-service/lib/i18n/i18n.ts.hbstools/plop/templates/react-service/lib/i18n/locales.ts
💤 Files with no reviewable changes (6)
- packages/webui/package.json
- services/web/lib/i18n/localized-paths.ts
- services/docs/messages/fr.json
- packages/webui/src/i18n/locales.ts
- services/docs/messages/de.json
- services/docs/messages/en.json
…atic assets
Convex's deploy bundler doesn't resolve workspace-package subpath exports
through transitive re-exports. Smoke test failed with "Could not resolve
'@tale/i18n/narrow-bcp47'", "@tale/ui/i18n/config", "@tale/i18n/locales"
because platform's convex code reaches those workspace packages via
relative imports through `lib/i18n/config.ts`, `lib/shared/constants/
agents.ts`, and `lib/shared/utils/resolve-agent-locale.ts`.
Inline those values where convex can see them:
- services/platform/lib/i18n/config.ts: inline `defaultLocale = 'en'`
(was a re-export from `@tale/ui/i18n/config`).
- services/platform/lib/shared/constants/agents.ts: inline
`SUPPORTED_AGENT_LOCALES = ['en', 'de', 'fr']` (was a re-export of
`SUPPORTED_LOCALES` from `@tale/i18n/locales`).
- services/platform/lib/shared/utils/narrow-bcp47.ts: restored as a
platform-internal helper; dropped from `@tale/i18n` (it had only one
consumer). Convex imports go through the relative path.
Both files carry a comment explaining why the re-export indirection is
intentional, in case a future cleanup rewires them through @tale/*.
Also fixes a real prod bug from CodeRabbit's review: the negotiator was
treating `/assets/index-Bn7FHL.js` as a candidate for redirect, so a
non-EN visitor's first request would 302 to `/de/assets/index-...js`,
load the SPA shell at that path, and break (script never loads).
`isLocaleNeutralPath` now skips `/assets/` and any path with a static
asset extension (.js, .css, .map, .png, .woff2, .ico, etc.).
Other CodeRabbit findings applied:
- localizedPath('en', '') returned '' (empty string); now returns '/'.
Root normalization happens before the EN fast path.
- resolveRegionalLocale return type tightened to
`SupportedLocale | RegionalLocale` (was `string`); REGIONAL_OVERRIDES
is now typed `ReadonlySet<RegionalLocale>` and the lookup goes through
a narrowed type guard.
- useCurrentLocale: replaced the `as { lang?: string }` cast with
TanStack Router's `select` callback with a parameter annotation —
no more `oxlint-disable` for that line.
- web/docs server.ts: `decodeURIComponent` catch now logs the offending
pathname + error before falling back to the SPA shell (was silent).
- web/docs Dockerfiles: dropped the build-stage `LOCALE_COOKIE_DOMAIN`
ARG/ENV. The value is read only at runtime by the Bun server, so it
doesn't need to be in the builder layer (avoids cache-bust on env
changes).
Skipped findings:
- Move i18next/react-i18next/i18next-icu to peerDependencies in
@tale/ui: workspace-deduped via bun, regular deps work; not minimal.
- Extract a `basepath` helper for docs entry-server/router: pre-existing
code, out of scope for this PR.
- keys-dynamic.txt comment update: the path it references
(packages/webui/src/layout/) is still correct — the switcher
*components* stayed in webui, only the i18n primitives moved to ui.
- Awaiting `loadDayjsLocale` before `setLocaleState`: pre-existing
platform behavior, out of scope.
- Wildcard re-export from `@tale/ui/i18n/locales` in the plop template:
the template doesn't include locales.ts anymore.
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.
Summary
@tale/i18npackage owns locale primitives, cookie helpers, and the path-locale negotiation used by the web/docs Bun servers. New@tale/ui/i18n/*(moved here from@tale/webuiso platform can use it) owns the React/i18next glue, including a singleLocaleSynccomponent now responsible fori18n.changeLanguage+<html lang>.Accept-Languageisdeorfr(and who hits an unprefixed path) is302-redirected to/de//frand atale_localecookie is set. Cookie domain is configurable viaLOCALE_COOKIE_DOMAIN(set to.tale.devin prod sotale.devanddocs.tale.devshare the preference). Locale-neutral paths (/api/*,/sitemap.xml,/robots.txt,/llms*.txt,*.md) skip negotiation.lib/i18n/folders are now identical thin re-export shims (client.tsx,config.ts,i18n-provider.tsx,i18n.ts,keys-dynamic.txt,locales.ts,messages.test.ts,messages-usage.test.ts,types.ts; web/docs additionally haveuse-current-locale.ts). The plopreact-servicetemplate matches.Behavior changes worth flagging
tale_localecookie persists the choice across subdomains.Dockerfilenow bundlesserver.ts→server.js(mirroring web) so the@tale/i18nworkspace import resolves at runtime.services/web/lib/i18n/localized-paths.tsremoved;localizedHref→localizedPathfrom@tale/ui/i18n/locales; the route table moved inline into the only consumer (LocalizedLink).notFound: { … }namespace removed from docs's en/de/fr bundles (the active keys arenotFoundTitleetc. inside thedocsnamespace).Things that stayed put
useLocale(localStorage + dayjs loader) is platform-specific business logic — kept where it is, but trimmed: it no longer callsi18n.changeLanguage, sinceLocaleSyncowns that.resolve-agent-locale,get-organization-default-locale) is platform-specific (selects locale-keyed agent metadata fields, not UI translation) — stays in platform.Test plan
bun run check(format, lint, typecheck, all tests) — 38/38 turbo tasks green: 45 i18n tests, 3025 platform tests, 26 docs tests.services/platform/messages/{en,de,fr}.json— N/A (no platform copy changes; platform i18n provider was rewired but message bundles unchanged)./docs/{en,de,fr}/for every user-visible change — N/A (no doc-content changes; the redirect is purely server-side and behavior-driven).bun run --filter @tale/docs lintandbun run --filter @tale/docs test— green.README.md,README.de.md,README.fr.md— N/A (no top-level repo behavior change visible to a reader of those READMEs).Manual smoke (web dev server, port 3781):
curl -i -H 'Accept-Language: de-DE,de;q=0.9' http://localhost:3781/→302 Location: /de+Set-Cookie: tale_locale=de.curl -i -H 'Cookie: tale_locale=fr' http://localhost:3781/pricing→302 Location: /fr/pricing.curl -i -H 'Accept-Language: en-US,en;q=0.9' http://localhost:3781/→200+Set-Cookie: tale_locale=en.curl -i http://localhost:3781/de/pricing→200+Set-Cookie: tale_locale=de(cookie refresh).curl -i http://localhost:3781/dewith cookie alreadyde→200+Varyonly, noSet-Cookie(no rewrite).curl -i http://localhost:3781/sitemap.xmland/api/health→ noSet-Cookie, noVary(skip).Outstanding manual checks
LOCALE_COOKIE_DOMAIN=.tale.devbaked into both Dockerfiles; visithttps://tale.dev/deand confirm DevTools → Application → Cookies showsDomain=.tale.dev; then visithttps://docs.tale.dev/and confirm302 → /de.localStorage.user-localeupdates and<html lang>flips (<html lang>is new for platform — previously not set).Reviewer notes
decodeURIComponentin both servers; explicitstrict: trueinpackages/i18n/tsconfig.json; case-insensitive.mdskip; doc-comment fix inresolve-locale.ts). Skipped findings that would change semantics (??forLOCALE_COOKIE_DOMAINwould break the empty-string env-var contract) or were YAGNI (cookie library swap,encodeURIComponentof constrained'en'|'de'|'fr'values, REGIONAL_FALLBACKS table).main(inservices/docs/app/{entry-server,router}.tsxaroundimport.meta.env.BASE_URL) and four pre-existingoxfmtdrifts (services/docs/{app/entry-server.tsx,app/router.tsx,index.html}andservices/web/app/components/layout/site-header.tsx) were fixed along the way to keepbun run checkgreen.Summary by CodeRabbit
Release Notes
New Features
Refactoring
Chores