feat(chrome-extension): add i18n foundation#2009
Conversation
| const localizedMessage = | ||
| typeof chrome === "undefined" | ||
| ? "" | ||
| : chrome.i18n.getMessage(key, substitutions); |
There was a problem hiding this comment.
window.chrome can exist outside an extension context (but without i18n), so typeof chrome === "undefined" alone can still blow up here. I’d guard i18n.getMessage as well, and maybe add a test for that case.
| : chrome.i18n.getMessage(key, substitutions); | |
| typeof chrome === "undefined" || typeof (chrome as any).i18n?.getMessage !== "function" | |
| ? "" | |
| : chrome.i18n.getMessage(key, substitutions); |
| ? "" | ||
| : chrome.i18n.getMessage(key, substitutions); | ||
|
|
||
| return localizedMessage || englishMessages[key].message; |
There was a problem hiding this comment.
When
chrome is available but getMessage returns "" (e.g., key missing in a locale), the function falls back to the English message string — but the caller-supplied substitutions are silently discarded. This means any future message that uses $1/$2 placeholders would render raw placeholder syntax in the fallback path instead of the substituted value. In the Chrome extension context this is unlikely (English is the default locale, so the API should always resolve), but a non-extension caller (test or Node script) using msg("someKey", someValue) would always get unsubstituted English text back.
| return localizedMessage || englishMessages[key].message; | |
| if (localizedMessage) return localizedMessage; | |
| const fallback = englishMessages[key].message; | |
| if (!substitutions) return fallback; | |
| const subs = Array.isArray(substitutions) ? substitutions : [substitutions]; | |
| return fallback.replace(/\$(\d+)/g, (_, i) => subs[Number(i) - 1] ?? ""); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/chrome-extension/src/shared/i18n.ts
Line: 11
Comment:
When `chrome` is available but `getMessage` returns `""` (e.g., key missing in a locale), the function falls back to the English message string — but the caller-supplied `substitutions` are silently discarded. This means any future message that uses `$1`/`$2` placeholders would render raw placeholder syntax in the fallback path instead of the substituted value. In the Chrome extension context this is unlikely (English is the default locale, so the API should always resolve), but a non-extension caller (test or Node script) using `msg("someKey", someValue)` would always get unsubstituted English text back.
```suggestion
if (localizedMessage) return localizedMessage;
const fallback = englishMessages[key].message;
if (!substitutions) return fallback;
const subs = Array.isArray(substitutions) ? substitutions : [substitutions];
return fallback.replace(/\$(\d+)/g, (_, i) => subs[Number(i) - 1] ?? "");
```
How can I resolve this? If you propose a fix, please make it concise.| @@ -0,0 +1,12 @@ | |||
| import englishMessages from "../../public/_locales/en/messages.json"; | |||
|
|
|||
| export type MessageKey = keyof typeof englishMessages; | |||
There was a problem hiding this comment.
MessageKey likely won’t be a literal key union from the JSON import (often just string), so a bad key could make englishMessages[key].message throw. Consider tightening the key type to string and guarding the fallback lookup.
| export type MessageKey = keyof typeof englishMessages; | |
| export type MessageKey = keyof typeof englishMessages & string; | |
| export const msg = (key: MessageKey, substitutions?: string | string[]) => { | |
| const localizedMessage = | |
| typeof chrome === "undefined" | |
| ? "" | |
| : chrome.i18n.getMessage(key, substitutions); | |
| return localizedMessage || englishMessages[key]?.message || key; | |
| }; |
Summary
Review context
This is the first focused split from #2008, which changed 426 files and exceeded the automated review limit. It addresses the four inline review comments without coupling tests or runtime diagnostics to translated copy. Follow-up PRs can migrate the remaining Chrome extension UI in smaller, reviewable groups.
Validation
Greptile Summary
This PR establishes the i18n foundation for the Chrome extension: it adds English and Simplified Chinese locale catalogs, updates
manifest.jsonto use__MSG_*__placeholders, and introduces a typedmsg()helper that wrapschrome.i18n.getMessagewith an English JSON fallback. It also decouples two E2E selectors from copy by adding stabledata-testidattributes to the close button and recording-error element.msg()ini18n.tscorrectly guards against an undefinedchromeglobal and falls back to the English catalog, with unit tests covering locale key alignment, locale resolution, and both fallback paths.default_locale: \"en\"setting is correct.data-testidattributes survive copy changes, and the existingaria-labelon the close button is preserved for accessibility.Confidence Score: 4/5
Safe to merge; changes are additive and well-tested with no impact on existing runtime paths beyond the offscreen justification string
The msg() helper's English fallback branch discards caller-supplied substitution arguments when Chrome returns an empty string. No current messages use placeholders, so nothing breaks today, but the helper would silently mis-render any future message that does.
apps/chrome-extension/src/shared/i18n.ts — the English fallback branch ignores the substitutions argument
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(chrome-extension): add localization..." | Re-trigger Greptile