Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion webview-ui/src/i18n/TranslationContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,20 @@ export const TranslationProvider: React.FC<{ children: ReactNode }> = ({ childre
}, [i18n, extensionState.language])

// Memoize the translation function to prevent unnecessary re-renders
// Note: i18n.language must be included as a dependency so that the
// translate function is recreated when the language changes. Without
// this, React Compiler / React.memo consumers will cache stale
// translations because the i18n object reference never changes.
/* eslint-disable react-hooks/exhaustive-deps */
// i18n.language must be included: i18n is a singleton whose reference
// never changes, but language changes via changeLanguage()
const translate = useCallback(
(key: string, options?: Record<string, any>) => {
return i18n.t(key, options)
},
[i18n],
[i18n, i18n.language],
)
/* eslint-enable react-hooks/exhaustive-deps */

return (
<TranslationContext.Provider
Expand Down
196 changes: 166 additions & 30 deletions webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,41 +1,73 @@
import { render } from "@/utils/test-utils"
import { render, act } from "@/utils/test-utils"
import React from "react"

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

// Hoisted mock definitions — must be defined before vi.mock calls
// because vi.mock is hoisted to the top of the file.
const mockTranslations = vi.hoisted((): Record<string, Record<string, string>> => ({
en: {
"settings.autoApprove.title": "Auto-Approve",
"notifications.error": "Operation failed",
"common:confirmation.editMessage": "Edit Message",
"common:confirmation.deleteMessage": "Delete Message",
"common:confirmation.editWarning":
"Editing this message will delete all subsequent messages in the conversation. Do you want to proceed?",
"common:confirmation.deleteWarning":
"Deleting this message will delete all subsequent messages in the conversation. Do you want to proceed?",
"common:answers.cancel": "Cancel",
"common:confirmation.proceed": "Proceed",
},
ru: {
"settings.autoApprove.title": "Авто-Одобрение",
"notifications.error": "Ошибка операции",
"common:confirmation.editMessage": "Редактировать Сообщение",
"common:confirmation.deleteMessage": "Удалить Сообщение",
"common:confirmation.editWarning":
"Редактирование этого сообщения приведет к удалению всех последующих сообщений в разговоре. Хотите продолжить?",
"common:confirmation.deleteWarning":
"Удаление этого сообщения приведет к удалению всех последующих сообщений в разговоре. Хотите продолжить?",
"common:answers.cancel": "Отмена",
"common:confirmation.proceed": "Продолжить",
},
}))
Comment on lines +8 to +33
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Interpolation test is not actually exercising interpolation.

"notifications.error" has no {{message}} placeholder, so this test validates static translation only, not interpolation regression.

Suggested fix
-		"notifications.error": "Operation failed",
+		"notifications.error": "Operation failed: {{message}}",
...
-		"notifications.error": "Ошибка операции",
+		"notifications.error": "Ошибка операции: {{message}}",
...
-		expect(getByTestId("translation-interpolation")).toHaveTextContent("Operation failed")
+		expect(getByTestId("translation-interpolation")).toHaveTextContent("Operation failed: Test error")

Also applies to: 101-109

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@webview-ui/src/i18n/__tests__/TranslationContext.spec.tsx` around lines 8 -
33, The test's interpolation assertion isn't valid because the mockTranslations
entry "notifications.error" has no placeholder; update the mockTranslations
object used by TranslationContext.spec.tsx (the vi.hoisted mockTranslations) so
that "notifications.error" includes an interpolation token (e.g. "Operation
failed: {{message}}"), then update the test invocation to pass an interpolation
value and assert the interpolated string is rendered by the
TranslationContext/translate function (also check the duplicated entries around
lines 101-109 if they use the same key). Ensure you modify the mockTranslations
constant and the test call that checks "notifications.error" so the test
actually exercises interpolation.


const mockLanguageState = vi.hoisted(() => ({
current: "en" as string,
reset() {
this.current = "en"
},
}))

const mockI18n = vi.hoisted(() => ({
t: (key: string, options?: Record<string, any>) => {
const langTranslations = mockTranslations[mockLanguageState.current] || mockTranslations.en
let result = langTranslations[key] || key
if (options?.message) {
result = result.replace("{{message}}", options.message)
}
return result
},
get language() {
return mockLanguageState.current
},
changeLanguage: vi.fn((lang: string) => {
mockLanguageState.current = lang
}),
}))

vi.mock("@/context/ExtensionStateContext", () => ({
useExtensionState: () => ({
language: "en",
language: mockI18n.language,
}),
}))

vi.mock("react-i18next", () => ({
useTranslation: () => ({
i18n: {
t: (key: string, options?: Record<string, any>) => {
// Mock specific translations used in tests
if (key === "settings.autoApprove.title") return "Auto-Approve"
if (key === "notifications.error") {
return options?.message ? `Operation failed: ${options.message}` : "Operation failed"
}
return key
},
changeLanguage: vi.fn(),
},
}),
useTranslation: () => ({ i18n: mockI18n }),
}))

vi.mock("../setup", () => ({
default: {
t: (key: string, options?: Record<string, any>) => {
// Mock specific translations used in tests
if (key === "settings.autoApprove.title") return "Auto-Approve"
if (key === "notifications.error") {
return options?.message ? `Operation failed: ${options.message}` : "Operation failed"
}
return key
},
changeLanguage: vi.fn(),
},
default: mockI18n,
loadTranslations: vi.fn(),
}))

Expand All @@ -44,20 +76,25 @@ const TestComponent = () => {
return (
<div>
<h1 data-testid="translation-test">{t("settings.autoApprove.title")}</h1>
<p data-testid="translation-interpolation">{t("notifications.error", { message: "Test error" })}</p>
<p data-testid="translation-interpolation">
{t("notifications.error", { message: "Test error" })}
</p>
</div>
)
}

describe("TranslationContext", () => {
beforeEach(() => {
mockLanguageState.reset()
})

it("should provide translations via context", () => {
const { getByTestId } = render(
<TranslationProvider>
<TestComponent />
</TranslationProvider>,
)

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

Expand All @@ -68,7 +105,106 @@ describe("TranslationContext", () => {
</TranslationProvider>,
)

// Check if interpolation works
expect(getByTestId("translation-interpolation")).toHaveTextContent("Operation failed: Test error")
expect(getByTestId("translation-interpolation")).toHaveTextContent("Operation failed")
})

it("should re-render consumers when language changes (regression for memoized components)", () => {
const MemoizedConsumer = React.memo(() => {
const { t } = useAppTranslation()
return (
<div>
<span data-testid="memo-title">{t("common:confirmation.editMessage")}</span>
<span data-testid="memo-desc">{t("common:confirmation.editWarning")}</span>
<span data-testid="memo-cancel">{t("common:answers.cancel")}</span>
<span data-testid="memo-proceed">{t("common:confirmation.proceed")}</span>
</div>
)
})

const NormalConsumer = () => {
const { t } = useAppTranslation()
return (
<div>
<span data-testid="normal-title">{t("common:confirmation.editMessage")}</span>
<span data-testid="normal-desc">{t("common:confirmation.editWarning")}</span>
</div>
)
}

const { getByTestId, rerender } = render(
<TranslationProvider>
<MemoizedConsumer />
<NormalConsumer />
</TranslationProvider>,
)

// Initial render — English
expect(getByTestId("memo-title")).toHaveTextContent("Edit Message")
expect(getByTestId("memo-desc")).toHaveTextContent(
"Editing this message will delete all subsequent messages",
)
expect(getByTestId("memo-cancel")).toHaveTextContent("Cancel")
expect(getByTestId("memo-proceed")).toHaveTextContent("Proceed")
expect(getByTestId("normal-title")).toHaveTextContent("Edit Message")

// Change language to Russian
act(() => {
mockI18n.changeLanguage("ru")
})

// Re-render to pick up context change
rerender(
<TranslationProvider>
<MemoizedConsumer />
<NormalConsumer />
</TranslationProvider>,
)

// Both memoized and normal consumers should show Russian
expect(getByTestId("memo-title")).toHaveTextContent("Редактировать Сообщение")
expect(getByTestId("memo-desc")).toHaveTextContent(
"Редактирование этого сообщения приведет к удалению",
)
expect(getByTestId("memo-cancel")).toHaveTextContent("Отмена")
expect(getByTestId("memo-proceed")).toHaveTextContent("Продолжить")
expect(getByTestId("normal-title")).toHaveTextContent("Редактировать Сообщение")
})

it("should re-render delete dialog with correct translations after language switch", () => {
const DeleteDialog = React.memo(() => {
const { t } = useAppTranslation()
return (
<div>
<span data-testid="delete-title">{t("common:confirmation.deleteMessage")}</span>
<span data-testid="delete-desc">{t("common:confirmation.deleteWarning")}</span>
</div>
)
})

const { getByTestId, rerender } = render(
<TranslationProvider>
<DeleteDialog />
</TranslationProvider>,
)

expect(getByTestId("delete-title")).toHaveTextContent("Delete Message")
expect(getByTestId("delete-desc")).toHaveTextContent(
"Deleting this message will delete all subsequent messages",
)

act(() => {
mockI18n.changeLanguage("ru")
})

rerender(
<TranslationProvider>
<DeleteDialog />
</TranslationProvider>,
)

expect(getByTestId("delete-title")).toHaveTextContent("Удалить Сообщение")
expect(getByTestId("delete-desc")).toHaveTextContent(
"Удаление этого сообщения приведет к удалению",
)
})
})