Skip to content

Commit 3892eca

Browse files
committed
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`.
1 parent cb5ef06 commit 3892eca

6 files changed

Lines changed: 51 additions & 30 deletions

File tree

‎webview-ui/playwright/AppProviders.tsx‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,10 @@ import { providerIdentifiers } from "@roo-code/types"
55

66
import { ExtensionStateContextProvider } from "@/context/ExtensionStateContext"
77
import TranslationProvider from "@/i18n/TranslationContext"
8-
import i18next, { loadTranslations } from "@/i18n/setup"
8+
import i18next from "@/i18n/setup"
99
import { TooltipProvider } from "@/components/ui/tooltip"
1010
import { TranslationContext as PlaywrightTranslationContext } from "@src/i18n/TranslationContext"
1111

12-
// The fallback locale registers itself when `setup` loads; this keeps the call
13-
// explicit so a gallery story that swaps `language` still starts from a
14-
// populated store.
15-
loadTranslations()
16-
1712
type InitialState = NonNullable<React.ComponentProps<typeof ExtensionStateContextProvider>["initialState"]>
1813

1914
interface AppProvidersProps {

‎webview-ui/src/components/chat/__tests__/IndexingStatusBadge.spec.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ vi.mock("@/i18n/setup", () => ({
1414
language: "en",
1515
changeLanguage: vi.fn(),
1616
},
17-
loadTranslations: vi.fn(),
1817
}))
1918

2019
vi.mock("react-i18next", () => ({

‎webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ vi.mock("../setup", () => ({
3838
},
3939
FALLBACK_LANGUAGE: "en",
4040
loadLanguage: vi.fn(() => Promise.resolve()),
41-
loadTranslations: vi.fn(),
4241
}))
4342

4443
const TestComponent = () => {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import i18next, { FALLBACK_LANGUAGE, loadLanguage } from "../setup"
2+
3+
// A dynamic import that rejects is what a 404 or a chunk missing from the
4+
// deployed manifest looks like at runtime. One namespace is enough: `loadLanguage`
5+
// registers a language only after every namespace resolves.
6+
vi.mock("../locales/ja/common.json", () => {
7+
throw new Error("Failed to fetch dynamically imported module")
8+
})
9+
10+
describe("i18n setup locale load failure", () => {
11+
afterEach(() => {
12+
vi.restoreAllMocks()
13+
})
14+
15+
it("resolves without throwing and keeps the English fallback", async () => {
16+
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined)
17+
18+
await expect(loadLanguage("ja")).resolves.toBeUndefined()
19+
20+
expect(warn).toHaveBeenCalledWith("Could not load ja translations:", expect.any(Error))
21+
// All-or-nothing per language: the namespaces that did resolve are not
22+
// registered either, so no half-translated locale can be activated.
23+
expect(i18next.hasResourceBundle("ja", "common")).toBe(false)
24+
expect(i18next.hasResourceBundle("ja", "settings")).toBe(false)
25+
expect(i18next.t("common:errorBoundary.title", { lng: "ja" })).toBe("Something went wrong")
26+
})
27+
28+
it("retries the fetch on the next call instead of caching the failure", async () => {
29+
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined)
30+
31+
await loadLanguage("ja")
32+
await loadLanguage("ja")
33+
34+
// `loadedLanguages` is only updated on success and `pendingLanguages` is
35+
// cleared in `finally`, so a transient failure must not be memoized.
36+
expect(warn).toHaveBeenCalledTimes(2)
37+
expect(i18next.language).toBe(FALLBACK_LANGUAGE)
38+
})
39+
})

‎webview-ui/src/i18n/__tests__/setup.spec.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,19 @@ describe("i18n setup locale loading", () => {
2323
})
2424

2525
it("shares one fetch across concurrent requests for the same locale", async () => {
26-
const results = await Promise.all([loadLanguage("ja"), loadLanguage("ja"), loadLanguage("ja")])
26+
const addResourceBundle = vi.spyOn(i18next, "addResourceBundle")
27+
28+
// `loadLanguage` memoizes the in-flight promise, so concurrent callers get
29+
// the very same promise back: one fetch, one registration pass.
30+
const inFlight = [loadLanguage("ja"), loadLanguage("ja"), loadLanguage("ja")]
31+
expect(inFlight[1]).toBe(inFlight[0])
32+
expect(inFlight[2]).toBe(inFlight[0])
33+
34+
const results = await Promise.all(inFlight)
2735

2836
expect(results).toHaveLength(3)
37+
// 9 namespaces registered once each, not 27.
38+
expect(addResourceBundle).toHaveBeenCalledTimes(9)
2939
expect(i18next.t("common:errorBoundary.title", { lng: "ja" })).toBe("問題が発生しました")
3040
})
3141

‎webview-ui/src/i18n/setup.ts‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -113,25 +113,4 @@ i18next.use(initReactI18next).init({
113113
},
114114
})
115115

116-
/**
117-
* Re-register the eagerly bundled fallback locale. `init` above already has it,
118-
* so this is only needed by callers that reset the store (the Playwright
119-
* gallery mounts stories against a shared i18next instance).
120-
*/
121-
export function loadTranslations() {
122-
const resources = collectLocaleResources(eagerLocaleFiles)
123-
124-
Object.entries(resources).forEach(([language, namespaces]) => {
125-
Object.entries(namespaces).forEach(([namespace, bundle]) => {
126-
try {
127-
i18next.addResourceBundle(language, namespace, bundle, true, true)
128-
} catch (error) {
129-
console.warn(`Could not load ${language} translations:`, error)
130-
}
131-
})
132-
})
133-
134-
loadedLanguages.add(FALLBACK_LANGUAGE)
135-
}
136-
137116
export default i18next

0 commit comments

Comments
 (0)