From cb5ef0699c3786914fb58a226a5acb052dcffebb Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 04:55:08 +0900 Subject: [PATCH 1/6] perf(webview): load locales on demand instead of eager-globbing all 18 What: `webview-ui/src/i18n/setup.ts` now eager-globs only `./locales/en/*.json` into the entry chunk and keeps a lazy `import.meta.glob` map for the other 17 locales. `loadLanguage(language)` fetches a locale's namespaces on first use, memoizes in-flight requests, and resolves on fetch failure so a broken chunk degrades to the English fallback instead of blocking the view. `TranslationProvider` loads the active language before calling `i18n.changeLanguage`, because that call is what emits `languageChanged`. Why: the eager glob pulled all 162 locale JSON files into the webview entry chunk on every first paint, including the 17 locales a given user cannot read. Impact: webview entry `assets/index.js` 6,187,772 B -> 4,004,079 B raw (-2,183,693 B, -35.29%) and 1,779,406 B -> 1,128,289 B gzip (-658,117 B, -36.99%). The 153 locale chunks (2,165,392 B total) are now fetched only when the user's language needs them. Language switching still works: the 4 new provider tests and 5 new setup tests cover initial non-English render, runtime switch, missing-locale fallback, and concurrent-fetch dedupe. --- webview-ui/playwright/AppProviders.tsx | 3 + webview-ui/src/i18n/TranslationContext.tsx | 30 ++-- ...TranslationContext.locale-loading.spec.tsx | 102 +++++++++++++ .../__tests__/TranslationContext.spec.tsx | 2 + webview-ui/src/i18n/__tests__/setup.spec.ts | 36 +++++ webview-ui/src/i18n/setup.ts | 139 ++++++++++++++---- 6 files changed, 273 insertions(+), 39 deletions(-) create mode 100644 webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx create mode 100644 webview-ui/src/i18n/__tests__/setup.spec.ts diff --git a/webview-ui/playwright/AppProviders.tsx b/webview-ui/playwright/AppProviders.tsx index 906c34ec47..3742a07770 100644 --- a/webview-ui/playwright/AppProviders.tsx +++ b/webview-ui/playwright/AppProviders.tsx @@ -9,6 +9,9 @@ import i18next, { loadTranslations } from "@/i18n/setup" import { TooltipProvider } from "@/components/ui/tooltip" import { TranslationContext as PlaywrightTranslationContext } from "@src/i18n/TranslationContext" +// The fallback locale registers itself when `setup` loads; this keeps the call +// explicit so a gallery story that swaps `language` still starts from a +// populated store. loadTranslations() type InitialState = NonNullable["initialState"]> diff --git a/webview-ui/src/i18n/TranslationContext.tsx b/webview-ui/src/i18n/TranslationContext.tsx index f2950ae2f8..1450a73241 100644 --- a/webview-ui/src/i18n/TranslationContext.tsx +++ b/webview-ui/src/i18n/TranslationContext.tsx @@ -1,6 +1,6 @@ import React, { createContext, useContext, ReactNode, useEffect, useCallback } from "react" import { useTranslation } from "react-i18next" -import i18next, { loadTranslations } from "./setup" +import i18next, { FALLBACK_LANGUAGE, loadLanguage } from "./setup" import { useExtensionState } from "@/context/ExtensionStateContext" // Create context for translations @@ -18,19 +18,27 @@ const TranslationProvider: React.FC<{ children: ReactNode }> = ({ children }) => const { i18n } = useTranslation() // Get the extension state directly - it already contains all state properties const extensionState = useExtensionState() + const language = extensionState.language || FALLBACK_LANGUAGE - // Load translations once when the component mounts + // Only the fallback locale ships in the entry chunk, so any other language + // has to fetch its bundles before i18next can resolve a key. Switching after + // the fetch is deliberate: `changeLanguage` is what emits `languageChanged`, + // the event `useTranslation` subscribes to for re-renders. Until it fires the + // subtree keeps resolving keys through `fallbackLng`, the same + // fallback-then-swap shape this provider had when every locale was bundled. useEffect(() => { - try { - loadTranslations() - } catch (error) { - console.error("Failed to load translations:", error) - } - }, []) + let cancelled = false - useEffect(() => { - i18n.changeLanguage(extensionState.language) - }, [i18n, extensionState.language]) + void loadLanguage(language).then(() => { + if (!cancelled) { + void i18n.changeLanguage(language) + } + }) + + return () => { + cancelled = true + } + }, [i18n, language]) // Memoize the translation function to prevent unnecessary re-renders const translate = useCallback( diff --git a/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx new file mode 100644 index 0000000000..93be04b688 --- /dev/null +++ b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx @@ -0,0 +1,102 @@ +import { act, render, screen, waitFor } from "@testing-library/react" +import React from "react" + +import i18next from "../setup" +import TranslationProvider, { useAppTranslation } from "../TranslationContext" + +// The provider reads the active language from extension state. Tests drive it +// through a mutable holder so a rerender can simulate a settings change. +const extensionState: { language: string } = { language: "en" } + +vi.mock("@/context/ExtensionStateContext", () => ({ + useExtensionState: () => extensionState, +})) + +const Probe = () => { + const { t, i18n } = useAppTranslation() + + return ( +
+ {t("common:errorBoundary.title")} + {i18n.language} +
+ ) +} + +const renderProbe = (language: string) => { + extensionState.language = language + + return render( + + + , + ) +} + +describe("TranslationProvider locale loading", () => { + beforeEach(async () => { + // i18next is a module-level singleton, so a language left active by an + // earlier test would mask the fallback path this suite is checking. + await i18next.changeLanguage("en") + }) + + it("renders translated strings for a non-English initial language", async () => { + renderProbe("ko") + + // Korean bundles are not in the entry chunk, so the first paint resolves + // through the English fallback and the switch lands after the fetch. + await waitFor(() => { + expect(screen.getByTestId("error-title")).toHaveTextContent("문제가 발생했습니다") + }) + expect(screen.getByTestId("active-language")).toHaveTextContent("ko") + }) + + it("loads a non-eager locale when the language changes at runtime", async () => { + const { rerender } = renderProbe("en") + + expect(screen.getByTestId("error-title")).toHaveTextContent("Something went wrong") + + extensionState.language = "ja" + await act(async () => { + rerender( + + + , + ) + }) + + await waitFor(() => { + expect(screen.getByTestId("error-title")).toHaveTextContent("問題が発生しました") + }) + expect(screen.getByTestId("active-language")).toHaveTextContent("ja") + }) + + it("falls back to English when the requested locale has no resources", async () => { + renderProbe("xx-unsupported") + + await waitFor(() => { + expect(screen.getByTestId("error-title")).toHaveTextContent("Something went wrong") + }) + }) + + it("keeps English available after switching away and back", async () => { + const { rerender } = renderProbe("de") + + await waitFor(() => { + expect(screen.getByTestId("error-title")).toHaveTextContent("Etwas ist schiefgelaufen") + }) + + extensionState.language = "en" + await act(async () => { + rerender( + + + , + ) + }) + + await waitFor(() => { + expect(screen.getByTestId("error-title")).toHaveTextContent("Something went wrong") + }) + }) +}) diff --git a/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx b/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx index cdf0c21acf..38fa9c0b80 100644 --- a/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx +++ b/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx @@ -36,6 +36,8 @@ vi.mock("../setup", () => ({ }, changeLanguage: vi.fn(), }, + FALLBACK_LANGUAGE: "en", + loadLanguage: vi.fn(() => Promise.resolve()), loadTranslations: vi.fn(), })) diff --git a/webview-ui/src/i18n/__tests__/setup.spec.ts b/webview-ui/src/i18n/__tests__/setup.spec.ts new file mode 100644 index 0000000000..6ae102dc60 --- /dev/null +++ b/webview-ui/src/i18n/__tests__/setup.spec.ts @@ -0,0 +1,36 @@ +import i18next, { FALLBACK_LANGUAGE, loadLanguage } from "../setup" + +describe("i18n setup locale loading", () => { + it("ships the fallback locale eagerly", () => { + expect(i18next.hasResourceBundle(FALLBACK_LANGUAGE, "common")).toBe(true) + expect(i18next.hasResourceBundle(FALLBACK_LANGUAGE, "settings")).toBe(true) + }) + + it("does not ship non-fallback locales in the entry graph", () => { + // `hasResourceBundle` is false until `loadLanguage` fetches a chunk, which + // is what keeps the 17 other locales out of the first paint. + for (const language of ["ko", "ja", "de", "zh-CN"]) { + expect(i18next.hasResourceBundle(language, "common")).toBe(false) + } + }) + + it("registers every namespace of a lazily loaded locale", async () => { + await loadLanguage("ko") + + expect(i18next.hasResourceBundle("ko", "common")).toBe(true) + expect(i18next.hasResourceBundle("ko", "settings")).toBe(true) + expect(i18next.t("common:errorBoundary.title", { lng: "ko" })).toBe("문제가 발생했습니다") + }) + + it("shares one fetch across concurrent requests for the same locale", async () => { + const results = await Promise.all([loadLanguage("ja"), loadLanguage("ja"), loadLanguage("ja")]) + + expect(results).toHaveLength(3) + expect(i18next.t("common:errorBoundary.title", { lng: "ja" })).toBe("問題が発生しました") + }) + + it("resolves without throwing for a locale that has no bundles", async () => { + await expect(loadLanguage("xx-unsupported")).resolves.toBeUndefined() + expect(i18next.t("common:errorBoundary.title", { lng: "xx-unsupported" })).toBe("Something went wrong") + }) +}) diff --git a/webview-ui/src/i18n/setup.ts b/webview-ui/src/i18n/setup.ts index 678cdc1d49..7b83d605e2 100644 --- a/webview-ui/src/i18n/setup.ts +++ b/webview-ui/src/i18n/setup.ts @@ -1,54 +1,137 @@ import i18next from "i18next" import { initReactI18next } from "react-i18next" -// Build translations object -const translations: Record> = {} +/** + * The only locale bundled into the webview entry chunk. Every other locale is + * fetched on demand by `loadLanguage` so first paint does not pay for + * translations the user may never read. + */ +export const FALLBACK_LANGUAGE = "en" -// Dynamically load locale files -const localeFiles = import.meta.glob("./locales/**/*.json", { eager: true }) +type LocaleResources = Record +type LocaleModule = { default?: LocaleResources } & LocaleResources -// Process all locale files -Object.entries(localeFiles).forEach(([path, module]) => { - // Extract language and namespace from path - // Example path: './locales/en/common.json' -> language: 'en', namespace: 'common' - const match = path.match(/\.\/locales\/([^/]+)\/([^/]+)\.json/) +// Example path: './locales/en/common.json' -> language: 'en', namespace: 'common' +const LOCALE_PATH_PATTERN = /\.\/locales\/([^/]+)\/([^/]+)\.json$/ - if (match) { - const [, language, namespace] = match +const eagerLocaleFiles = import.meta.glob("./locales/en/*.json", { eager: true }) +const lazyLocaleFiles = import.meta.glob("./locales/**/*.json") - // Initialize language object if it doesn't exist - if (!translations[language]) { - translations[language] = {} +const pendingLanguages = new Map>() +const loadedLanguages = new Set([FALLBACK_LANGUAGE]) + +function parseLocalePath(path: string) { + const match = path.match(LOCALE_PATH_PATTERN) + + return match ? { language: match[1], namespace: match[2] } : undefined +} + +function collectLocaleResources(files: Record) { + const resources: Record> = {} + + Object.entries(files).forEach(([path, module]) => { + const parsed = parseLocalePath(path) + + if (!parsed) { + return } - // Add namespace resources to language - translations[language][namespace] = (module as any).default || module + const { language, namespace } = parsed + resources[language] = resources[language] || {} + resources[language][namespace] = module.default || module + }) + + return resources +} + +/** + * Register a locale's namespaces with i18next, fetching them on first use. + * + * Resolves even when the fetch fails so callers fall back to + * `FALLBACK_LANGUAGE` through i18next's `fallbackLng` instead of blocking the + * UI on a broken chunk. Concurrent calls for the same language share one fetch. + */ +export function loadLanguage(language: string): Promise { + if (loadedLanguages.has(language)) { + return Promise.resolve() + } + + const pending = pendingLanguages.get(language) + + if (pending) { + return pending } -}) -console.log("Dynamically loaded translations:", Object.keys(translations)) + const loaders = Object.entries(lazyLocaleFiles) + .filter(([path]) => parseLocalePath(path)?.language === language) + .map(([path, load]) => load().then((module) => ({ path, module }))) + + // Unknown language: leave the fallback bundles in place so i18next resolves + // every key through `fallbackLng`. + if (loaders.length === 0) { + loadedLanguages.add(language) + return Promise.resolve() + } + + const promise = Promise.all(loaders) + .then((modules) => { + modules.forEach(({ path, module }) => { + const parsed = parseLocalePath(path) + + if (!parsed) { + return + } + + i18next.addResourceBundle(parsed.language, parsed.namespace, module.default || module, true, true) + }) + + loadedLanguages.add(language) + }) + .catch((error) => { + console.warn(`Could not load ${language} translations:`, error) + }) + .finally(() => { + pendingLanguages.delete(language) + }) + + pendingLanguages.set(language, promise) + + return promise +} // Initialize i18next for React // This will be initialized with the VSCode language in TranslationProvider i18next.use(initReactI18next).init({ - lng: "en", // Default language (will be overridden) - fallbackLng: "en", + lng: FALLBACK_LANGUAGE, // Default language (will be overridden) + fallbackLng: FALLBACK_LANGUAGE, debug: false, + // Handed to `init` rather than added afterwards so the first render already + // resolves keys; `addResourceBundle` before init settles would be dropped. + resources: collectLocaleResources(eagerLocaleFiles), interpolation: { escapeValue: false, // React already escapes by default }, }) +/** + * Re-register the eagerly bundled fallback locale. `init` above already has it, + * so this is only needed by callers that reset the store (the Playwright + * gallery mounts stories against a shared i18next instance). + */ export function loadTranslations() { - Object.entries(translations).forEach(([lang, namespaces]) => { - try { - Object.entries(namespaces).forEach(([namespace, resources]) => { - i18next.addResourceBundle(lang, namespace, resources, true, true) - }) - } catch (error) { - console.warn(`Could not load ${lang} translations:`, error) - } + const resources = collectLocaleResources(eagerLocaleFiles) + + Object.entries(resources).forEach(([language, namespaces]) => { + Object.entries(namespaces).forEach(([namespace, bundle]) => { + try { + i18next.addResourceBundle(language, namespace, bundle, true, true) + } catch (error) { + console.warn(`Could not load ${language} translations:`, error) + } + }) }) + + loadedLanguages.add(FALLBACK_LANGUAGE) } export default i18next From 3892eca726689e0eba08818149f2e852ff2ec650 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 07:08:56 +0900 Subject: [PATCH 2/6] fix(webview): drop the orphaned loadTranslations export and cover the load failure What: delete `loadTranslations` from `webview-ui/src/i18n/setup.ts` and the `AppProviders` call that was its last consumer. `i18next.init` already receives the eager `en` bundles through `resources:`, so the function only re-registered what init had just registered. Add `setup.rejection.spec.ts`, which stubs one lazy namespace module to throw and asserts `loadLanguage` resolves, warns, leaves the language unregistered, and stays retryable. Tighten the concurrent-fetch test to assert the memoized promise identity and a single registration pass. Why: knip reported "Unused exports (1) loadTranslations" and failed the required code-qa knip leg (CI job 124281842516). The diff removed the export's last src-side consumer and left only `playwright/AppProviders.tsx`, which knip cannot see because `knip.json` disables the playwright plugins. The `.catch` at setup.ts:90 was also the only unexercised branch of the new load path. Impact: `pnpm knip` and `pnpm knip:production` exit 0 (were exit 1), the knip policy guardrail stays 8/8, and the i18n suite covers the rejection path. No production behavior change: the gallery still gets `en` from `init`. --- webview-ui/playwright/AppProviders.tsx | 7 +--- .../__tests__/IndexingStatusBadge.spec.tsx | 1 - .../__tests__/TranslationContext.spec.tsx | 1 - .../i18n/__tests__/setup.rejection.spec.ts | 39 +++++++++++++++++++ webview-ui/src/i18n/__tests__/setup.spec.ts | 12 +++++- webview-ui/src/i18n/setup.ts | 21 ---------- 6 files changed, 51 insertions(+), 30 deletions(-) create mode 100644 webview-ui/src/i18n/__tests__/setup.rejection.spec.ts diff --git a/webview-ui/playwright/AppProviders.tsx b/webview-ui/playwright/AppProviders.tsx index 3742a07770..44ec76b3db 100644 --- a/webview-ui/playwright/AppProviders.tsx +++ b/webview-ui/playwright/AppProviders.tsx @@ -5,15 +5,10 @@ import { providerIdentifiers } from "@roo-code/types" import { ExtensionStateContextProvider } from "@/context/ExtensionStateContext" import TranslationProvider from "@/i18n/TranslationContext" -import i18next, { loadTranslations } from "@/i18n/setup" +import i18next from "@/i18n/setup" import { TooltipProvider } from "@/components/ui/tooltip" import { TranslationContext as PlaywrightTranslationContext } from "@src/i18n/TranslationContext" -// The fallback locale registers itself when `setup` loads; this keeps the call -// explicit so a gallery story that swaps `language` still starts from a -// populated store. -loadTranslations() - type InitialState = NonNullable["initialState"]> interface AppProvidersProps { diff --git a/webview-ui/src/components/chat/__tests__/IndexingStatusBadge.spec.tsx b/webview-ui/src/components/chat/__tests__/IndexingStatusBadge.spec.tsx index 25f92757f8..04ba9cc60e 100644 --- a/webview-ui/src/components/chat/__tests__/IndexingStatusBadge.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/IndexingStatusBadge.spec.tsx @@ -14,7 +14,6 @@ vi.mock("@/i18n/setup", () => ({ language: "en", changeLanguage: vi.fn(), }, - loadTranslations: vi.fn(), })) vi.mock("react-i18next", () => ({ diff --git a/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx b/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx index 38fa9c0b80..953234ff6d 100644 --- a/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx +++ b/webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx @@ -38,7 +38,6 @@ vi.mock("../setup", () => ({ }, FALLBACK_LANGUAGE: "en", loadLanguage: vi.fn(() => Promise.resolve()), - loadTranslations: vi.fn(), })) const TestComponent = () => { diff --git a/webview-ui/src/i18n/__tests__/setup.rejection.spec.ts b/webview-ui/src/i18n/__tests__/setup.rejection.spec.ts new file mode 100644 index 0000000000..fe61bc2498 --- /dev/null +++ b/webview-ui/src/i18n/__tests__/setup.rejection.spec.ts @@ -0,0 +1,39 @@ +import i18next, { FALLBACK_LANGUAGE, loadLanguage } from "../setup" + +// A dynamic import that rejects is what a 404 or a chunk missing from the +// deployed manifest looks like at runtime. One namespace is enough: `loadLanguage` +// registers a language only after every namespace resolves. +vi.mock("../locales/ja/common.json", () => { + throw new Error("Failed to fetch dynamically imported module") +}) + +describe("i18n setup locale load failure", () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it("resolves without throwing and keeps the English fallback", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined) + + await expect(loadLanguage("ja")).resolves.toBeUndefined() + + expect(warn).toHaveBeenCalledWith("Could not load ja translations:", expect.any(Error)) + // All-or-nothing per language: the namespaces that did resolve are not + // registered either, so no half-translated locale can be activated. + expect(i18next.hasResourceBundle("ja", "common")).toBe(false) + expect(i18next.hasResourceBundle("ja", "settings")).toBe(false) + expect(i18next.t("common:errorBoundary.title", { lng: "ja" })).toBe("Something went wrong") + }) + + it("retries the fetch on the next call instead of caching the failure", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined) + + await loadLanguage("ja") + await loadLanguage("ja") + + // `loadedLanguages` is only updated on success and `pendingLanguages` is + // cleared in `finally`, so a transient failure must not be memoized. + expect(warn).toHaveBeenCalledTimes(2) + expect(i18next.language).toBe(FALLBACK_LANGUAGE) + }) +}) diff --git a/webview-ui/src/i18n/__tests__/setup.spec.ts b/webview-ui/src/i18n/__tests__/setup.spec.ts index 6ae102dc60..867fc03f4e 100644 --- a/webview-ui/src/i18n/__tests__/setup.spec.ts +++ b/webview-ui/src/i18n/__tests__/setup.spec.ts @@ -23,9 +23,19 @@ describe("i18n setup locale loading", () => { }) it("shares one fetch across concurrent requests for the same locale", async () => { - const results = await Promise.all([loadLanguage("ja"), loadLanguage("ja"), loadLanguage("ja")]) + const addResourceBundle = vi.spyOn(i18next, "addResourceBundle") + + // `loadLanguage` memoizes the in-flight promise, so concurrent callers get + // the very same promise back: one fetch, one registration pass. + const inFlight = [loadLanguage("ja"), loadLanguage("ja"), loadLanguage("ja")] + expect(inFlight[1]).toBe(inFlight[0]) + expect(inFlight[2]).toBe(inFlight[0]) + + const results = await Promise.all(inFlight) expect(results).toHaveLength(3) + // 9 namespaces registered once each, not 27. + expect(addResourceBundle).toHaveBeenCalledTimes(9) expect(i18next.t("common:errorBoundary.title", { lng: "ja" })).toBe("問題が発生しました") }) diff --git a/webview-ui/src/i18n/setup.ts b/webview-ui/src/i18n/setup.ts index 7b83d605e2..475e15a282 100644 --- a/webview-ui/src/i18n/setup.ts +++ b/webview-ui/src/i18n/setup.ts @@ -113,25 +113,4 @@ i18next.use(initReactI18next).init({ }, }) -/** - * Re-register the eagerly bundled fallback locale. `init` above already has it, - * so this is only needed by callers that reset the store (the Playwright - * gallery mounts stories against a shared i18next instance). - */ -export function loadTranslations() { - const resources = collectLocaleResources(eagerLocaleFiles) - - Object.entries(resources).forEach(([language, namespaces]) => { - Object.entries(namespaces).forEach(([namespace, bundle]) => { - try { - i18next.addResourceBundle(language, namespace, bundle, true, true) - } catch (error) { - console.warn(`Could not load ${language} translations:`, error) - } - }) - }) - - loadedLanguages.add(FALLBACK_LANGUAGE) -} - export default i18next From 224db259214b32ce72cc5069e6555edae70aa9c1 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 07:44:36 +0900 Subject: [PATCH 3/6] test(webview): cover the cancelled arm of the locale-switch guard What: add one test to `TranslationContext.locale-loading.spec.tsx` that renders the provider with language "vi", unmounts while the locale chunk is still in flight, then awaits the memoized `loadLanguage("vi")` promise. Because the provider attached its `.then` first, its callback has already run when that await returns, so the assertions prove the guard suppressed the switch rather than the promise still being pending. Why: `codecov/patch` failed at 71.42% (target 80%) on the previous head. The two partial branches were both introduced by this PR: the `cancelled` arm at TranslationContext.tsx:33 and the `||` fallback arm at line 21. The unmount path is a real user action (closing the view or switching tasks mid-fetch), so the guard is behavior worth pinning rather than coverage worth suppressing. Impact: branch coverage for `TranslationContext.tsx` 50% -> 75%, and every instrumented added line in the file is now hit (7/7). No production change. --- ...TranslationContext.locale-loading.spec.tsx | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx index 93be04b688..000dea7a45 100644 --- a/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx +++ b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx @@ -1,7 +1,7 @@ import { act, render, screen, waitFor } from "@testing-library/react" import React from "react" -import i18next from "../setup" +import i18next, { loadLanguage } from "../setup" import TranslationProvider, { useAppTranslation } from "../TranslationContext" // The provider reads the active language from extension state. Tests drive it @@ -99,4 +99,23 @@ describe("TranslationProvider locale loading", () => { expect(screen.getByTestId("error-title")).toHaveTextContent("Something went wrong") }) }) + + it("skips the language switch when the provider unmounts before the fetch resolves", async () => { + const changeLanguage = vi.spyOn(i18next, "changeLanguage") + + const { unmount } = renderProbe("vi") + // The locale chunk is still in flight here, so the effect cleanup runs + // before the fetch settles and `cancelled` decides the outcome. + unmount() + + // `loadLanguage` memoizes the in-flight promise, so awaiting the same + // locale here waits for the exact resolution the guard is deciding on. + // The provider attached its `.then` first, so its callback has already run + // by the time this returns: the switch was suppressed, not left pending. + await loadLanguage("vi") + + expect(i18next.hasResourceBundle("vi", "common")).toBe(true) + expect(changeLanguage).not.toHaveBeenCalledWith("vi") + expect(i18next.language).not.toBe("vi") + }) }) From 4fcf7eee50cb757fd3fc23a4375f8476fcc53a71 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 09:11:02 +0900 Subject: [PATCH 4/6] fix(webview): type locale resources and correct the switch comment What: - Replace the `Record` locale aliases in i18n/setup.ts with a recursive `string | LocaleResources` map, so a locale JSON leaf that is not a string is a type error instead of silently reaching addResourceBundle. - Correct the TranslationProvider comment: during an uncached fetch the subtree resolves through whichever language is still active, which is `fallbackLng` on a cold start but the previous language on a warm switch between two non-English locales. - Rename the rejection test to what it proves: `loadLanguage` does not memoize a failed load. The comment now records that the engine module map, not this function, decides whether the chunk is re-requested. Why: CodeRabbit raised three threads on PR #2267. The `any` aliases were the only untyped hop on the path from locale JSON to i18next, and the comment described cold-start behavior as if it were the only case. Impact: Type-level only for the aliases; no runtime change. The comment and test name now match observed behavior. `pnpm knip`, `knip:production`, `test:knip-policy`, `tsc --noEmit`, and the four i18n vitest suites stay green. --- webview-ui/src/i18n/TranslationContext.tsx | 6 ++++-- webview-ui/src/i18n/__tests__/setup.rejection.spec.ts | 6 +++++- webview-ui/src/i18n/setup.ts | 7 ++++++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/webview-ui/src/i18n/TranslationContext.tsx b/webview-ui/src/i18n/TranslationContext.tsx index 1450a73241..978c77018a 100644 --- a/webview-ui/src/i18n/TranslationContext.tsx +++ b/webview-ui/src/i18n/TranslationContext.tsx @@ -24,8 +24,10 @@ const TranslationProvider: React.FC<{ children: ReactNode }> = ({ children }) => // has to fetch its bundles before i18next can resolve a key. Switching after // the fetch is deliberate: `changeLanguage` is what emits `languageChanged`, // the event `useTranslation` subscribes to for re-renders. Until it fires the - // subtree keeps resolving keys through `fallbackLng`, the same - // fallback-then-swap shape this provider had when every locale was bundled. + // subtree keeps resolving keys through whichever language is still active: + // `fallbackLng` on a cold start, the previous language on a warm switch + // between two non-English locales. The swap-then-render shape is the same one + // this provider had when every locale was bundled. useEffect(() => { let cancelled = false diff --git a/webview-ui/src/i18n/__tests__/setup.rejection.spec.ts b/webview-ui/src/i18n/__tests__/setup.rejection.spec.ts index fe61bc2498..d6812fc788 100644 --- a/webview-ui/src/i18n/__tests__/setup.rejection.spec.ts +++ b/webview-ui/src/i18n/__tests__/setup.rejection.spec.ts @@ -25,7 +25,7 @@ describe("i18n setup locale load failure", () => { expect(i18next.t("common:errorBoundary.title", { lng: "ja" })).toBe("Something went wrong") }) - it("retries the fetch on the next call instead of caching the failure", async () => { + it("does not memoize a failed load, so the next call re-attempts it", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined) await loadLanguage("ja") @@ -33,6 +33,10 @@ describe("i18n setup locale load failure", () => { // `loadedLanguages` is only updated on success and `pendingLanguages` is // cleared in `finally`, so a transient failure must not be memoized. + // This covers `loadLanguage`'s own bookkeeping only: engines that cache a + // failed module fetch in the module map (every Chromium below 156, so the + // VS Code webview) can still reject the second import from that cache + // without re-requesting the chunk. expect(warn).toHaveBeenCalledTimes(2) expect(i18next.language).toBe(FALLBACK_LANGUAGE) }) diff --git a/webview-ui/src/i18n/setup.ts b/webview-ui/src/i18n/setup.ts index 475e15a282..6010111e2a 100644 --- a/webview-ui/src/i18n/setup.ts +++ b/webview-ui/src/i18n/setup.ts @@ -8,7 +8,12 @@ import { initReactI18next } from "react-i18next" */ export const FALLBACK_LANGUAGE = "en" -type LocaleResources = Record +/** + * A locale JSON file is a tree of translated strings, e.g. + * `{ "errorBoundary": { "title": "Something went wrong" } }`. Typing the leaves + * keeps the values checked on their way into `i18next.addResourceBundle`. + */ +type LocaleResources = { [key: string]: string | LocaleResources } type LocaleModule = { default?: LocaleResources } & LocaleResources // Example path: './locales/en/common.json' -> language: 'en', namespace: 'common' From fee8599ac898c504c07e20b7f7ec97ca2821501a Mon Sep 17 00:00:00 2001 From: tiebreak-probe Date: Sat, 31 Oct 2026 14:33:21 +0900 Subject: [PATCH 5/6] test(webview): assert the English first paint for a non-English initial locale What: add one synchronous assertion in the non-English initial-language test that the first paint resolves the English fallback string before the loader finishes and the switch lands on Korean. Why: the test jumped straight to a waitFor for the final Korean title, so a provider that gated rendering on the loader, or one that eagerly bundled the requested locale, still passed. A mutation probe that gates non-fallback languages on loadLanguage survives all five existing tests and is caught only by this assertion. Impact: test-only. No production source changes; the suite stays at 5 passing and the i18n folder at 14 passing. --- .../i18n/__tests__/TranslationContext.locale-loading.spec.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx index 000dea7a45..1f53b25afa 100644 --- a/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx +++ b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx @@ -45,6 +45,8 @@ describe("TranslationProvider locale loading", () => { // Korean bundles are not in the entry chunk, so the first paint resolves // through the English fallback and the switch lands after the fetch. + expect(screen.getByTestId("error-title")).toHaveTextContent("Something went wrong") + await waitFor(() => { expect(screen.getByTestId("error-title")).toHaveTextContent("문제가 발생했습니다") }) From 09d0180f12ed5d5af9d918993d6ddab55e702625 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 17:05:08 +0900 Subject: [PATCH 6/6] test(webview): pin the warm locale switch to the previous language What: add a de->fr case to TranslationContext.locale-loading.spec.tsx that asserts the rendered error title is still German between the rerender and the French load settling, then asserts the final French state. Why: every warm-path case in the file either starts at English or ends at English, so a fallback-reset flash (changeLanguage(FALLBACK_LANGUAGE) fired mid-switch) passed all five existing tests. French is loaded by no other case, so the intermediate frame is observable and the assertion does not depend on test order. Impact: test-only, 27 added lines, no production change. The new assertion fails 3/3 against the injected fallback-reset mutant while the other five stay green, and passes 5/5 serial runs on the fixed provider. --- ...TranslationContext.locale-loading.spec.tsx | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx index 1f53b25afa..0717361f50 100644 --- a/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx +++ b/webview-ui/src/i18n/__tests__/TranslationContext.locale-loading.spec.tsx @@ -102,6 +102,33 @@ describe("TranslationProvider locale loading", () => { }) }) + it("keeps the previous locale rendered while a warm switch is still loading", async () => { + const { rerender } = renderProbe("de") + + await waitFor(() => { + expect(screen.getByTestId("error-title")).toHaveTextContent("Etwas ist schiefgelaufen") + }) + + extensionState.language = "fr" + await act(async () => { + rerender( + + + , + ) + }) + + // French is not in the entry chunk and no other case loads it, so the + // switch cannot have landed yet. A `changeLanguage` fired before the fetch + // resolves would paint the English fallback here instead of German. + expect(screen.getByTestId("error-title")).toHaveTextContent("Etwas ist schiefgelaufen") + + await waitFor(() => { + expect(screen.getByTestId("error-title")).toHaveTextContent("Une erreur s'est produite") + }) + expect(screen.getByTestId("active-language")).toHaveTextContent("fr") + }) + it("skips the language switch when the provider unmounts before the fetch resolves", async () => { const changeLanguage = vi.spyOn(i18next, "changeLanguage")