feat(i18n): fail CI on translation keys that don't exist - #4233
Conversation
There was a problem hiding this comment.
Pull request overview
Adds TypeScript typing for vue-i18n locale messages by declaring en_US as the global schema via DefineLocaleMessage, improving editor completion and safer refactors across the frontend i18n keyspace. This also introduces a drift test to ensure new namespace JSON files are reflected in the schema, and updates a v2 unit test to use a local, per-instance schema.
Changes:
- Add
vue-i18nmodule augmentation (DefineLocaleMessage) based on theen_USnamespaces to drive typed key completion. - Cast the glob-built
messagesobject at the i18n setup boundary to the global schema type. - Add/adjust tests: a schema drift test for namespace coverage, and a per-instance typed
createI18ninPlatformsStatsSection.test.ts.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/v2/components/Settings/PlatformsStatsSection.test.ts | Types the test-local i18n instance schema for the single settings namespace. |
| frontend/src/locales/vue-i18n.d.ts | Adds global DefineLocaleMessage augmentation using en_US namespaces as the schema source. |
| frontend/src/locales/schema.test.ts | Adds a drift test to ensure every en_US namespace JSON is declared in the schema file. |
| frontend/src/locales/index.ts | Imports DefineLocaleMessage and casts the glob-assembled messages object to the schema type at the boundary. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Greptile SummaryThis PR defines the application-wide vue-i18n message schema from the
Confidence Score: 4/5The PR appears safe to merge, with one non-blocking limitation in the schema drift test's handling of hyphenated namespace names. The runtime locale behavior remains unchanged, and the only accepted concern is that the new test recognizes a narrower namespace syntax than the existing loader. Files Needing Attention: frontend/src/locales/schema.test.ts Important Files Changed
Prompt To Fix All With AI### Issue 1
frontend/src/locales/schema.test.ts:18
**Namespace parser excludes hyphens**
The loader accepts hyphenated namespace filenames, but `\w+` excludes their quoted interface properties from `declared`, so a valid runtime namespace and schema declaration fail this drift test.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(i18n): type locale messages with a ..." | Re-trigger Greptile |
| .sort(); | ||
|
|
||
| const schema = readFileSync(join(dir, "vue-i18n.d.ts"), "utf8"); | ||
| const declared = [...schema.matchAll(/^ {4}(\w+): typeof \w+;$/gm)] |
There was a problem hiding this comment.
Namespace parser excludes hyphens
The loader accepts hyphenated namespace filenames, but \w+ excludes their quoted interface properties from declared, so a valid runtime namespace and schema declaration fail this drift test.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/locales/schema.test.ts
Line: 18
Comment:
**Namespace parser excludes hyphens**
The loader accepts hyphenated namespace filenames, but `\w+` excludes their quoted interface properties from `declared`, so a valid runtime namespace and schema declaration fail this drift test.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.vue-i18n renders an unknown key as the key itself, so a typo or a rename that misses a call site ships as `settings.scan-settings` on screen. The existing checks compare locales against each other and never look at how the frontend uses them, and TypeScript can't help: `t()` accepts any string by design, so dynamic keys keep working. check_i18n_keys.py resolves every literal `t()` / `$t()` / `tm()` key in src against en_US, and runs alongside the parity and sort checks. It skips dynamic keys and `te()`, whose whole job is asking whether a key is missing. It found five, now fixed: - The three firmware strings passed an English default, so they rendered but could never be translated. They are real keys now, in every locale, and the call sites drop the redundant fallback. - The v1 platforms-drawer row named two keys that never existed. They feed a `:key` attribute rather than any visible text, so nothing was broken on screen; it now points at the translated key the control beside it already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
12b08a8 to
8e1af2d
Compare
Auditing what the checker cannot see turned up two things.
The regex refused a `.` before `t(`, to keep `get(` and `format(` from
passing as calls. That also excluded every `i18n.global.t("...")` site,
52 of them, in the stores and the router. Only a word character has to be
refused, so a qualified call now counts.
Hand-resolving the nine template-literal keys found a real gap the
checker structurally can't: `PermEntity` carries `playlists`, the
permission catalog endpoint returns the whole enum, and the matrix labels
each row with `t(`settings.perm-entity.${e}`)`. There was no
`perm-entity.playlists`, so that row rendered its key. It exists now in
every locale, following the block's existing English-except-pt_BR style.
Nested key paths resolve properly too, so a literal
`t("settings.perm-entity.playlists")` no longer reads as missing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closing this: it grew past what it should be for the value it delivers. What it became was a new CI check plus 72 lines of locale strings I'd authored across 17 languages, bundled together. The translations in particular don't belong in a tooling PR, and the check itself would be a process change for every frontend PR, which is a maintainer call rather than something to slip in alongside a fix. Recording what came out of it, in case any of it is useful later:
The one real bug it surfaced is split out into #4234: the permissions matrix renders Also noted along the way, unfixed: the three |
Description
vue-i18n renders an unknown key as the key itself, so a typo or a rename that misses a call site ships as
settings.scan-settingson screen instead of "Scan settings". Nothing in CI catches that today:check_i18n_locales.pyandcheck_i18n_sorted.pycompare the locale files against each other and never look at how the frontend uses them.check_i18n_keys.pycloses that gap. It resolves every literalt()/$t()/tm()key underfrontend/srcagainsten_USand fails with afile:line: keylist, and it runs in the existing i18n workflow next to the other two checks. The workflow's path filter now also covers.ts/.vue, since a call site can break this without any locale file changing.Deliberately out of scope: dynamic keys (
t(m.labelKey),t(`settings.grant-${state}`)), which can't be resolved statically, andte(), whose whole purpose is asking whether a key is missing. Keys referenced only inside comments are skipped too.What it found, all fixed here
platform.firmware-missing-from-fs,platform.firmware-verified-tooltipandplatform.firmware-toggle-all-fswere called with an English string as the fallback argument, so they rendered correctly but were untranslatable in all 18 locales. They are real keys now, translated, and the three call sites drop the redundant fallback.settings.group-platforms-byandsettings.group-platforms-by-descin the v1 platforms-drawer settings row never existed. Worth being precise: they feed a:keyattribute rather than any rendered text, so nothing was visibly broken. The row'stitlenow points atsettings.platforms-drawer-group-by, the already-translated key the select beside it uses, and the unuseddescriptiongoes away. That keeps the v1 change to two lines, in line with v1 being frozen.Then, auditing what the checker can't see, two more:
.beforet(so thatget(andformat(wouldn't pass as calls, which also excluded all 52i18n.global.t("...")sites in the stores and the router. Only a word character needs refusing. Coverage went from 2735 to 2790 literal keys; no new misses appeared.settings.perm-entity.playlistswas missing and rendered its raw key.PermEntityincludesplaylists,/permissions/catalogreturns the whole enum,EditUserDialogpasses it straight to the matrix, and the matrix labels each row witht(`settings.perm-entity.${e}`). Dynamic keys are exactly what this checker can't resolve, so this came out of hand-resolving all nine template-literal patterns against their value unions. It's the only miss among them.Why this rather than TypeScript or eslint
This PR started as a vue-i18n
DefineLocaleMessageschema, which turned out to buy editor autocompletion and nothing in CI: the call signature is<Key extends string>(key: Key | ResourceKeys | number): string, soKeywidens to any string andt("common.nope")typechecks clean.@intlify/eslint-plugin-vue-i18n'sno-missing-keysis the usual answer and is ESLint 10 compatible, but it can't read this repo's layout. Keys here live inlocales/<locale>/<namespace>.jsonand are referenced asnamespace.key; the plugin builds its message tree from raw file contents with no filename prefix, so a trial run reported 2985 errors, essentially all false positives (settings.username"missing" because it looks forsettingsinsidesettings.json). Making it work would mean nesting all 304 locale JSONs under their namespace key, plus updating the loader and both existing checkers.A ~90-line script in the style of the two checkers already in this directory covers the same ground with no dependency and no restructuring.
Also worth a separate pass
The whole
settings.perm-entity/settings.perm-actionblock is untranslated English in every locale except pt_BR, so the permissions matrix renders English labels everywhere.playlistsfollows that existing style here rather than being the one translated row; translating the block properly is its own change.Verification
The checker was confirmed to catch a planted typo (
settings.theme-drak→src/v2/views/Settings/UserInterface.vue:73), to catch a bad nested path on a qualifiedi18n.global.t()call, and to pass on a clean tree.check_i18n_locales.pyandcheck_i18n_sorted.pyboth pass with the new keys.npm run typecheck,npm run test(736), prettier and eslint pass on the touched files. The fourvue/html-indentwarnings eslint reports inFirmwareTab.vueare pre-existing on master, in a block this PR doesn't touch.Checklist
The check itself is the test here; it's wired into CI rather than vitest, alongside the two existing i18n checkers.
AI assistance disclosure
This change was written with AI assistance (Claude Code). The AI wrote the checker, the fixes it surfaced and the translations, and ran the verification described above; the result was reviewed by me before submitting. The 51 new translated strings deserve a native-speaker eye in particular.