fix(i18n): address PR review feedback
- config: explicit Hans script wins over HK/MO region (zh-Hans-HK -> zh-CN), with regression tests for script-vs-region precedence - dashboard i18n: detection.caches [] so the detector's init-time auto-persist can't masquerade as a user choice and suppress server-settings hydration - LanguageSelector: role=group (radiogroup conflicted with aria-pressed) - bin: validate --lang against SUPPORTED_LOCALES (fail loudly, not silent fallback); help text clarifies the flag is terminal-UI-only - cli i18n test: assert a real fr catalog lookup (defaultValue could mask a catalog that never loaded); comment the async changeLanguage re-init seam - assert-locale-chunks: only dirs containing common.json count as locales - plan doc: merge duplicate Ink 6.8->7.0 risk bullets - AGENTS.md/solution doc: frontmatter field list + normalizer excerpt synced Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
@@ -205,7 +205,7 @@ Scoped exception (FN-5819): shared-branch-group members (`branchContext.assignme
|
||||
- `./docs/soft-delete-verification-matrix.md` — mandatory soft-delete verification matrix.
|
||||
- `./docs/cli-reference.md` — CLI and terminal UI reference.
|
||||
- `./docs/contributing.md` — contributing conventions and release-adjacent context.
|
||||
- `./docs/solutions/` — documented solutions to past problems (bugs, architecture patterns, best practices), organized by category with YAML frontmatter (`module`, `tags`, `problem_type`). Relevant when implementing or debugging in documented areas.
|
||||
- `./docs/solutions/` — documented solutions to past problems (bugs, architecture patterns, best practices), organized by category with YAML frontmatter (`category`, `module`, `tags`, `problem_type`, `applies_when`). Relevant when implementing or debugging in documented areas.
|
||||
- `./CONCEPTS.md` — shared domain vocabulary (entities, named processes, status concepts). Relevant when orienting to the codebase or discussing domain concepts.
|
||||
|
||||
### Lazy-Loaded Heavy Views
|
||||
|
||||
@@ -404,10 +404,9 @@ In scope: the localization runtime for both surfaces, English extraction of user
|
||||
|
||||
## Risks & Dependencies
|
||||
|
||||
- **Ink 6.8 → 7.0 upgrade for CJK width (U6).** Ink 7.0 measures CJK as double-width (via `string-width`) and fixes column/border alignment, but it also reworked input handling — a major bump that needs a TUI regression pass. Mitigation: prefer the upgrade for correctness, but the fallback (stay on 6.8 + explicit `string-width` math at hand-built layout sites) is viable if the input-handling changes prove disruptive. Decide early in U6; treat as the unit's primary risk.
|
||||
- **Ink 6.8 → 7.0 upgrade for CJK width (U6).** Ink 7.0 measures CJK as double-width (via `string-width`) and fixes column/border alignment, but it also reworked input handling (a major bump needing a TUI regression pass) and raises the React peer floor to 19.2. Mitigation: prefer the upgrade for correctness with the React peer bump (U6); fallback is staying on 6.8 with explicit `string-width` math at hand-built layout sites if the input-handling changes prove disruptive. Decide early in U6; treat as the unit's primary risk.
|
||||
- **Vite variable dynamic-import code-splitting (KTD3a/U3).** Vite 6 only code-splits a variable `import()` when the prefix is *relative to the importing file*; an aliased, bare, or cross-package specifier defeats `@rollup/plugin-dynamic-import-vars`, and `import.meta.glob` won't cross `node_modules`. The plan resolves this by making the **app-local generated-catalog path primary** (catalogs synced into a gitignored `app/locales/`, imported by a plainly app-relative template) rather than relying on undocumented cross-package symlink resolution. Residual risk: the dashboard has no existing variable-dynamic-import precedent, so U3 keeps a build assertion that per-locale chunks actually emit, as a regression guard before U5 builds on the runtime.
|
||||
- **react-i18next under the Ink reconciler (KTD1/U6).** The single-runtime decision assumes `useTranslation`/`<Trans>` work under Ink's custom (non-react-dom) reconciler; there is no cited precedent. Mitigation: U6 spikes this before committing the runtime; synchronous `initImmediate: false` init already sidesteps Suspense, and plain `t()` is the fallback if `<Trans>` misbehaves.
|
||||
- **Ink 6.8 → 7.0 upgrade for CJK width (U6).** Ink 7.0 measures CJK as double-width and fixes alignment, but reworked input handling (a major bump needing a TUI regression pass) and raises the React peer floor to 19.2. Mitigation: prefer the upgrade with the peer bump (U6); fallback is staying on 6.8 with explicit `string-width` math.
|
||||
- **CLI ships all locales in the published binary.** tsup `splitting: false` + `noExternal` inlines every catalog into `@runfusion/fusion`'s `dist/bin.js`, growing linearly with translation volume and locale count. Mitigation: accepted tradeoff for v1; keep the `cli` namespace separate so dashboard strings aren't dragged in; lazy fs-loading of copied catalog assets is a future optimization if size becomes material.
|
||||
- **String-extraction surface is large (~464 dashboard files).** Mitigation: incremental per-view migration (U5/U7) with a named-set completion gate, not a big-bang sweep; the plan ships a working foundation + high-traffic coverage and defers the long tail.
|
||||
- **kb→fn rename collision.** Touching user-facing strings during the in-flight rename risks churn. Mitigation: KTD9 (brand tokens as `{{brand}}` variables) and KTD8 (reuse `kb-` localStorage prefix) keep this plan rename-neutral.
|
||||
|
||||
@@ -68,6 +68,7 @@ export function normalizeToSupportedLocale(tag: string): Locale | undefined {
|
||||
if (isLocale(norm)) return norm;
|
||||
const lower = norm.toLowerCase();
|
||||
if (lower.startsWith("zh")) {
|
||||
if (lower.includes("hans")) return "zh-CN"; // explicit script wins over region (zh-Hans-HK)
|
||||
if (lower.includes("hant") || lower.includes("-tw") ||
|
||||
lower.includes("-hk") || lower.includes("-mo")) return "zh-TW";
|
||||
return "zh-CN";
|
||||
|
||||
@@ -420,7 +420,7 @@ Options:
|
||||
--interactive Interactive mode (port selection for dashboard, issue selection for import)
|
||||
--paused Start with engine paused (automation disabled)
|
||||
--dev Start dashboard only (no AI engine)
|
||||
--lang <locale> UI locale for this run (en, zh-CN, zh-TW, fr, es)
|
||||
--lang <locale> Terminal-UI locale for this run (en, zh-CN, zh-TW, fr, es); the browser dashboard resolves its own language
|
||||
--attach <file> Attach file(s) on task create (repeatable)
|
||||
--depends <id> Declare dependency on task create (repeatable)
|
||||
--no-dedup Bypass deterministic duplicate guard on task create
|
||||
@@ -736,6 +736,15 @@ async function main() {
|
||||
const token = dashTokenIdx !== -1 && dashTokenIdx + 1 < args.length ? args[dashTokenIdx + 1] : undefined;
|
||||
const dashLangIdx = args.indexOf("--lang");
|
||||
const lang = dashLangIdx !== -1 && dashLangIdx + 1 < args.length ? args[dashLangIdx + 1] : undefined;
|
||||
if (lang !== undefined) {
|
||||
// Fail loudly on a bad explicit flag instead of silently falling back
|
||||
// to setting/env resolution inside the TUI.
|
||||
const { isLocale, SUPPORTED_LOCALES } = await import("@fusion/core");
|
||||
if (!isLocale(lang)) {
|
||||
console.error(`Invalid --lang "${lang}". Supported: ${SUPPORTED_LOCALES.join(", ")}`);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
await runDashboard(port, { paused, dev, interactive, host, noAuth, token, lang });
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -71,6 +71,23 @@ describe("react-i18next under the Ink reconciler", () => {
|
||||
expect(lastFrame()).toContain("Loading…");
|
||||
});
|
||||
|
||||
it("renders from a real catalog, not the defaultValue fallback", async () => {
|
||||
// t("tui.loading", "Loading…") can't distinguish a loaded catalog from the
|
||||
// inline defaultValue. A non-en lookup with no defaultValue proves the
|
||||
// bundled catalog itself resolved.
|
||||
const i18n = initCliI18n("en");
|
||||
await i18n.changeLanguage("fr");
|
||||
function FrLoading() {
|
||||
const { t } = useTranslation("cli");
|
||||
return createElement(Text, null, t("tui.loading"));
|
||||
}
|
||||
const { lastFrame } = render(
|
||||
createElement(I18nextProvider, { i18n }, createElement(FrLoading)),
|
||||
);
|
||||
expect(lastFrame()).toContain("Chargement…");
|
||||
await i18n.changeLanguage("en");
|
||||
});
|
||||
|
||||
it("re-renders on changeLanguage", async () => {
|
||||
const i18n = initCliI18n("en");
|
||||
cliI18n.addResourceBundle("zh-CN", "cli", { tui: { loading: "加载中…" } }, true, true);
|
||||
|
||||
@@ -65,6 +65,10 @@ export function initCliI18n(locale: Locale): I18nInstance {
|
||||
});
|
||||
initialized = true;
|
||||
} else if (i18next.language !== locale) {
|
||||
// changeLanguage is async; the instance carries the previous locale until
|
||||
// the microtask queue drains. Benign in production (called once per
|
||||
// process) — callers that need the settled locale before rendering should
|
||||
// await i18next.changeLanguage(locale) directly.
|
||||
void i18next.changeLanguage(locale);
|
||||
}
|
||||
return i18next;
|
||||
|
||||
@@ -21,7 +21,9 @@ export function LanguageSelector() {
|
||||
return (
|
||||
<div className="language-selector">
|
||||
<div className="language-selector-title">{label}</div>
|
||||
<div className="language-options" role="radiogroup" aria-label={label}>
|
||||
{/* role="group" + aria-pressed: toggle-button semantics (radiogroup would
|
||||
conflict with aria-pressed and confuse screen readers). */}
|
||||
<div className="language-options" role="group" aria-label={label}>
|
||||
{supportedLocales.map((locale) => (
|
||||
<button
|
||||
key={locale}
|
||||
|
||||
@@ -41,7 +41,12 @@ export const i18nReady = i18next.init({
|
||||
detection: {
|
||||
order: ["localStorage", "navigator", "htmlTag"],
|
||||
lookupLocalStorage: LANGUAGE_STORAGE_KEY,
|
||||
caches: ["localStorage"],
|
||||
// Don't auto-persist the *detected* locale: the detector would write the
|
||||
// storage key during init, and useLanguage distinguishes a real user choice
|
||||
// from mere detection by this key's presence (its server-settings hydration
|
||||
// tier only fires when no local choice exists). setLanguage() writes the
|
||||
// key explicitly on actual user choices.
|
||||
caches: [],
|
||||
// Normalize multi-subtag detections (e.g. navigator "zh-Hans-CN" /
|
||||
// "zh-Hant-TW") to a supported locale before fallback, matching the CLI's
|
||||
// env detection.
|
||||
|
||||
@@ -15,10 +15,13 @@ const i18nRoot = join(here, "..", "..", "i18n");
|
||||
const namespaces = JSON.parse(readFileSync(join(i18nRoot, "namespaces.json"), "utf8")).dashboard;
|
||||
|
||||
// Derive the expected per-namespace chunk floor from the authored locale set
|
||||
// rather than hardcoding a count, so adding a locale needs no edit here.
|
||||
// rather than hardcoding a count, so adding a locale needs no edit here. Only
|
||||
// directories that actually contain the default-namespace catalog count as
|
||||
// locales, so a stray dir (editor artifact, partial sync) can't inflate the
|
||||
// floor and produce a spurious failure.
|
||||
const localesDir = join(i18nRoot, "locales");
|
||||
const expectedLocaleCount = readdirSync(localesDir, { withFileTypes: true }).filter(
|
||||
(d) => d.isDirectory(),
|
||||
(d) => d.isDirectory() && existsSync(join(localesDir, d.name, "common.json")),
|
||||
).length;
|
||||
|
||||
if (!existsSync(assetsDir)) {
|
||||
|
||||
@@ -79,6 +79,15 @@ describe("normalizeToSupportedLocale", () => {
|
||||
expect(normalizeToSupportedLocale("zh-SG")).toBe("zh-CN");
|
||||
});
|
||||
|
||||
it("lets an explicit Simplified script win over a Traditional-leaning region", () => {
|
||||
// zh-Hans-HK / zh-Hans-MO are valid BCP-47: Simplified script in HK/Macau.
|
||||
expect(normalizeToSupportedLocale("zh-Hans-HK")).toBe("zh-CN");
|
||||
expect(normalizeToSupportedLocale("zh-Hans-MO")).toBe("zh-CN");
|
||||
expect(normalizeToSupportedLocale("zh_Hans_HK")).toBe("zh-CN");
|
||||
// And the reverse stays Traditional regardless of region.
|
||||
expect(normalizeToSupportedLocale("zh-Hant-CN")).toBe("zh-TW");
|
||||
});
|
||||
|
||||
it("strips region subtags on other languages and rejects unsupported", () => {
|
||||
expect(normalizeToSupportedLocale("fr-FR")).toBe("fr");
|
||||
expect(normalizeToSupportedLocale("es-419")).toBe("es");
|
||||
|
||||
@@ -58,6 +58,9 @@ export function normalizeToSupportedLocale(tag: string): Locale | undefined {
|
||||
|
||||
const lower = norm.toLowerCase();
|
||||
if (lower.startsWith("zh")) {
|
||||
// An explicit Simplified script subtag wins over region: zh-Hans-HK /
|
||||
// zh-Hans-MO are valid BCP-47 for Simplified Chinese used in HK/Macau.
|
||||
if (lower.includes("hans")) return "zh-CN";
|
||||
if (
|
||||
lower.includes("hant") ||
|
||||
lower.includes("-tw") ||
|
||||
|
||||
Reference in New Issue
Block a user