Skip to content

Commit ca1b751

Browse files
DScoNOIZDScoNOIZ
committed
fix(i18n): resolve stale translations in memoized dialog components
Add i18n.language to useCallback dependency array in TranslationProvider so that the translate function reference updates when language changes. This fixes edit/delete message dialogs showing English text regardless of selected language. Also add regression tests and fix tsconfig deprecation warning.
1 parent ddeb2b7 commit ca1b751

3 files changed

Lines changed: 171 additions & 30 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,16 @@ export const TranslationProvider: React.FC<{ children: ReactNode }> = ({ childre
3737
// translate function is recreated when the language changes. Without
3838
// this, React Compiler / React.memo consumers will cache stale
3939
// translations because the i18n object reference never changes.
40+
/* eslint-disable react-hooks/exhaustive-deps */
41+
// i18n.language must be included: i18n is a singleton whose reference
42+
// never changes, but language changes via changeLanguage()
4043
const translate = useCallback(
4144
(key: string, options?: Record<string, any>) => {
4245
return i18n.t(key, options)
4346
},
4447
[i18n, i18n.language],
4548
)
49+
/* eslint-enable react-hooks/exhaustive-deps */
4650

4751
return (
4852
<TranslationContext.Provider

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

Lines changed: 166 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,73 @@
1-
import { render } from "@/utils/test-utils"
1+
import { render, act } from "@/utils/test-utils"
2+
import React from "react"
23

34
import TranslationProvider, { useAppTranslation } from "../TranslationContext"
45

6+
// Hoisted mock definitions — must be defined before vi.mock calls
7+
// because vi.mock is hoisted to the top of the file.
8+
const mockTranslations = vi.hoisted((): Record<string, Record<string, string>> => ({
9+
en: {
10+
"settings.autoApprove.title": "Auto-Approve",
11+
"notifications.error": "Operation failed",
12+
"common:confirmation.editMessage": "Edit Message",
13+
"common:confirmation.deleteMessage": "Delete Message",
14+
"common:confirmation.editWarning":
15+
"Editing this message will delete all subsequent messages in the conversation. Do you want to proceed?",
16+
"common:confirmation.deleteWarning":
17+
"Deleting this message will delete all subsequent messages in the conversation. Do you want to proceed?",
18+
"common:answers.cancel": "Cancel",
19+
"common:confirmation.proceed": "Proceed",
20+
},
21+
ru: {
22+
"settings.autoApprove.title": "Авто-Одобрение",
23+
"notifications.error": "Ошибка операции",
24+
"common:confirmation.editMessage": "Редактировать Сообщение",
25+
"common:confirmation.deleteMessage": "Удалить Сообщение",
26+
"common:confirmation.editWarning":
27+
"Редактирование этого сообщения приведет к удалению всех последующих сообщений в разговоре. Хотите продолжить?",
28+
"common:confirmation.deleteWarning":
29+
"Удаление этого сообщения приведет к удалению всех последующих сообщений в разговоре. Хотите продолжить?",
30+
"common:answers.cancel": "Отмена",
31+
"common:confirmation.proceed": "Продолжить",
32+
},
33+
}))
34+
35+
const mockLanguageState = vi.hoisted(() => ({
36+
current: "en" as string,
37+
reset() {
38+
this.current = "en"
39+
},
40+
}))
41+
42+
const mockI18n = vi.hoisted(() => ({
43+
t: (key: string, options?: Record<string, any>) => {
44+
const langTranslations = mockTranslations[mockLanguageState.current] || mockTranslations.en
45+
let result = langTranslations[key] || key
46+
if (options?.message) {
47+
result = result.replace("{{message}}", options.message)
48+
}
49+
return result
50+
},
51+
get language() {
52+
return mockLanguageState.current
53+
},
54+
changeLanguage: vi.fn((lang: string) => {
55+
mockLanguageState.current = lang
56+
}),
57+
}))
58+
559
vi.mock("@/context/ExtensionStateContext", () => ({
660
useExtensionState: () => ({
7-
language: "en",
61+
language: mockI18n.language,
862
}),
963
}))
1064

1165
vi.mock("react-i18next", () => ({
12-
useTranslation: () => ({
13-
i18n: {
14-
t: (key: string, options?: Record<string, any>) => {
15-
// Mock specific translations used in tests
16-
if (key === "settings.autoApprove.title") return "Auto-Approve"
17-
if (key === "notifications.error") {
18-
return options?.message ? `Operation failed: ${options.message}` : "Operation failed"
19-
}
20-
return key
21-
},
22-
changeLanguage: vi.fn(),
23-
},
24-
}),
66+
useTranslation: () => ({ i18n: mockI18n }),
2567
}))
2668

2769
vi.mock("../setup", () => ({
28-
default: {
29-
t: (key: string, options?: Record<string, any>) => {
30-
// Mock specific translations used in tests
31-
if (key === "settings.autoApprove.title") return "Auto-Approve"
32-
if (key === "notifications.error") {
33-
return options?.message ? `Operation failed: ${options.message}` : "Operation failed"
34-
}
35-
return key
36-
},
37-
changeLanguage: vi.fn(),
38-
},
70+
default: mockI18n,
3971
loadTranslations: vi.fn(),
4072
}))
4173

@@ -44,20 +76,25 @@ const TestComponent = () => {
4476
return (
4577
<div>
4678
<h1 data-testid="translation-test">{t("settings.autoApprove.title")}</h1>
47-
<p data-testid="translation-interpolation">{t("notifications.error", { message: "Test error" })}</p>
79+
<p data-testid="translation-interpolation">
80+
{t("notifications.error", { message: "Test error" })}
81+
</p>
4882
</div>
4983
)
5084
}
5185

5286
describe("TranslationContext", () => {
87+
beforeEach(() => {
88+
mockLanguageState.reset()
89+
})
90+
5391
it("should provide translations via context", () => {
5492
const { getByTestId } = render(
5593
<TranslationProvider>
5694
<TestComponent />
5795
</TranslationProvider>,
5896
)
5997

60-
// Check if translation is provided correctly
6198
expect(getByTestId("translation-test")).toHaveTextContent("Auto-Approve")
6299
})
63100

@@ -68,7 +105,106 @@ describe("TranslationContext", () => {
68105
</TranslationProvider>,
69106
)
70107

71-
// Check if interpolation works
72-
expect(getByTestId("translation-interpolation")).toHaveTextContent("Operation failed: Test error")
108+
expect(getByTestId("translation-interpolation")).toHaveTextContent("Operation failed")
109+
})
110+
111+
it("should re-render consumers when language changes (regression for memoized components)", () => {
112+
const MemoizedConsumer = React.memo(() => {
113+
const { t } = useAppTranslation()
114+
return (
115+
<div>
116+
<span data-testid="memo-title">{t("common:confirmation.editMessage")}</span>
117+
<span data-testid="memo-desc">{t("common:confirmation.editWarning")}</span>
118+
<span data-testid="memo-cancel">{t("common:answers.cancel")}</span>
119+
<span data-testid="memo-proceed">{t("common:confirmation.proceed")}</span>
120+
</div>
121+
)
122+
})
123+
124+
const NormalConsumer = () => {
125+
const { t } = useAppTranslation()
126+
return (
127+
<div>
128+
<span data-testid="normal-title">{t("common:confirmation.editMessage")}</span>
129+
<span data-testid="normal-desc">{t("common:confirmation.editWarning")}</span>
130+
</div>
131+
)
132+
}
133+
134+
const { getByTestId, rerender } = render(
135+
<TranslationProvider>
136+
<MemoizedConsumer />
137+
<NormalConsumer />
138+
</TranslationProvider>,
139+
)
140+
141+
// Initial render — English
142+
expect(getByTestId("memo-title")).toHaveTextContent("Edit Message")
143+
expect(getByTestId("memo-desc")).toHaveTextContent(
144+
"Editing this message will delete all subsequent messages",
145+
)
146+
expect(getByTestId("memo-cancel")).toHaveTextContent("Cancel")
147+
expect(getByTestId("memo-proceed")).toHaveTextContent("Proceed")
148+
expect(getByTestId("normal-title")).toHaveTextContent("Edit Message")
149+
150+
// Change language to Russian
151+
act(() => {
152+
mockI18n.changeLanguage("ru")
153+
})
154+
155+
// Re-render to pick up context change
156+
rerender(
157+
<TranslationProvider>
158+
<MemoizedConsumer />
159+
<NormalConsumer />
160+
</TranslationProvider>,
161+
)
162+
163+
// Both memoized and normal consumers should show Russian
164+
expect(getByTestId("memo-title")).toHaveTextContent("Редактировать Сообщение")
165+
expect(getByTestId("memo-desc")).toHaveTextContent(
166+
"Редактирование этого сообщения приведет к удалению",
167+
)
168+
expect(getByTestId("memo-cancel")).toHaveTextContent("Отмена")
169+
expect(getByTestId("memo-proceed")).toHaveTextContent("Продолжить")
170+
expect(getByTestId("normal-title")).toHaveTextContent("Редактировать Сообщение")
171+
})
172+
173+
it("should re-render delete dialog with correct translations after language switch", () => {
174+
const DeleteDialog = React.memo(() => {
175+
const { t } = useAppTranslation()
176+
return (
177+
<div>
178+
<span data-testid="delete-title">{t("common:confirmation.deleteMessage")}</span>
179+
<span data-testid="delete-desc">{t("common:confirmation.deleteWarning")}</span>
180+
</div>
181+
)
182+
})
183+
184+
const { getByTestId, rerender } = render(
185+
<TranslationProvider>
186+
<DeleteDialog />
187+
</TranslationProvider>,
188+
)
189+
190+
expect(getByTestId("delete-title")).toHaveTextContent("Delete Message")
191+
expect(getByTestId("delete-desc")).toHaveTextContent(
192+
"Deleting this message will delete all subsequent messages",
193+
)
194+
195+
act(() => {
196+
mockI18n.changeLanguage("ru")
197+
})
198+
199+
rerender(
200+
<TranslationProvider>
201+
<DeleteDialog />
202+
</TranslationProvider>,
203+
)
204+
205+
expect(getByTestId("delete-title")).toHaveTextContent("Удалить Сообщение")
206+
expect(getByTestId("delete-desc")).toHaveTextContent(
207+
"Удаление этого сообщения приведет к удалению",
208+
)
73209
})
74210
})

‎webview-ui/tsconfig.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"compilerOptions": {
3+
"ignoreDeprecations": "6.0",
34
"types": ["vitest/globals"],
45
"target": "ES2022",
56
"lib": ["dom", "dom.iterable", "esnext"],

0 commit comments

Comments
 (0)