feat(i18n): add i18n infrastructure with zh-CN support (v2 — typed keys, locale-aware head, parity test)#729
Conversation
…pages - Add lightweight i18n system (types, translate, useI18n hook) - Create en.ts and zh-CN.ts locale files - Add language switcher to Header - Extract and translate strings in: - Header, Footer, Home page - Skills browse/toolbar/results - Dashboard, Settings, Stars, Upload Ref: openclaw#606
…nagement, souls, profile) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ail page, management, and header
- Keep Skill/Agent/Soul as untranslated proper nouns - Normalize CJK-Latin spacing (pangu) throughout zh-CN locale - Improve translation quality: badge, subtitles, security text - Fix mobile language switcher to enumerate all locales (not hardcoded toggle) - SSR guards for localStorage/navigator already in place (prior commit)
…arity test Addresses maintainer feedback from PR openclaw#607 review: 1. Typed translation keys (compile-time safety): - Add FlattenKeys<T> recursive type that derives dot-path union from en.ts - Export TranslationKey type; t() now only accepts valid keys - en.ts uses 'as const satisfies TranslationMap' to preserve literal types - Typos in t('xxx') are now caught by TypeScript at compile time 2. Locale-aware document shell: - <html lang> dynamically reflects the current i18n locale - <title> and <meta description> use t() for site name/description - Add site.skillsName/Description and site.soulsName/Description keys 3. Locale parity test (vitest): - New test file asserts en and zh-CN have identical key sets - Reports missing/extra keys with clear error messages - Validates all leaf values are non-empty strings 4. Translate new upstream hardcoded strings: - slugCollision.ts: 'Slug is already taken...' - upload.tsx: 'Accept the MIT-0 license terms...' (×2) - upload/-utils.ts: 'Publish failed...' - Added common.slugTaken, common.acceptLicense, common.publishFailed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the Amantus Machina Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds a solid i18n infrastructure: a typed Three issues are worth addressing before merge:
Confidence Score: 3/5
Last reviewed commit: cf595f0 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| head: () => { | ||
| const mode = getSiteMode() | ||
| const siteName = getSiteName(mode) | ||
| const siteDescription = getSiteDescription(mode) | ||
| const siteName = mode === 'souls' ? t('site.soulsName') : t('site.skillsName') | ||
| const siteDescription = | ||
| mode === 'souls' ? t('site.soulsDescription') : t('site.skillsDescription') | ||
| const siteUrl = getSiteUrlForMode(mode) | ||
| const ogImage = `${siteUrl}/og.png` |
There was a problem hiding this comment.
Non-reactive head() metadata — locale changes won't update <title> or <meta>
The head() function calls the module-level t() singleton at route-definition time. Because createRootRoute's head callback is evaluated once (not on every locale change), the generated <title>, <meta name="description">, and all OG/Twitter meta tags will remain in the initial locale even after the user switches language via the Header dropdown.
Only <html lang={locale}> in RootDocument is wired to useSyncExternalStore and will update reactively. The document title and description won't follow suit.
The PR description states "Locale-aware document shell (<html>, <head>)", but this is only partially true — <html lang> is locale-aware, while <head> content is not.
To make the title reactive, consider moving the title/description logic into RootDocument using useI18n() and a document effect:
// inside RootDocument, after const { locale } = useI18n()
useEffect(() => {
const mode = getSiteMode()
document.title = mode === 'souls' ? t('site.soulsName') : t('site.skillsName')
}, [locale, t])Or — if SSR/static title is sufficient and runtime updates aren't required — document this intentional limitation clearly in a comment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/routes/__root.tsx
Line: 16-22
Comment:
**Non-reactive `head()` metadata — locale changes won't update `<title>` or `<meta>`**
The `head()` function calls the module-level `t()` singleton at route-definition time. Because `createRootRoute`'s `head` callback is evaluated once (not on every locale change), the generated `<title>`, `<meta name="description">`, and all OG/Twitter meta tags will remain in the initial locale even after the user switches language via the Header dropdown.
Only `<html lang={locale}>` in `RootDocument` is wired to `useSyncExternalStore` and will update reactively. The document title and description won't follow suit.
The PR description states "Locale-aware document shell (`<html>`, `<head>`)", but this is only partially true — `<html lang>` is locale-aware, while `<head>` content is not.
To make the title reactive, consider moving the title/description logic into `RootDocument` using `useI18n()` and a document effect:
```ts
// inside RootDocument, after const { locale } = useI18n()
useEffect(() => {
const mode = getSiteMode()
document.title = mode === 'souls' ? t('site.soulsName') : t('site.skillsName')
}, [locale, t])
```
Or — if SSR/static title is sufficient and runtime updates aren't required — document this intentional limitation clearly in a comment.
How can I resolve this? If you propose a fix, please make it concise.| <div className="site-footer-copy"> | ||
| {siteName} · An{' '} | ||
| {siteName} · {t('footer.an')}{t('footer.an') ? ' ' : ''} | ||
| <a href="https://openclaw.ai" target="_blank" rel="noreferrer"> |
There was a problem hiding this comment.
t() called twice; empty-string contract is implicit and fragile
t('footer.an') is evaluated twice in this expression: once to render the word, and again to conditionally add a trailing space. This works because footer.an is intentionally set to "" (empty string) in zh-CN, but that contract is not enforced anywhere:
t()only falls back to English when the resolved value isundefined— an empty string""is returned as-is (seetranslate.ts).- The locale-parity test only validates that en values are non-empty; it does not enforce that
zh-CNvalues are non-empty, so an accidental""in a key that should have a translation would silently render nothing.
Consider using a single variable or restructuring the key to avoid the double call and make the conditional explicit:
| <a href="https://openclaw.ai" target="_blank" rel="noreferrer"> | |
| {siteName} · {t('footer.an') ? `${t('footer.an')} ` : ''} |
Or better, collapse this into a single key footer.anSpace with value "An " (en) / "" (zh-CN), removing the two-step pattern entirely.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/Footer.tsx
Line: 14
Comment:
**`t()` called twice; empty-string contract is implicit and fragile**
`t('footer.an')` is evaluated twice in this expression: once to render the word, and again to conditionally add a trailing space. This works because `footer.an` is intentionally set to `""` (empty string) in `zh-CN`, but that contract is not enforced anywhere:
- `t()` only falls back to English when the resolved value is `undefined` — an empty string `""` is returned as-is (see `translate.ts`).
- The locale-parity test only validates that **en** values are non-empty; it does **not** enforce that `zh-CN` values are non-empty, so an accidental `""` in a key that *should* have a translation would silently render nothing.
Consider using a single variable or restructuring the key to avoid the double call and make the conditional explicit:
```suggestion
{siteName} · {t('footer.an') ? `${t('footer.an')} ` : ''}
```
Or better, collapse this into a single key `footer.anSpace` with value `"An "` (en) / `""` (zh-CN), removing the two-step pattern entirely.
How can I resolve this? If you propose a fix, please make it concise.| (cb) => i18n.subscribe(cb), | ||
| () => i18n.getLocale(), | ||
| ); | ||
|
|
There was a problem hiding this comment.
Missing getServerSnapshot — risk of SSR hydration mismatch on <html lang>
useSyncExternalStore accepts an optional third argument getServerSnapshot. Without it, React uses the client getSnapshot on the server too. On the server, i18n.detectLocale() correctly returns "en" (no localStorage / navigator). However, on the first client render, if the user previously stored "zh-CN" in localStorage, i18n.detectLocale() will return "zh-CN", making the hydrated value ("zh-CN") diverge from the server-rendered value ("en").
This produces a React hydration mismatch warning on <html lang> in RootDocument, and potentially on any translated string rendered during the shell phase.
Provide a stable server snapshot to suppress the mismatch:
| (cb) => i18n.subscribe(cb), | |
| () => i18n.getLocale(), | |
| ); | |
| const locale = useSyncExternalStore( | |
| (cb) => i18n.subscribe(cb), | |
| () => i18n.getLocale(), | |
| () => 'en' as const, // server snapshot — always English | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/i18n/useI18n.ts
Line: 7-10
Comment:
**Missing `getServerSnapshot` — risk of SSR hydration mismatch on `<html lang>`**
`useSyncExternalStore` accepts an optional third argument `getServerSnapshot`. Without it, React uses the client `getSnapshot` on the server too. On the server, `i18n.detectLocale()` correctly returns `"en"` (no `localStorage` / `navigator`). However, on the first client render, if the user previously stored `"zh-CN"` in `localStorage`, `i18n.detectLocale()` will return `"zh-CN"`, making the hydrated value (`"zh-CN"`) diverge from the server-rendered value (`"en"`).
This produces a React hydration mismatch warning on `<html lang>` in `RootDocument`, and potentially on any translated string rendered during the shell phase.
Provide a stable server snapshot to suppress the mismatch:
```suggestion
const locale = useSyncExternalStore(
(cb) => i18n.subscribe(cb),
() => i18n.getLocale(),
() => 'en' as const, // server snapshot — always English
);
```
How can I resolve this? If you propose a fix, please make it concise.| } | ||
| return typeof current !== "string" || current.trim() === ""; | ||
| }); | ||
| if (empty.length > 0) { | ||
| throw new Error(`en has ${empty.length} empty value(s):\n ${empty.join("\n ")}`); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Parity test doesn't validate zh-CN values are non-empty
The fifth assertion ("all leaf values in en should be non-empty strings") only checks the en locale. The zh-CN locale can — and does — contain intentional empty strings (e.g., footer.an: ""). Without a corresponding check, an accidentally empty zh-CN translation would pass CI silently and render a blank string in the UI (since t() only falls back to English for undefined, not "").
Consider adding a companion test that documents which keys may intentionally be empty in zh-CN, and flags unexpected empty values:
const INTENTIONALLY_EMPTY_ZH_CN = new Set(['footer.an'])
it("zh-CN non-intentional values should be non-empty strings", () => {
const zhValues = zhKeys.filter((k) => !INTENTIONALLY_EMPTY_ZH_CN.has(k))
const empty = zhValues.filter((k) => {
// resolve k against zhCN...
return typeof resolved !== 'string' || resolved.trim() === ''
})
if (empty.length > 0) {
throw new Error(`zh-CN has unexpected empty value(s): ${empty.join(', ')}`)
}
})This makes the intentional-empty contract explicit and protects against accidental blanks in future translations.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/i18n/__tests__/locale-parity.test.ts
Line: 56-63
Comment:
**Parity test doesn't validate zh-CN values are non-empty**
The fifth assertion (`"all leaf values in en should be non-empty strings"`) only checks the `en` locale. The zh-CN locale can — and does — contain intentional empty strings (e.g., `footer.an: ""`). Without a corresponding check, an accidentally empty zh-CN translation would pass CI silently and render a blank string in the UI (since `t()` only falls back to English for `undefined`, not `""`).
Consider adding a companion test that documents **which** keys may intentionally be empty in zh-CN, and flags unexpected empty values:
```ts
const INTENTIONALLY_EMPTY_ZH_CN = new Set(['footer.an'])
it("zh-CN non-intentional values should be non-empty strings", () => {
const zhValues = zhKeys.filter((k) => !INTENTIONALLY_EMPTY_ZH_CN.has(k))
const empty = zhValues.filter((k) => {
// resolve k against zhCN...
return typeof resolved !== 'string' || resolved.trim() === ''
})
if (empty.length > 0) {
throw new Error(`zh-CN has unexpected empty value(s): ${empty.join(', ')}`)
}
})
```
This makes the intentional-empty contract explicit and protects against accidental blanks in future translations.
How can I resolve this? If you propose a fix, please make it concise.
|
Please do not ping me for PRs. I see them, pinging them will make me less likely to merge. What we really need instead of PRs:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf595f070f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (typeof globalThis.localStorage !== "undefined") { | ||
| const stored = localStorage.getItem("clawhub.locale"); | ||
| if (stored === "en" || stored === "zh-CN") return stored; |
There was a problem hiding this comment.
The locale is initialized from localStorage/navigator inside a process-wide singleton, but server rendering has no access to user localStorage and falls back to English. In this SSR app (TanStack Start + Nitro), users who previously selected zh-CN will receive English HTML/head from the server and then hydrate into Chinese on the client, which causes hydration mismatch/flicker and can force extra client re-rendering on every page load for those users.
Useful? React with 👍 / 👎.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
|
|
||
| <div className="card upload-panel" ref={validationRef}> | ||
| <h2 className="upload-panel-title">Validation</h2> | ||
| <h2 className="upload-panel-title">{t('upload.validation')}</h2> |
There was a problem hiding this comment.
Multiple hardcoded English strings in upload.tsx are not using their defined translation keys, causing Chinese users to see English text.
| ? 'No matching users.' | ||
| : 'No users yet.' | ||
| ? t('management.noMatchingUsers') | ||
| : t('management.noUsers') |
There was a problem hiding this comment.
Hardcoded 'Loading users…' string in userEmptyLabel breaks internationalization
2 participants
Summary
Lightweight i18n infrastructure with Simplified Chinese (zh-CN) as the first non-English locale. Zero new runtime dependencies.
This is v2 of #607, fully addressing the review feedback from @steipete:
What changed from v1
1. Typed translation keys (compile-time safety)
FlattenKeys<T>recursive type derives a union of all valid dot-path keys fromen.tst()now only acceptsTranslationKey— typos are caught at compile timeen.tsusesas const satisfies TranslationMapto preserve literal types while maintaining structure validation2. Locale-aware document shell (
<html>,<head>)<html lang>dynamically reflects the current i18n locale (was hardcoded"en")<title>and<meta name="description">uset()keys (site.skillsName,site.skillsDescription, etc.)3. Locale parity test (vitest)
src/i18n/__tests__/locale-parity.test.ts— 5 assertionsAlso in this PR (from v1)
I18nManagersingleton with dot-path key resolution,{param}interpolation,localStoragepersistence,navigator.languageauto-detection, and English fallbackuseI18nReact hook viauseSyncExternalStorefor reactive re-renderslocalStorage/navigatoraccessTest results
No new TypeScript errors introduced (pre-existing upstream errors unchanged).
Ref: #606