perf(i18n): drop the pseudo QA locale from production bundles (#2361)

Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
João Salavisa
2026-08-07 13:21:38 +02:00
committed by GitHub
parent dd2403dea5
commit ee40cd6235
12 changed files with 377 additions and 79 deletions
+4 -1
View File
@@ -74,8 +74,11 @@ jobs:
working-directory: apps
run: pnpm install --frozen-lockfile
# `build-web-e2e`, not `build-web`: the production web build no longer
# bundles the pseudo QA catalog, and tests/i18n/pseudo-coverage.spec.ts
# needs it. See apps/web/lib/i18n/bundling.ts.
- name: Build backend and web
run: make build-backend build-web
run: make build-backend build-web-e2e
# Fixture plugin package consumed by the plugin E2E specs. Built here
# (the build job has the Go toolchain) and shipped to the shards as its
+13 -3
View File
@@ -399,6 +399,16 @@ build-web:
@printf "$(CYAN)Building web app...$(RESET)\n"
@cd $(APPS_DIR) && VITE_KANDEV_API_PORT= VITE_KANDEV_DEBUG= $(PNPM) --filter @kandev/web build
## Web build for the E2E harness. Identical to build-web except that it keeps the
## pseudo QA catalog, which a production build drops (see
## apps/web/lib/i18n/bundling.ts). e2e/tests/i18n/pseudo-coverage.spec.ts is the
## only oracle for copy the jsx-only eslint guard cannot see, and it needs the
## catalog present in the artifact it runs against.
.PHONY: build-web-e2e
build-web-e2e:
@printf "$(CYAN)Building web app (with the pseudo QA locale)...$(RESET)\n"
@cd $(APPS_DIR) && VITE_KANDEV_API_PORT= VITE_KANDEV_DEBUG= $(PNPM) --filter @kandev/web build:e2e
.PHONY: build-web-quiet
build-web-quiet:
@printf " $(DIM)Web app$(RESET)\n"
@@ -508,17 +518,17 @@ test-scripts:
@node --test scripts/validate-public-docs.test.mjs
.PHONY: test-e2e
test-e2e: build-backend build-web build-e2e-plugin-package
test-e2e: build-backend build-web-e2e build-e2e-plugin-package
@printf "$(CYAN)Running E2E tests (headless, parallel)...$(RESET)\n"
@cd $(APPS_DIR) && $(PNPM) --filter @kandev/web e2e
.PHONY: test-e2e-headed
test-e2e-headed: build-backend build-web build-e2e-plugin-package
test-e2e-headed: build-backend build-web-e2e build-e2e-plugin-package
@printf "$(CYAN)Running E2E tests (headed)...$(RESET)\n"
@cd $(APPS_DIR) && $(PNPM) --filter @kandev/web e2e:headed
.PHONY: test-e2e-ui
test-e2e-ui: build-backend build-web build-e2e-plugin-package
test-e2e-ui: build-backend build-web-e2e build-e2e-plugin-package
@printf "$(CYAN)Opening Playwright UI mode...$(RESET)\n"
@cd $(APPS_DIR) && $(PNPM) --filter @kandev/web e2e:ui
+64
View File
@@ -19,6 +19,8 @@ export default function globalSetup() {
throw new Error(`Vite web build not found: ${spaIndex}\nRun "make build-web" first.`);
}
assertPseudoCatalogBundled();
// tests/plugins/plugins.spec.ts installs this package through the real
// upload UI. Like the binaries above, this only checks existence — not
// freshness — so rebuild after touching cmd/plugin-fixture (see
@@ -30,3 +32,65 @@ export default function globalSetup() {
);
}
}
/**
* Fail fast when the coverage oracle is asked to run against a bundle that has
* no pseudo catalog.
*
* A production `vite build` deliberately drops it (see lib/i18n/bundling.ts).
* Run tests/i18n/pseudo-coverage.spec.ts against such a bundle and every screen
* falls back to `en` — so all ten tests fail with a wall of findings that reads
* exactly like a mass externalization regression, when the real cause is that
* the artifact was built with `make build-web` instead of `make build-web-e2e`.
* Only checked under KANDEV_I18N_COVERAGE=1; nothing else needs the catalog.
*
* The markers are read from the catalogs themselves rather than hardcoded, so
* this cannot drift as `pnpm run i18n:pseudo` regenerates them — and the whole
* SOURCE directory is scanned rather than one named file, so adding, renaming or
* splitting a namespace cannot quietly turn the check into a no-op. Every path
* out of here either throws or has actually compared markers against `dist/`: a
* guard that can only ever pass is worth less than no guard, because it also
* reports clean.
*/
function assertPseudoCatalogBundled() {
if (process.env.KANDEV_I18N_COVERAGE !== "1") return;
const catalogDir = path.join(WEB_DIR, "src", "locales", "pseudo");
const catalogs = fs.existsSync(catalogDir)
? fs.readdirSync(catalogDir).filter((name) => name.endsWith(".json"))
: [];
const markers = catalogs
.flatMap((name) =>
Object.values(
JSON.parse(fs.readFileSync(path.join(catalogDir, name), "utf8")) as Record<string, unknown>,
),
)
.filter((value): value is string => typeof value === "string" && value.length >= 20)
.slice(0, 50);
if (markers.length === 0) {
throw new Error(
`KANDEV_I18N_COVERAGE=1 but no usable pseudo source catalog was found in ${catalogDir}.\n` +
'The coverage oracle cannot mean anything without it — run "pnpm run i18n:pseudo" ' +
"to regenerate it. See docs/i18n.md.",
);
}
const assets = path.join(WEB_DIR, "dist", "assets");
const bundled = fs
.readdirSync(assets)
.filter((name) => name.endsWith(".js"))
.some((name) => {
const source = fs.readFileSync(path.join(assets, name), "utf8");
return markers.some((marker) => source.includes(marker));
});
if (!bundled) {
throw new Error(
"KANDEV_I18N_COVERAGE=1 but dist/ contains no pseudo catalog, so the coverage " +
"oracle would report every screen as un-externalized.\n" +
'Rebuild the SPA with the QA locale: "make build-web-e2e" ' +
'(or "pnpm --filter @kandev/web build:e2e"). See docs/i18n.md.',
);
}
}
+7 -2
View File
@@ -92,9 +92,14 @@ restore_pr_asset_ownership() {
}
}
# `build:e2e`, not `build`: a plain `vite build` drops the pseudo catalog from
# the bundle (it is the largest locale and no production user can select it), and
# tests/i18n/pseudo-coverage.spec.ts is the oracle that needs it. See
# apps/web/lib/i18n/bundling.ts. Mirror any change here in the CI e2e build job
# (.github/workflows/e2e-tests.yml -> make build-web-e2e).
build_fe() {
log "building Vite web assets"
( cd "$REPO_ROOT/apps" && pnpm --filter @kandev/web build ) \
log "building Vite web assets (with the pseudo QA locale)"
( cd "$REPO_ROOT/apps" && pnpm --filter @kandev/web build:e2e ) \
|| die "FE build failed"
}
+40
View File
@@ -0,0 +1,40 @@
import { describe, expect, it } from "vitest";
import { PSEUDO_BUNDLE_ENV_VAR, shouldBundlePseudoLocale } from "./bundling";
/**
* Both directions, deliberately.
*
* This switch is only exercised for real by a `vite build`, so nothing in the
* unit or e2e suites would notice it silently stuck on "include" — which is the
* state it is replacing. Asserting only the e2e/dev side would leave the whole
* point of the change untested.
*/
describe("shouldBundlePseudoLocale", () => {
it("excludes pseudo from a plain production build", () => {
expect(shouldBundlePseudoLocale({ command: "build", env: {} })).toBe(false);
});
it("keeps pseudo when a build opts in", () => {
for (const value of ["1", "true", "yes", "TRUE", " 1 "]) {
expect(
shouldBundlePseudoLocale({ command: "build", env: { [PSEUDO_BUNDLE_ENV_VAR]: value } }),
`${PSEUDO_BUNDLE_ENV_VAR}=${JSON.stringify(value)} should opt in`,
).toBe(true);
}
});
it("does not treat an arbitrary non-empty value as opting in", () => {
for (const value of ["", "0", "false", "no", "off"]) {
expect(
shouldBundlePseudoLocale({ command: "build", env: { [PSEUDO_BUNDLE_ENV_VAR]: value } }),
`${PSEUDO_BUNDLE_ENV_VAR}=${JSON.stringify(value)} should not opt in`,
).toBe(false);
}
});
it("always keeps pseudo in serve mode, which covers `pnpm dev` and vitest", () => {
expect(shouldBundlePseudoLocale({ command: "serve", env: {} })).toBe(true);
expect(shouldBundlePseudoLocale({ command: "serve" })).toBe(true);
});
});
+42
View File
@@ -0,0 +1,42 @@
/**
* Build-time decision: does this bundle compile in the `pseudo` QA catalog?
*
* `pseudo` is the largest catalog we ship (accented characters are multi-byte)
* and no production user can ever select it — `selectableLocales` has always
* filtered it out of the switcher. Until this switch existed it was still
* *bundled*, so every user downloaded it. Filtering at runtime cannot fix that;
* the entries have to be dropped before rolldown builds the module graph.
*
* `import.meta.env.PROD` is the wrong signal, and the reason is the whole
* difficulty of this change: the e2e harness serves a **production** bundle, so
* that constant is true there too. e2e is exactly where `pseudo` must survive —
* `e2e/tests/i18n/pseudo-coverage.spec.ts` is the only oracle that sees copy the
* jsx-only eslint guard structurally cannot (strings in variables, default
* parameters, `.ts` helpers, SCREAMING_CASE config tables). So the decision is
* taken from the *build invocation* instead: `vite dev` and vitest always keep
* it, and a `vite build` keeps it only when asked to.
*
* Kept as a pure function, separate from `vite.config.ts`, so both directions
* are unit-tested. A bundling switch is only ever exercised by a real
* production build, and one that is silently stuck on "include" is
* indistinguishable from one that works.
*/
/** Environment variable that opts a `vite build` into shipping `pseudo`. */
export const PSEUDO_BUNDLE_ENV_VAR = "KANDEV_I18N_BUNDLE_PSEUDO";
/** The same truthy set the Makefile uses (`$(filter 1 true yes,...)`). */
const TRUTHY = new Set(["1", "true", "yes"]);
export function shouldBundlePseudoLocale(options: {
/** Vite's `ConfigEnv.command`. */
command: string;
/** Usually `process.env`. */
env?: Record<string, string | undefined>;
}): boolean {
// `serve` covers `pnpm dev` and vitest, which resolves config in serve mode.
// Neither produces an artifact a user downloads, and both want the oracle.
if (options.command === "serve") return true;
const raw = options.env?.[PSEUDO_BUNDLE_ENV_VAR];
return TRUTHY.has((raw ?? "").trim().toLowerCase());
}
+22 -1
View File
@@ -6,6 +6,7 @@ import {
i18n,
isSupportedLocale,
normalizeLocale,
PSEUDO_LOCALE_BUNDLED,
selectableLocales,
SUPPORTED_LOCALES,
} from "./index";
@@ -50,10 +51,30 @@ describe("locale predicates", () => {
expect([...SUPPORTED_LOCALES]).toEqual(["en", "pt-pt", "zh-cn", "pseudo"]);
});
it("hides the pseudo locale in production builds only", () => {
// Only the `isProd` half of the contract. `selectableLocales` also requires
// `PSEUDO_LOCALE_BUNDLED`, and that half cannot be exercised here: the constant
// is fixed to `true` for the whole vitest run (config resolves in serve mode).
// `bundling.test.ts` covers the decision that produces it, in both directions.
it("hides the pseudo locale from production builds", () => {
expect(selectableLocales(false)).toEqual(["en", "pt-pt", "zh-cn", "pseudo"]);
expect(selectableLocales(true)).toEqual(["en", "pt-pt", "zh-cn"]);
});
/**
* The include half of the bundling switch, asserted end to end.
*
* `shouldBundlePseudoLocale` is unit-tested in `bundling.test.ts`, but that
* only covers the decision — not that `vite.config.ts` actually defines
* `__KANDEV_PSEUDO_LOCALE_BUNDLED__` and that the constant reaches this module.
* If that wiring breaks in the "exclude" direction, dev and e2e lose the
* pseudo oracle silently: every screen simply falls back to `en` and reads as
* un-externalized. This pins it, and "resolves real catalog messages for the
* active locale" below is what proves the catalog itself came with it.
*/
it("bundles the pseudo catalog outside a production build", () => {
expect(PSEUDO_LOCALE_BUNDLED).toBe(true);
expect(selectableLocales(false)).toContain("pseudo");
});
});
describe("activateLocale", () => {
+42 -8
View File
@@ -9,8 +9,8 @@ import { writeLocaleCookie } from "./cookie";
* `en` is the source locale; `pseudo` is a QA-only accented/padded locale used
* to visually prove every string was externalized (any plain-ASCII text on
* screen under `pseudo` is a literal that was never routed through `t`).
* `pseudo` is filtered out of the language switcher in production builds, but
* the runtime can still activate it if a `kandev_locale=pseudo` cookie is set.
* `pseudo` is filtered out of the language switcher in production builds, and
* its catalog is not compiled into them at all — see `PSEUDO_LOCALE_BUNDLED`.
*
* Catalogs live at `src/locales/<locale>/<namespace>.json` and are loaded
* eagerly per locale via Vite's glob import, so a locale switch needs no
@@ -55,6 +55,16 @@ export function normalizeLocale(value: unknown): SupportedLocale {
: DEFAULT_LOCALE;
}
/**
* Whether this bundle contains the pseudo catalog at all.
*
* `pseudo` stays in `SUPPORTED_LOCALES` unconditionally — that type drives the
* `en` ↔ `pseudo` parity gate in `i18n:check` and the generator — so this is the
* only thing that distinguishes "shipped" from "supported". `false` in a plain
* `vite build`; see `./bundling.ts`.
*/
export const PSEUDO_LOCALE_BUNDLED: boolean = __KANDEV_PSEUDO_LOCALE_BUNDLED__;
/**
* Locales offered by the switcher. The pseudo QA locale is hidden in production.
*
@@ -62,18 +72,42 @@ export function normalizeLocale(value: unknown): SupportedLocale {
* `import.meta.env.PROD`: the e2e harness serves a production bundle, so that
* constant is true there too and would hide the locale the QA tests need. The
* boot payload's `nonProduction` flag is the authoritative signal.
*
* `PSEUDO_LOCALE_BUNDLED` is checked as well, because those two signals can now
* disagree: a released binary started with `KANDEV_DEBUG_DEV_MODE=true` reports
* `nonProduction` while serving a bundle that has no pseudo catalog. Offering it
* there would switch the user to a locale that resolves entirely through the
* `en` fallback — plain English under a "Pseudo (QA)" label, which reads exactly
* like a total externalization failure.
*/
export function selectableLocales(isProd: boolean): SupportedLocale[] {
return SUPPORTED_LOCALES.filter((locale) => locale !== "pseudo" || !isProd);
return SUPPORTED_LOCALES.filter(
(locale) => locale !== "pseudo" || (!isProd && PSEUDO_LOCALE_BUNDLED),
);
}
type CatalogModule = Record<string, unknown>;
// Vite resolves this at build time; each entry is `../../src/locales/<locale>/<ns>.json`.
const catalogModules = import.meta.glob<CatalogModule>("../../src/locales/*/*.json", {
eager: true,
import: "default",
});
// Vite resolves these at build time; each entry is `../../src/locales/<locale>/<ns>.json`.
//
// `pseudo` is globbed separately, behind the build-time constant, so a production
// build folds the branch away and rolldown never pulls those JSON modules into
// the graph. Filtering the merged object at runtime instead would still ship
// every byte — and the bytes are the entire problem: pseudo is the LARGEST
// catalog we have (accented characters are multi-byte) and no production user
// can select it.
const catalogModules: Record<string, CatalogModule> = {
...import.meta.glob<CatalogModule>(
["../../src/locales/*/*.json", "!../../src/locales/pseudo/*.json"],
{ eager: true, import: "default" },
),
...(__KANDEV_PSEUDO_LOCALE_BUNDLED__
? import.meta.glob<CatalogModule>("../../src/locales/pseudo/*.json", {
eager: true,
import: "default",
})
: {}),
};
function localeResources(locale: SupportedLocale): Record<string, CatalogModule> {
const resources: Record<string, CatalogModule> = {};
+1
View File
@@ -8,6 +8,7 @@
"prebuild": "node scripts/generate-release-notes.mjs && node scripts/generate-changelog.mjs",
"build": "vite build",
"build:vite": "vite build",
"build:e2e": "cross-env KANDEV_I18N_BUNDLE_PSEUDO=1 pnpm run build",
"start": "vite preview --host 0.0.0.0",
"lint": "eslint --max-warnings 0",
"lint:i18n": "eslint --config eslint.i18n.config.mjs",
+7
View File
@@ -1 +1,8 @@
/// <reference types="vite/client" />
/**
* Injected by the `kandev:i18n-pseudo-locale-bundling` plugin in `vite.config.ts`.
* `false` in a plain `vite build`, `true` under `vite dev`, vitest, and a build
* run with `KANDEV_I18N_BUNDLE_PSEUDO=1` (the e2e artifact).
*/
declare const __KANDEV_PSEUDO_LOCALE_BUNDLED__: boolean;
+28 -2
View File
@@ -1,9 +1,11 @@
import path from "node:path";
import react from "@vitejs/plugin-react";
import { defineConfig } from "vite";
import { defineConfig, type Plugin } from "vite";
import { shouldBundlePseudoLocale } from "./lib/i18n/bundling";
export default defineConfig({
plugins: [react()],
plugins: [react(), pseudoLocaleBundling()],
server: {
port: readPort(process.env.PORT),
strictPort: Boolean(process.env.PORT),
@@ -26,6 +28,30 @@ export default defineConfig({
},
});
/**
* Defines `__KANDEV_PSEUDO_LOCALE_BUNDLED__`, which gates the `pseudo` catalog's
* `import.meta.glob` in `lib/i18n/index.ts`. See `lib/i18n/bundling.ts` for why
* the build invocation — not `import.meta.env.PROD` — decides this.
*
* A plugin rather than a top-level `define` because only the `config` hook is
* handed `ConfigEnv.command`, and keeping the default export an object literal
* leaves `vitest.config.ts`'s `mergeConfig(viteConfig, …)` working unchanged.
*/
function pseudoLocaleBundling(): Plugin {
return {
name: "kandev:i18n-pseudo-locale-bundling",
config(_userConfig, env) {
return {
define: {
__KANDEV_PSEUDO_LOCALE_BUNDLED__: JSON.stringify(
shouldBundlePseudoLocale({ command: env.command, env: process.env }),
),
},
};
},
};
}
function readPort(value: string | undefined): number | undefined {
if (!value) return undefined;
const port = Number(value);
+107 -62
View File
@@ -56,32 +56,38 @@ band, so `i18n:check` reports a real-locale gap as a warning. See
```json
{ "deletedItems": "Deleted {{count}} items" }
```
```tsx
t("common:deletedItems", { count })
t("common:deletedItems", { count });
```
### Plurals
i18next uses suffixed keys; the call site passes `count`:
```json
{ "sessions_one": "{{count}} session", "sessions_other": "{{count}} sessions" }
```
```tsx
t("kanban:sessions", { count: sessionCount })
t("kanban:sessions", { count: sessionCount });
```
### Copy containing markup
Use react-i18next's `<Trans>`. Tag indices count **every** child (text nodes
included), and the tag body is the element's text:
```tsx
<Trans i18nKey="settings:typeToConfirm" values={{ phrase }}>
Type <strong>{phrase}</strong> to continue.
</Trans>
```
```json
{ "typeToConfirm": "Type <1>{{phrase}}</1> to continue." }
```
Pass `values` explicitly whenever the message interpolates.
Because the indices are positional, **the message and the JSX are coupled**:
@@ -99,10 +105,12 @@ drifted indices onto the actual element positions. Never put a `t()` call inside
`.ts` helpers and non-component functions cannot use the hook — import the
module-level `t`, which binds to the shared instance:
```ts
import { t } from "@/lib/i18n";
export const label = () => t("common:saved");
```
Note this resolves at call time; a value captured at module load will not update
on a locale switch.
@@ -122,7 +130,7 @@ rule ends up at the call site, so a language with three or six forms has nowhere
to express them, and a translator editing the catalog cannot fix it.
`scripts/check-inline-plurals.mjs` (part of `i18n:check`) rejects that shape, and
`scripts/fix-inline-plurals.mjs --write` converts existing ones. Add `_one`/`_other`
to *every* message that interpolates `{{count}}`, even where English does not
to _every_ message that interpolates `{{count}}`, even where English does not
inflect (`{{count}} selected`) — other languages agree the participle with the
number, and identical forms cost nothing.
@@ -145,8 +153,8 @@ token in a constant and interpolate it, so the shown and compared values cannot
drift:
```tsx
const CONFIRM_TOKEN = "RESET"; // sentinel, never translated
t("settings:typeToConfirm", { token: CONFIRM_TOKEN })
const CONFIRM_TOKEN = "RESET"; // sentinel, never translated
t("settings:typeToConfirm", { token: CONFIRM_TOKEN });
```
The guard's `words.exclude` carries a multi-word SCREAMING-CASE pattern for this
@@ -182,7 +190,7 @@ directory you are about to migrate; it never gates a merge. The real gate is
Unlike the eslint guard, `i18n:check` is **not** scoped to the allowlist — its
four checks scan `components`, `app`, `hooks` and `lib` repo-wide, and it runs in
CI (Frontend Tests → *Run i18n checks*). That is deliberate: each one pins a bug
CI (Frontend Tests → _Run i18n checks_). That is deliberate: each one pins a bug
that fails **silently at runtime** rather than merely leaving a string in English,
so there is no reason to let it into unmigrated code either. Unmigrated files pass
trivially, because a file that never calls `t()` cannot break any of the four.
@@ -198,9 +206,43 @@ scope.
Module scope matters because the module-level `t` resolves when the file is
imported: `const ROWS = [{ label: t("k") }]` pins that copy to the boot locale, so
it renders correctly and simply never changes on a switch. The pseudo-locale
cannot see it — the text *is* translated. Store the key and resolve it at render,
cannot see it — the text _is_ translated. Store the key and resolve it at render,
or make the value a component.
### Pseudo is not bundled in production
`pseudo` is the **largest** catalog we have — accented characters are multi-byte —
and no production user can ever select it, because `selectableLocales` filters it
out of the switcher. So a plain `vite build` does not compile it in at all:
`lib/i18n/index.ts` globs it separately behind `__KANDEV_PSEUDO_LOCALE_BUNDLED__`,
which `vite.config.ts` defines from `shouldBundlePseudoLocale` in
`lib/i18n/bundling.ts`. The branch folds away and rolldown never pulls those JSON
modules into the graph. Filtering the merged catalog at runtime instead would
still ship every byte, which is the entire problem.
| invocation | pseudo? |
| ----------------------------------------------------------------------- | ------- |
| `pnpm dev`, `vitest` (Vite `serve`) | yes |
| `pnpm build` / `make build-web` / release / desktop | **no** |
| `pnpm build:e2e` / `make build-web-e2e` (`KANDEV_I18N_BUNDLE_PSEUDO=1`) | yes |
`import.meta.env.PROD` cannot make this decision: the e2e harness serves a
**production** bundle, so it is true there too — and e2e is exactly where pseudo
has to survive, because `e2e/tests/i18n/pseudo-coverage.spec.ts` is the only
oracle that sees copy the jsx-only guard structurally cannot. That is why the e2e
runner (`e2e/scripts/run-e2e.sh`) and the CI e2e build job both use the `:e2e`
target. **If you add another build path that e2e runs against, use that target** —
the coverage spec fails loudly rather than silently when pseudo is absent (every
screen falls back to `en` and reads as un-externalized), but it fails as a wall of
findings that look like a migration regression rather than a build misconfiguration.
`SUPPORTED_LOCALES` still lists `pseudo` unconditionally, and must: that type
drives the `en` ↔ `pseudo` parity gate in `i18n:check` and `i18n:pseudo`. The
exclusion is a bundling decision only.
To eyeball pseudo in a production-mode local instance (rather than `pnpm dev`),
build with `make build-web-e2e` instead of `make build-web`.
## The guard is scoped, on purpose
`i18next/no-literal-string` is an **error**, but only for the paths listed in
@@ -231,12 +273,12 @@ exists — globs included, resolved with `fs.globSync`. So **never remove an ent
to make a build pass, and equally, never tidy several file globs into one
`dir/**`. Both produce the same diff to the array, the check cannot tell them
apart, and refusing to guess is the point: one of them silently un-protects
migrated copy. A broader glob added *alongside* the existing entries passes;
migrated copy. A broader glob added _alongside_ the existing entries passes;
swapping them for it does not.
The same check rejects a path listed **twice**. An exact duplicate changes no
behaviour, so nothing else looks for one — the eslint rule is indifferent to a
repeated pattern, and the removal check above only inspects entries that *left*
repeated pattern, and the removal check above only inspects entries that _left_
the array. That is how #2214 landed two past every gate the repo then had. What a
duplicate costs is the record: it claims an earlier migration's coverage as the
new PR's. Entries that a broader glob already covers are deliberate and stay —
@@ -245,7 +287,7 @@ check, not a redundancy check.
#### An entry can be born dead, and only a glob expansion shows it
Both checks above ask what happened to an entry *between* two states. Neither
Both checks above ask what happened to an entry _between_ two states. Neither
asks the prior question: **does this entry match any file at all?** An entry that
matched nothing on the day it was added is indistinguishable from a healthy one —
it never leaves the array, it is never a duplicate, and `pnpm lint` passes
@@ -267,13 +309,13 @@ it. So when an entry contains `[`, escape it as `[[]…[]]`.
particular class cannot recur — the three checks it runs are deliberately
separate because each is blind to the others' failure:
| check | sees |
|---|---|
| removal | an entry that **left** the array while its path still exists |
| duplicate | an entry listed **twice** |
| check | sees |
| --------- | --------------------------------------------------------------------------- |
| removal | an entry that **left** the array while its path still exists |
| duplicate | an entry listed **twice** |
| unmatched | an entry that **never matched anything**, including on the day it was added |
The first two compare two *states* of the array, which is why neither can see a
The first two compare two _states_ of the array, which is why neither can see a
born-dead entry. If you add a check here, add the test that shows the existing
ones miss what it catches — `scripts/lib/guard-allowlist.test.ts` pins that for
all three.
@@ -284,10 +326,10 @@ Scoping the whole-file guard leaves a hole: a brand-new component under an
un-migrated directory would be unchecked. `check-new-code-i18n.mjs` closes it by
judging the **change** rather than the file:
| | Judged on |
|---|---|
| A file the change **added** | the whole file — it carries no legacy debt |
| A file the change **modified** | only the lines it touched |
| | Judged on |
| ------------------------------ | ------------------------------------------ |
| A file the change **added** | the whole file — it carries no legacy debt |
| A file the change **modified** | only the lines it touched |
Existing literals elsewhere in a modified file are somebody else's migration, so
this can be repo-wide without recreating the treadmill. It is the same contract
@@ -325,12 +367,14 @@ pnpm run lint:i18n components/<area> # 5. clean?
8. `pnpm run i18n:check && pnpm lint --max-warnings 0 && pnpm exec vitest run`.
9. Switch to **Pseudo (QA)** in Settings → General → Appearance and look at the
screen. Anything still in plain English was missed — see below for why lint
cannot tell you this.
cannot tell you this. Use `pnpm dev`; a production build does not bundle the
pseudo catalog and will not offer the option (see _Pseudo is not bundled in
production_).
### The guard has blind spots
`i18next/no-literal-string` only inspects **literals in JSX**. Template literals
*are* now checked (`should-validate-template: true` — measured at +2 to +5
_are_ now checked (`should-validate-template: true` — measured at +2 to +5
findings per un-migrated directory, all of them real copy), but it still cannot
see a `confirm()` argument or copy returned from a plain `.ts` helper.
@@ -338,9 +382,9 @@ It has one more blind spot worth knowing, because it is the shape config tables
take: **a literal assigned to a SCREAMING_CASE identifier is skipped entirely.**
```tsx
const HEADER = <span>Archive board</span>; // NOT flagged
const header = <span>Archive board</span>; // flagged
const ROWS = [{ label: "Disk usage" }]; // NOT flagged
const HEADER = <span>Archive board</span>; // NOT flagged
const header = <span>Archive board</span>; // flagged
const ROWS = [{ label: "Disk usage" }]; // NOT flagged
```
That is why module-scope config objects survived the sweep, and why the
@@ -372,7 +416,7 @@ export function ActionButtonContent({
}: Props) {
```
These are evaluated in the parameter list, which runs *before* the component
These are evaluated in the parameter list, which runs _before_ the component
body, so a `const { t } = useTranslation()` in the body is not in scope for them —
you cannot fix this in place. Resolve the fallback inside the body instead
(`const pending = pendingLabel ?? t("system:jobRunning")`). Prop defaults on a
@@ -396,7 +440,7 @@ exits 0. It prints two separate sections, because mixing "this is broken" with
1. **English plural concatenation** — `{n} file{n === 1 ? "" : "s"}`, and the
hoisted `const plural = count === 1 ? "" : "s"` form. These are defects, and
**no gate we own sees them**: `no-literal-string` is jsx-only, and
`check-inline-plurals.mjs` only inspects morphemes passed *through* `t()`. A
`check-inline-plurals.mjs` only inspects morphemes passed _through_ `t()`. A
repo-wide sweep found 44 of them across 36 files. One helper had 2 of its 11
call sites visible to lint — migrating the visible two would have left nine
broken while the directory reported clean. **30 of those 44 are under
@@ -408,7 +452,7 @@ exits 0. It prints two separate sections, because mixing "this is broken" with
content is sent verbatim to a model, so translating it would be a bug
(`components/github/pr-checks-section.tsx` and `pr-ci-popover.tsx` are the
known-good examples, and the tool prints this caveat itself). And the section
is a *narrow sample*, not an inventory — its noise filter drops any literal
is a _narrow sample_, not an inventory — its noise filter drops any literal
starting with a letter, so it surfaces bracketed log prefixes and media
queries but not a `{ label: "Disk usage" }` config table. A short section is
not evidence a directory is clean.
@@ -435,7 +479,7 @@ the completeness check.
these are localized at the **call site**, not in the package. Check the shape
before inventing a seam, because the two halves differ:
- **A label in an ATTRIBUTE needs nothing.** Every one of them sits on an
element that spreads `{...props}` *after* it, so the overridable string prop
element that spreads `{...props}` _after_ it, so the overridable string prop
the spec asks for already exists and the English literal is only its default.
`Breadcrumb`'s `aria-label="breadcrumb"` was this shape: the fix was
`aria-label={t("common:breadcrumb")}` at its three consumers (`page-topbar`,
@@ -455,8 +499,9 @@ the completeness check.
A third-party primitive is the same call: sonner names its toast region
`Notifications alt+T` and accepts `containerAriaLabel`, which `AppToaster` in
`src/app-shell.tsx` passes. That has to be its own component — `<I18nProvider>`
is mounted *by* `AppShell`, so a `t()` call in `AppShell`'s own render sits
is mounted _by_ `AppShell`, so a `t()` call in `AppShell`'s own render sits
outside the provider and would not re-render on a locale switch.
- **Tests**: `vitest.setup.ts` calls `initI18nForTests()`. react-i18next falls
back to the default instance when no provider is present, so no render wrapper
is needed and every test's render tree is unchanged.
@@ -530,7 +575,7 @@ the share builders mix all of them into the same lines:
- **Emoji**, which stay in the Go format string rather than the catalog.
**Contract for new user-facing backend output:** prefer returning a stable error
*code* the frontend translates. Only use `i18n.T` when Go itself renders the bytes
_code_ the frontend translates. Only use `i18n.T` when Go itself renders the bytes
a user reads.
## Pseudo-locale QA (the completeness oracle)
@@ -560,7 +605,7 @@ stays English by design, so it renders unaccented and is still correct. See
Two things it cannot show you at all — the second is detailed below:
1. **Module-scope `t()`.** That copy *is* translated, just frozen at the boot
1. **Module-scope `t()`.** That copy _is_ translated, just frozen at the boot
locale, so it renders accented and looks correct while never responding to a
switch. `check-module-scope-t.mjs` is what catches it.
2. **An un-migrated value interpolated into a migrated frame** — it renders
@@ -568,7 +613,7 @@ Two things it cannot show you at all — the second is detailed below:
A third used to belong on that list — copy that never becomes a text node — and
the spec now reads it directly. That pass is described next, because what it does
*not* check still matters when you migrate.
_not_ check still matters when you migrate.
### It reads attribute copy, which nothing on screen shows you
@@ -592,7 +637,7 @@ aria-label on button#language-select: "Display language (pseudo)"
renders as `Mōŕē ĩńfōŕmàţĩōń`, which has no 4-letter ASCII run and so is dropped
at the `wordlike` gate before the accented test ever sees it. Soundness is fine —
a real miss is pure ASCII — but it means a working pass and a pass whose selector
matched *nothing* produce the same empty output. One agent scanning by hand
matched _nothing_ produce the same empty output. One agent scanning by hand
concluded their scan was broken on exactly this. The spec therefore asserts
`inspectedAttributes > 0` per screen, prints how many rendered fully accented, and
pins accent detection once against `#language-select`'s known-migrated
@@ -616,9 +661,9 @@ output:
bury every other string on the screen.
- **One accented character does not clear an attribute**, unlike the text pass.
An `aria-label` built as `` `Collapse ${label}` `` renders `Collapse Ĝēńēŕàĺ` —
an un-migrated English *frame* around a migrated value, which accent-clears-all
an un-migrated English _frame_ around a migrated value, which accent-clears-all
would call done. Allowlist stripping is what separates that from the legitimate
case, where the ASCII fragment is allowlisted *data* rather than the frame
case, where the ASCII fragment is allowlisted _data_ rather than the frame
(`Àćţĩōńś ƒōŕ Agent` on the layouts screen strips to nothing). Such findings are
tagged `— English frame, migrated value`.
@@ -657,7 +702,7 @@ grep -rnE 't\("[^"]+", \{ (error|message)' components app hooks | grep -v '\.tes
see [Backend](#what-the-backend-deliberately-does-not-translate) for why the
~1,100 server error strings are out of scope. Around 37 catalog keys already
interpolate one; none of them is a miss. Where such a call needs a fallback for
a missing payload, the *fallback* is copy and is translated
a missing payload, the _fallback_ is copy and is translated
(`t("linear:testFailed", { error: result.error || t("linear:unknownError") })`)
while the payload itself is not.
- **A client-side literal reaching a translated frame is a miss.** A string
@@ -690,9 +735,9 @@ Which screens you walk matters as much as what you look for. A component's copy
is verified by walking **every route that mounts it** — not the routes your
migration owns.
The natural method fails here. Walking the import closure *from your own pages*
The natural method fails here. Walking the import closure _from your own pages_
traverses the graph in the wrong direction: it finds what your routes render, and
by construction never finds a route that renders *you*. One migration hit it in
by construction never finds a route that renders _you_. One migration hit it in
both directions — the cards it converted mount only on a sibling migration's
route, and a card that sibling owned mounts on its own. Neither could have been
verified from its own pages alone.
@@ -718,7 +763,7 @@ rediscovering the question.
### Copy that belongs to no directory
A file shared by *every* area has no natural owner and so gets skipped by all of
A file shared by _every_ area has no natural owner and so gets skipped by all of
them. `src/settings-routes.tsx` holds the live title and description for every
settings route in one `SETTINGS_ROUTES` map — a SCREAMING_CASE identifier the
guard skips entirely, in a 650-line file no single migration's closure claims.
@@ -739,15 +784,15 @@ Two surfaces rendering "the same" sentence have usually **drifted**. Settings
System had an unreferenced `app/settings/system/<route>/page.tsx` twin for every
route, and the twins were migrated at a different time by a different PR:
| surface | copy |
|---|---|
| `SETTINGS_ROUTES` — live, what users see | `Create a diagnostic ZIP with frontend and backend logs.` |
| surface | copy |
| --------------------------------------------------------------------------- | ------------------------------------------------------------------------- |
| `SETTINGS_ROUTES` — live, what users see | `Create a diagnostic ZIP with frontend and backend logs.` |
| `app/settings/system/logs/page.tsx` — dead → `settings:logsPageDescription` | `Download a bounded diagnostic ZIP containing frontend and backend logs.` |
Pointing the live route at the existing key changed the sentence on a shipping
page. `pnpm lint`, `pnpm run i18n:check`, the ratchet and 8,000+ unit tests were
all green — the key resolved, the catalogs matched, nothing was hardcoded. The
copy was simply *different*. `feature-toggles` had drifted the same way in the
copy was simply _different_. `feature-toggles` had drifted the same way in the
same directory.
Nothing in the toolchain can catch this, because every check verifies that a key
@@ -791,8 +836,8 @@ migration: a value extracted into `{{interpolation}}`, either half of a `<Trans>
sentence split around an element, and an `_one`/`_other` pair.
**The hazard has two shapes, and only one of them is above.** The case in this
section is an *attribute* — `description="…"` on a route shell — which is
quoted, so even a crude scan finds it. The other is *element text*, which is
section is an _attribute_ — `description="…"` on a route shell — which is
quoted, so even a crude scan finds it. The other is _element text_, which is
not quoted and is where most migrated copy lives; a reused key that rewrites a
button or a link label leaves nothing for a quoted-string search to find. Do not
read this section as "route descriptions drift". Any copy rendered from two
@@ -800,7 +845,7 @@ places can.
It is **advisory and deliberately not in CI**: it is diff-based, so it needs a
base ref (`i18n:check` is repo-wide on purpose), and a finding is a prompt to
*classify* rather than proof of a bug — each is a value extraction, a literal
_classify_ rather than proof of a bug — each is a value extraction, a literal
hoisted to a constant, or the real thing. It narrows the diff for a human; it
does not decide. `--base <ref>` overrides the `origin/main` fork point, and
`--all` widens past prose to identifier-shaped strings.
@@ -809,7 +854,7 @@ Some benign findings recur and are worth recognising rather than investigating:
an array of fragments collapsed into one comma-separated value is accepted
because every fragment survives verbatim inside the join — the safe property is
that the collapse is **concatenative**, not that it is an array, and a collapse
that also *rewords* is reported, correctly. The shape to be wary of is not
that also _rewords_ is reported, correctly. The shape to be wary of is not
"array collapsed to string", it is "collapse that also edits".
### Prove it can fail, before you believe it passed
@@ -821,13 +866,13 @@ check that read your diff and found nothing from one that could not have failed.
Both print the same tick.
That distinction is not hypothetical. Every defect found in this tool during
review had the same signature — *it returned a pass it had not earned* — and not
review had the same signature — _it returned a pass it had not earned_ — and not
one was visible to its own unit tests, because those assume a well-formed
catalog and a committed tree. A message that was nothing but a placeholder
compiled to a match-anything pattern and accepted every string in the
repository. The "after" side read `HEAD` while the file list came from the
working tree, so uncommitted edits were invisible and it printed a tick *with
the correct file count*. Each was found by someone running it for real and
working tree, so uncommitted edits were invisible and it printed a tick _with
the correct file count_. Each was found by someone running it for real and
declining to believe the result.
**The probe must be a rewrite, and it must change the opening words.** Two
@@ -848,7 +893,7 @@ A good probe changes the whole sentence: `"What do you want to change?"` →
message with too little of it earns no pattern at all — it needs two words of
prose. That threshold is not arbitrary. One short word clears any "is there a
word?" bar while leaving the pattern almost unbounded: `"Delete {{name}}"`
compiled to `/^Delete [\s\S]+?$/` and accepted *every* sentence beginning
compiled to `/^Delete [\s\S]+?$/` and accepted _every_ sentence beginning
"Delete ", which left 19 of 43 strings unprotected in the first file this check
was pointed at.
@@ -863,7 +908,7 @@ is a separate step, and why "the check was clean" is not on its own an answer to
One limit is structural rather than a tuning choice, and is worth knowing before
anyone tries to close it: `"{{count}} of {{total}}"` (a legitimate value
extraction) and `"{{a}} of {{b}}"` (far too permissive to anchor anything) both
reduce to the residue `"of"` and compile to the *same* regex. Nothing computed
reduce to the residue `"of"` and compile to the _same_ regex. Nothing computed
from the message can accept one and reject the other. Both are rejected, so a
string like `"3 of 9"` is reported and classified by hand — the right direction
for an advisory check, since a false positive costs someone ten seconds and a
@@ -943,16 +988,16 @@ Worth stating plainly, because assuming otherwise has already produced two wrong
conclusions: a green run does **not** mean "translated". The pass compares
shapes. Measured, one mutation at a time:
| mutation | result |
|---|---|
| value identical to English | **not caught** |
| whole catalog copy-pasted from `en` | **not caught** |
| swapped `_one` / `_other` | **not caught** |
| empty / whitespace / non-string value | caught |
| missing key, extra key | caught |
| missing namespace, extra namespace | caught |
| dropped `{{placeholder}}` | caught |
| dropped or renumbered `<n>` tag | caught |
| mutation | result |
| ------------------------------------- | -------------- |
| value identical to English | **not caught** |
| whole catalog copy-pasted from `en` | **not caught** |
| swapped `_one` / `_other` | **not caught** |
| empty / whitespace / non-string value | caught |
| missing key, extra key | caught |
| missing namespace, extra namespace | caught |
| dropped `{{placeholder}}` | caught |
| dropped or renumbered `<n>` tag | caught |
The identical-to-English tolerance is deliberate and cannot be tightened
naively: brand nouns and technical literals must stay verbatim (`automations`