From 4f6013ff3a04167adb67129cc75f6261fd872837 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 09:03:16 -0700 Subject: [PATCH] test+refactor(i18n): close code-review residuals - Extract a shared normalizeToSupportedLocale helper in @fusion/i18n used by both the CLI env detection and the dashboard navigator detection (convertDetectedLanguage), fixing multi-subtag navigator tags (zh-Hans-CN) and unifying Chinese script resolution. - Add a cross-tab storage listener to useLanguage so a language change in one tab propagates to others. - Dedup the namespace lists into packages/i18n/namespaces.json, consumed by config.ts and all three build scripts (no more 3-way drift risk). - Add tests: dashboard i18n/index.ts (document.lang mirror, init non-blocking), useLanguage concurrent-hydration race + cross-tab adoption, normalizeToSupportedLocale, namespace-source consistency. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/cli/src/i18n/index.ts | 29 +++--------- .../app/hooks/__tests__/useLanguage.test.ts | 24 ++++++++++ packages/dashboard/app/hooks/useLanguage.ts | 19 ++++++++ .../app/i18n/__tests__/index.test.ts | 26 +++++++++++ packages/dashboard/app/i18n/index.ts | 11 ++++- .../scripts/assert-locale-chunks.mjs | 6 ++- packages/dashboard/scripts/sync-locales.mjs | 11 +++-- packages/i18n/namespaces.json | 5 +++ packages/i18n/scripts/gen-cli-catalogs.mjs | 6 +-- packages/i18n/src/__tests__/config.test.ts | 37 +++++++++++++++ packages/i18n/src/config.ts | 45 ++++++++++++++++--- packages/i18n/tsconfig.json | 2 +- 12 files changed, 180 insertions(+), 41 deletions(-) create mode 100644 packages/dashboard/app/i18n/__tests__/index.test.ts create mode 100644 packages/i18n/namespaces.json diff --git a/packages/cli/src/i18n/index.ts b/packages/cli/src/i18n/index.ts index d382c40396..d5abfe73a6 100644 --- a/packages/cli/src/i18n/index.ts +++ b/packages/cli/src/i18n/index.ts @@ -4,6 +4,7 @@ import { CLI_NAMESPACES, cliResources, DEFAULT_NAMESPACE, + normalizeToSupportedLocale, } from "@fusion/i18n"; import i18next, { type i18n as I18nInstance, type Resource } from "i18next"; import { initReactI18next } from "react-i18next"; @@ -24,30 +25,10 @@ import { initReactI18next } from "react-i18next"; export function detectEnvLocale(env: NodeJS.ProcessEnv = process.env): Locale | undefined { const raw = env.LC_ALL || env.LC_MESSAGES || env.LANG || env.LANGUAGE; if (!raw) return undefined; - - // Strip encoding/modifier (`.UTF-8`, `@euro`) and normalize `_` to `-`. - const code = raw.split(/[.:@\s]/)[0].replaceAll("_", "-"); - if (isLocale(code)) return code; - - const lower = code.toLowerCase(); - // Script/region-aware Chinese resolution BEFORE the bare-language fallback, - // so Traditional-script tags (zh_Hant, zh_Hant_TW, zh_HK, zh_MO) are not - // silently served Simplified. Region-only zh_CN/zh_TW already matched above. - if (lower.startsWith("zh")) { - if ( - lower.includes("hant") || - lower.includes("-tw") || - lower.includes("-hk") || - lower.includes("-mo") - ) { - return "zh-TW"; - } - return "zh-CN"; - } - - const lang = lower.split("-")[0]; - if (isLocale(lang)) return lang; - return undefined; + // Strip encoding/modifier (`.UTF-8`, `@euro`), then normalize via the shared + // helper so env detection matches the dashboard's navigator detection + // (incl. Traditional-script Chinese → zh-TW). + return normalizeToSupportedLocale(raw.split(/[.:@\s]/)[0]); } /** diff --git a/packages/dashboard/app/hooks/__tests__/useLanguage.test.ts b/packages/dashboard/app/hooks/__tests__/useLanguage.test.ts index 365b1537ed..99101ed096 100644 --- a/packages/dashboard/app/hooks/__tests__/useLanguage.test.ts +++ b/packages/dashboard/app/hooks/__tests__/useLanguage.test.ts @@ -100,6 +100,30 @@ describe("useLanguage", () => { expect(i18nMock.changeLanguage).not.toHaveBeenCalledWith("es"); }); + it("does not let in-flight server hydration override a concurrent user choice", async () => { + let resolveFetch!: (s: Settings) => void; + mockFetch.mockImplementation(() => new Promise((r) => { resolveFetch = r; })); + const { result } = renderHook(() => useLanguage()); + // User picks fr while the hydration fetch is still pending (sets userSetRef). + act(() => result.current.setLanguage("fr")); + i18nMock.changeLanguage.mockClear(); + // Now the server responds with a different locale — must be ignored. + resolveFetch({ language: "es" } as Settings); + await new Promise((r) => setTimeout(r, 0)); + expect(i18nMock.changeLanguage).not.toHaveBeenCalledWith("es"); + }); + + it("adopts a language change made in another tab via the storage event", () => { + renderHook(() => useLanguage()); + i18nMock.changeLanguage.mockClear(); + act(() => { + window.dispatchEvent( + new StorageEvent("storage", { key: LANGUAGE_STORAGE_KEY, newValue: "zh-TW" }), + ); + }); + expect(i18nMock.changeLanguage).toHaveBeenCalledWith("zh-TW"); + }); + it("degrades gracefully when localStorage is unavailable", () => { vi.stubGlobal("localStorage", { getItem: () => null, diff --git a/packages/dashboard/app/hooks/useLanguage.ts b/packages/dashboard/app/hooks/useLanguage.ts index 6ab39df2e3..37b4f37146 100644 --- a/packages/dashboard/app/hooks/useLanguage.ts +++ b/packages/dashboard/app/hooks/useLanguage.ts @@ -49,6 +49,25 @@ export function useLanguage(): UseLanguageReturn { }; }, [i18n]); + // Keep tabs in sync: when another tab changes the persisted language, adopt it + // here too (the storage event only fires in *other* tabs, so this never loops). + useEffect(() => { + if (!isBrowser) return; + const onStorage = (event: StorageEvent) => { + if ( + event.key === LANGUAGE_STORAGE_KEY && + isLocale(event.newValue) && + event.newValue !== i18n.resolvedLanguage + ) { + void i18n.changeLanguage(event.newValue); + } + }; + window.addEventListener("storage", onStorage); + return () => { + window.removeEventListener("storage", onStorage); + }; + }, [i18n]); + // Hydrate from server settings, but never override a local/user choice. useEffect(() => { if (!isBrowser) return; diff --git a/packages/dashboard/app/i18n/__tests__/index.test.ts b/packages/dashboard/app/i18n/__tests__/index.test.ts new file mode 100644 index 0000000000..3d75801357 --- /dev/null +++ b/packages/dashboard/app/i18n/__tests__/index.test.ts @@ -0,0 +1,26 @@ +import { beforeAll, describe, expect, it } from "vitest"; +import i18n, { i18nReady, LANGUAGE_STORAGE_KEY } from "../index"; + +// Exercises the real dashboard i18next instance (not mocked): the languageChanged +// handler that mirrors the active locale onto and the storage key. +describe("dashboard i18n runtime", () => { + beforeAll(async () => { + await i18nReady; + }); + + it("initializes to a supported locale and never blocks the app", () => { + // i18nReady resolved above; init did not reject (main.tsx relies on this). + expect(i18n.isInitialized).toBe(true); + }); + + it("uses the kb-dashboard-language storage key", () => { + expect(LANGUAGE_STORAGE_KEY).toBe("kb-dashboard-language"); + }); + + it("mirrors the active locale onto document.documentElement.lang", async () => { + await i18n.changeLanguage("fr"); + expect(document.documentElement.lang).toBe("fr"); + await i18n.changeLanguage("zh-TW"); + expect(document.documentElement.lang).toBe("zh-TW"); + }); +}); diff --git a/packages/dashboard/app/i18n/index.ts b/packages/dashboard/app/i18n/index.ts index ef1d498f92..00543f80f8 100644 --- a/packages/dashboard/app/i18n/index.ts +++ b/packages/dashboard/app/i18n/index.ts @@ -1,4 +1,9 @@ -import { baseInitOptions, DASHBOARD_NAMESPACES, DEFAULT_NAMESPACE } from "@fusion/i18n/config"; +import { + baseInitOptions, + DASHBOARD_NAMESPACES, + DEFAULT_NAMESPACE, + normalizeToSupportedLocale, +} from "@fusion/i18n/config"; import i18next from "i18next"; import LanguageDetector from "i18next-browser-languagedetector"; import resourcesToBackend from "i18next-resources-to-backend"; @@ -37,6 +42,10 @@ export const i18nReady = i18next.init({ order: ["localStorage", "navigator", "htmlTag"], lookupLocalStorage: LANGUAGE_STORAGE_KEY, caches: ["localStorage"], + // 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. + convertDetectedLanguage: (lng: string) => normalizeToSupportedLocale(lng) ?? lng, }, react: { // First paint is gated on `i18nReady` in main.tsx, so Suspense is not diff --git a/packages/dashboard/scripts/assert-locale-chunks.mjs b/packages/dashboard/scripts/assert-locale-chunks.mjs index 1c2ba1031b..8f23fb0265 100644 --- a/packages/dashboard/scripts/assert-locale-chunks.mjs +++ b/packages/dashboard/scripts/assert-locale-chunks.mjs @@ -10,11 +10,13 @@ import { fileURLToPath } from "node:url"; const here = dirname(fileURLToPath(import.meta.url)); const assetsDir = join(here, "..", "dist", "client", "assets"); -const namespaces = ["common", "app", "errors"]; +const i18nRoot = join(here, "..", "..", "i18n"); +// Single source of truth shared with @fusion/i18n config.ts. +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. -const localesDir = join(here, "..", "..", "i18n", "locales"); +const localesDir = join(i18nRoot, "locales"); const expectedLocaleCount = readdirSync(localesDir, { withFileTypes: true }).filter( (d) => d.isDirectory(), ).length; diff --git a/packages/dashboard/scripts/sync-locales.mjs b/packages/dashboard/scripts/sync-locales.mjs index 31e89bdc29..7def8edd37 100644 --- a/packages/dashboard/scripts/sync-locales.mjs +++ b/packages/dashboard/scripts/sync-locales.mjs @@ -4,17 +4,20 @@ // generated app/locales/ dir is gitignored — @fusion/i18n/locales is the // source-of-truth. Only the dashboard namespaces are copied (the terminal-only // `cli` namespace is skipped). Runs as a predev/prebuild step. -import { cpSync, mkdirSync, readdirSync, rmSync } from "node:fs"; +import { cpSync, mkdirSync, readdirSync, readFileSync, rmSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; const here = dirname(fileURLToPath(import.meta.url)); const dashboardRoot = join(here, ".."); -const srcLocales = join(dashboardRoot, "..", "i18n", "locales"); +const i18nRoot = join(dashboardRoot, "..", "i18n"); +const srcLocales = join(i18nRoot, "locales"); const destLocales = join(dashboardRoot, "app", "locales"); -// Keep in sync with DASHBOARD_NAMESPACES in @fusion/i18n config.ts. -const DASHBOARD_NAMESPACES = ["common", "app", "errors"]; +// Single source of truth shared with @fusion/i18n config.ts. +const DASHBOARD_NAMESPACES = JSON.parse( + readFileSync(join(i18nRoot, "namespaces.json"), "utf8"), +).dashboard; const locales = readdirSync(srcLocales, { withFileTypes: true }) .filter((d) => d.isDirectory()) diff --git a/packages/i18n/namespaces.json b/packages/i18n/namespaces.json new file mode 100644 index 0000000000..f34872606c --- /dev/null +++ b/packages/i18n/namespaces.json @@ -0,0 +1,5 @@ +{ + "all": ["common", "app", "errors", "cli"], + "dashboard": ["common", "app", "errors"], + "cli": ["common", "cli", "errors"] +} diff --git a/packages/i18n/scripts/gen-cli-catalogs.mjs b/packages/i18n/scripts/gen-cli-catalogs.mjs index 8739db3d93..e47ec92b4c 100644 --- a/packages/i18n/scripts/gen-cli-catalogs.mjs +++ b/packages/i18n/scripts/gen-cli-catalogs.mjs @@ -5,7 +5,7 @@ // map rather than a dynamic loader. Driving this off the locales/ directory // listing keeps "add a language" a no-code operation on the CLI side: add the // locale (via `i18next-cli sync`), regenerate, done. -import { readdirSync, writeFileSync } from "node:fs"; +import { readdirSync, readFileSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -13,8 +13,8 @@ const here = dirname(fileURLToPath(import.meta.url)); const root = join(here, ".."); const localesDir = join(root, "locales"); -// Keep in sync with CLI_NAMESPACES in src/config.ts. -const CLI_NAMESPACES = ["common", "cli", "errors"]; +// Single source of truth shared with src/config.ts and the dashboard scripts. +const CLI_NAMESPACES = JSON.parse(readFileSync(join(root, "namespaces.json"), "utf8")).cli; const locales = readdirSync(localesDir, { withFileTypes: true }) .filter((d) => d.isDirectory()) diff --git a/packages/i18n/src/__tests__/config.test.ts b/packages/i18n/src/__tests__/config.test.ts index 91836e5c90..806eee83e8 100644 --- a/packages/i18n/src/__tests__/config.test.ts +++ b/packages/i18n/src/__tests__/config.test.ts @@ -8,6 +8,7 @@ import { DEFAULT_NAMESPACE, FALLBACK_LNG, NAMESPACES, + normalizeToSupportedLocale, } from "../config.js"; describe("@fusion/i18n config", () => { @@ -48,4 +49,40 @@ describe("@fusion/i18n config", () => { expect(cliResources.en.cli).toMatchObject({ tui: { loading: expect.any(String) } }); expect(cliResources.en.common).toMatchObject({ columns: { done: "Done" } }); }); + + it("keeps dashboard/cli namespace lists as subsets of the canonical set", () => { + for (const ns of [...DASHBOARD_NAMESPACES, ...CLI_NAMESPACES]) { + expect(NAMESPACES).toContain(ns); + } + // The shared json source drives all three: config + the two build scripts. + expect([...NAMESPACES]).toEqual(["common", "app", "errors", "cli"]); + }); +}); + +describe("normalizeToSupportedLocale", () => { + it("passes through exact supported codes", () => { + expect(normalizeToSupportedLocale("en")).toBe("en"); + expect(normalizeToSupportedLocale("zh-CN")).toBe("zh-CN"); + expect(normalizeToSupportedLocale("zh-TW")).toBe("zh-TW"); + }); + + it("resolves Traditional-script/region Chinese to zh-TW", () => { + expect(normalizeToSupportedLocale("zh-Hant")).toBe("zh-TW"); + expect(normalizeToSupportedLocale("zh-Hant-TW")).toBe("zh-TW"); + expect(normalizeToSupportedLocale("zh_HK")).toBe("zh-TW"); + expect(normalizeToSupportedLocale("zh-MO")).toBe("zh-TW"); + }); + + it("resolves Simplified/other Chinese to zh-CN", () => { + expect(normalizeToSupportedLocale("zh")).toBe("zh-CN"); + expect(normalizeToSupportedLocale("zh-Hans-CN")).toBe("zh-CN"); + expect(normalizeToSupportedLocale("zh-SG")).toBe("zh-CN"); + }); + + it("strips region subtags on other languages and rejects unsupported", () => { + expect(normalizeToSupportedLocale("fr-FR")).toBe("fr"); + expect(normalizeToSupportedLocale("es-419")).toBe("es"); + expect(normalizeToSupportedLocale("de-DE")).toBeUndefined(); + expect(normalizeToSupportedLocale("")).toBeUndefined(); + }); }); diff --git a/packages/i18n/src/config.ts b/packages/i18n/src/config.ts index 016ddec593..447c7ebdce 100644 --- a/packages/i18n/src/config.ts +++ b/packages/i18n/src/config.ts @@ -1,5 +1,6 @@ -import { DEFAULT_LOCALE, SUPPORTED_LOCALES } from "@fusion/core"; +import { DEFAULT_LOCALE, isLocale, type Locale, SUPPORTED_LOCALES } from "@fusion/core"; import type { FallbackLngObjList, InitOptions } from "i18next"; +import namespaces from "../namespaces.json"; /** * Shared, framework-agnostic i18next configuration for both Fusion UI surfaces. @@ -9,18 +10,20 @@ import type { FallbackLngObjList, InitOptions } from "i18next"; * chain, and base options defined here so the two surfaces stay consistent. */ -/** All translation namespaces. Split so each surface loads only what it needs. */ -export const NAMESPACES = ["common", "app", "errors", "cli"] as const; -export type Namespace = (typeof NAMESPACES)[number]; +/** All translation namespaces. Split so each surface loads only what it needs. + * Sourced from namespaces.json so the build scripts and this config can never + * drift (the scripts read the same JSON). */ +export type Namespace = "common" | "app" | "errors" | "cli"; +export const NAMESPACES = namespaces.all as readonly Namespace[]; /** Default namespace keys resolve against when none is specified. */ export const DEFAULT_NAMESPACE: Namespace = "common"; /** Namespaces the browser dashboard loads (skips the terminal-only `cli`). */ -export const DASHBOARD_NAMESPACES: readonly Namespace[] = ["common", "app", "errors"]; +export const DASHBOARD_NAMESPACES = namespaces.dashboard as readonly Namespace[]; /** Namespaces the terminal UI loads (skips the dashboard-only `app`). */ -export const CLI_NAMESPACES: readonly Namespace[] = ["common", "cli", "errors"]; +export const CLI_NAMESPACES = namespaces.cli as readonly Namespace[]; /** * Script-aware fallback chain. A generic `zh` resolves to Simplified, the @@ -40,6 +43,36 @@ export const FALLBACK_LNG: FallbackLngObjList = { * adds its own resource-loading strategy (lazy backend for the dashboard, * static `resources` for the CLI) plus framework plugins. */ +/** + * Normalize a BCP-47-ish or POSIX-ish language tag to a supported {@link Locale}, + * or undefined when nothing matches. Shared by the dashboard (navigator + * detection) and the CLI (env detection) so Chinese script/region resolution is + * identical on both surfaces — Traditional tags (zh-Hant, zh-TW, zh-HK, zh-MO) + * resolve to zh-TW, everything else Chinese to zh-CN, and region subtags on + * other languages strip to the base. + */ +export function normalizeToSupportedLocale(tag: string): Locale | undefined { + if (!tag) return undefined; + const norm = tag.replaceAll("_", "-"); + if (isLocale(norm)) return norm; + + const lower = norm.toLowerCase(); + if (lower.startsWith("zh")) { + if ( + lower.includes("hant") || + lower.includes("-tw") || + lower.includes("-hk") || + lower.includes("-mo") + ) { + return "zh-TW"; + } + return "zh-CN"; + } + + const base = lower.split("-")[0]; + return isLocale(base) ? base : undefined; +} + export function baseInitOptions(): InitOptions { return { supportedLngs: [...SUPPORTED_LOCALES], diff --git a/packages/i18n/tsconfig.json b/packages/i18n/tsconfig.json index b55e80cea4..bcf9e98e40 100644 --- a/packages/i18n/tsconfig.json +++ b/packages/i18n/tsconfig.json @@ -6,6 +6,6 @@ "types": ["node", "vitest/globals"], "resolveJsonModule": true }, - "include": ["src/**/*", "locales/**/*.json"], + "include": ["src/**/*", "locales/**/*.json", "namespaces.json"], "exclude": ["src/**/*.test.ts"] }