diff --git a/client/CLAUDE.md b/client/CLAUDE.md index ccff8474f..bef0198a8 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -64,7 +64,7 @@ The offline core is flagship work surrounded by a periphery that ignores it. New ## Big-picture pieces - **Maps** (`src/components/Map/`): two interchangeable renderers — Leaflet and Mapbox/MapLibre GL — chosen at runtime by `MapViewAuto.tsx`. Keep both in sync when changing map features, and keep `mapbox-gl` and `maplibre-gl` in separate chunks (`check:gl-split`). The raster prefetcher fetches tiles `no-cors` so custom tile providers without CORS headers keep working; the vector prefetcher deliberately uses `cors` because it has to read the responses — don't "align" one with the other. -- **i18n** (`src/i18n/TranslationContext.tsx`): `en` is bundled; every other locale is a dynamic `import('@trek/shared/i18n/')` so Vite emits one chunk per locale. Strings live in `shared/`, never here. `t()` returns an unknown key unchanged, so `lint:i18n-keys` (`scripts/i18n-keys.mjs`) resolves every literal key in `t()`, `tHtml()`, `tr()`, ``, `translateApiError` and `*Key` props against `shared/src/i18n/en`; a template key is read with each interpolation as one key segment and passes by itself only when it reaches between 1 and `MAX_IMPLICIT_MATCHES` (30) en keys; a wider one, or one with no fixed dotted prefix, needs a `DYNAMIC_ALLOWED` entry keyed by file and template that says what bounds the value, and an entry the check no longer needs fails. Pass counts as numbers, and let a plural group pick the wording instead of switching keys on the count. +- **i18n** (`src/i18n/TranslationContext.tsx`): `en` is bundled; every other locale is a dynamic `import('@trek/shared/i18n/')` so Vite emits one chunk per locale. Strings live in `shared/`, never here. `t()` returns an unknown key unchanged, so `lint:i18n-keys` (`scripts/i18n-keys.mjs`) resolves every literal key in `t()`, `tHtml()`, `tr()`, ``, `translateApiError` and `*Key` props against `shared/src/i18n/en`; a template whose interpolations only choose between string literals is expanded and each branch checked as a key; any other template key is read with each interpolation as one key segment and passes by itself only when it reaches between 1 and `MAX_IMPLICIT_MATCHES` (30) en keys; a wider one, or one with no fixed dotted prefix, needs a `DYNAMIC_ALLOWED` entry keyed by file and template that says what bounds the value, and an entry the check no longer needs fails. Pass counts as numbers, and let a plural group pick the wording instead of switching keys on the count. - **Mobile shell** (`src/mobile/`): below the phone breakpoint (`useIsPhone`) `App.tsx` wraps routes in `MobileShell` and the `M*` screens under `mobile/screens/` take over. A UI change to a domain with an `M*` twin usually needs both — and the shared-hook rule below. - **Plugins UI** (`src/components/Plugins/`): third-party surfaces render only inside the sandboxed `PluginFrame` iframe (postMessage bridge, live theme-token sync). Host-rendered contribution points (widgets, schedule rows, map layers, planner columns/actions) render server-normalized primitives only — plugin markup never runs inline. Authoring is covered by the `trek-plugin-dev` skill. - **Managed installs** (`src/managed/index.tsx`): the attachment point for screens that only exist on a centrally administered install. Empty here by design — an operator replaces it at build time. Don't put features there. diff --git a/client/scripts/i18n-keys.mjs b/client/scripts/i18n-keys.mjs index 820eebcbb..8724b7c4a 100644 --- a/client/scripts/i18n-keys.mjs +++ b/client/scripts/i18n-keys.mjs @@ -20,7 +20,11 @@ * named *Key, read as a template key * * A key exists when en declares it, or declares `key.other` (a plural group - * whose general form is spelled out). A template literal key such as + * whose general form is spelled out). A template whose interpolations can + * only produce string literals (t(`roadtrip.window.${a ? 'onRoute' : 'atStop'}`)) + * is expanded into those keys and each is checked like a literal one; a + * literal fallback beside data (`${role ?? 'editor'}`) is checked as a key + * too. Any other template literal key such as * t(`budget.category.${id}`), or a prefix with the rest appended * (t('costs.filter.' + f)), cannot be resolved. It is read as a pattern in * which each interpolation stands for one key segment (no dot), and it passes @@ -30,8 +34,11 @@ * (`settings.${mode}` reaches every settings key, so a wrong suffix would * pass unseen) and one without a fixed dotted prefix need an entry in * DYNAMIC_ALLOWED, keyed by file and template, saying what bounds the value. - * Listing every narrow template instead would be a hundred entries restating - * their own prefix; the bound is what keeps those honest. + * Listing every narrow template instead would be well over a hundred entries + * restating their own prefix, and nothing in an entry would be checked that + * the bound does not already check. What the bound cannot see is a runtime + * value with no key under a narrow prefix: that needs the value's type, which + * this text scan does not have, and is the price of not listing them. * A key held in a variable (`t(opt.label)`) is out of reach unless the table * names it in a *Key property. * @@ -198,6 +205,126 @@ export function templatePattern(template) { return new RegExp(`^${parts.map(escape).join('[^.]+')}$`) } +/** + * The top-level operator positions of an expression: ternary `?` and `:`, + * and the `??` and `||` fallbacks. Strings and brackets are skipped as units, + * `?.` is optional chaining. + */ +function topLevelOperators(expr) { + const ops = [] + let depth = 0 + for (let i = 0; i < expr.length; i++) { + const c = expr[i] + if (c === "'" || c === '"' || c === '`') { + for (i++; i < expr.length && expr[i] !== c; i++) if (expr[i] === '\\') i++ + } else if (c === '(' || c === '[' || c === '{') depth++ + else if (c === ')' || c === ']' || c === '}') depth-- + else if (depth > 0) continue + else if ((c === '?' && expr[i + 1] === '?') || (c === '|' && expr[i + 1] === '|')) ops.push({ op: c + c, at: i++ }) + else if (c === '?' && expr[i + 1] !== '.') ops.push({ op: '?', at: i }) + else if (c === ':') ops.push({ op: ':', at: i }) + } + return ops +} + +const QUOTED_RE = /^(['"])((?:\\.|(?!\1)[^\\\n])*)\1$/ + +/** Whether `e` is one parenthesized expression, its first `(` closing at its last character. */ +function wrapsWhole(e) { + if (!e.startsWith('(') || !e.endsWith(')')) return false + let depth = 0 + for (let i = 0; i < e.length; i++) { + const c = e[i] + if (c === "'" || c === '"' || c === '`') { + for (i++; i < e.length && e[i] !== c; i++) if (e[i] === '\\') i++ + } else if (c === '(') depth++ + else if (c === ')' && --depth === 0) return i === e.length - 1 + } + return false +} + +/** + * The string literals an interpolated expression can produce, and whether + * those are all it can produce. `x ? 'a' : y ? 'b' : 'c'` gives a, b and c, + * complete; `role ?? 'editor'` gives editor, incomplete (role is data); + * `phase` gives nothing. A literal in a condition (`phase === 'start'`) is + * never a result. + */ +export function interpolationResults(expr) { + let e = expr.trim() + while (wrapsWhole(e)) e = e.slice(1, -1).trim() + const ops = topLevelOperators(e) + const q = ops.findIndex((o) => o.op === '?') + if (q >= 0) { + // The matching `:` of the first `?`: nested ternaries in the consequent raise the count. + let open = 0 + for (const o of ops.slice(q + 1)) { + if (o.op === '?') open++ + else if (o.op === ':' && open-- === 0) { + const a = interpolationResults(e.slice(ops[q].at + 1, o.at)) + const b = interpolationResults(e.slice(o.at + 1)) + return { literals: [...a.literals, ...b.literals], complete: a.complete && b.complete } + } + } + return { literals: [], complete: false } + } + const fallback = ops.filter((o) => o.op === '??' || o.op === '||') + if (fallback.length) { + const operands = [] + let from = 0 + for (const o of fallback) { + operands.push(e.slice(from, o.at)) + from = o.at + 2 + } + operands.push(e.slice(from)) + // The left operands are tested values; only the literal ones among them could be produced. + const literals = operands.flatMap((x) => interpolationResults(x).literals) + return { literals, complete: false } + } + const m = QUOTED_RE.exec(e) + return m ? { literals: [m[2]], complete: true } : { literals: [], complete: false } +} + +/** How many keys a template with only literal choices may expand to before it is read as a pattern instead. */ +const MAX_EXPANSION = 64 + +/** + * The references one template literal makes. Interpolations that can only + * produce string literals (`${open ? 'a' : 'b'}`) are expanded, so each + * branch is checked as a literal key and a typo in one fails. When some + * interpolation is data, the template stays a pattern, and each literal an + * interpolation can still produce (`${role ?? 'editor'}`) is checked with + * that interpolation fixed, under the original template's allow-list site. + */ +function templateRefs(file, line, template) { + const parts = template.split(/\$\{([^}]*)\}/) + const fixed = parts.filter((_, i) => i % 2 === 0) + const results = parts.filter((_, i) => i % 2 === 1).map(interpolationResults) + const literal = [] + const dynamic = [] + const build = (choices) => fixed.map((f, i) => f + (i < choices.length ? choices[i] : '')).join('') + const size = results.reduce((n, r) => n * Math.max(r.literals.length, 1), 1) + if (results.every((r) => r.complete) && size <= MAX_EXPANSION) { + let keys = [''] + results.forEach((r, i) => { + keys = keys.flatMap((k) => r.literals.map((lit) => k + fixed[i] + lit)) + }) + for (const k of keys) literal.push({ file, line, key: k + fixed[fixed.length - 1] }) + return { literal, dynamic } + } + dynamic.push({ file, line, template, pattern: templatePattern(template) }) + const raw = parts.filter((_, i) => i % 2 === 1).map((x) => `\${${x}}`) + results.forEach((r, i) => { + for (const lit of r.literals) { + const choices = raw.map((x, j) => (j === i ? lit : x)) + const derived = build(choices) + if (!derived.includes('${')) literal.push({ file, line, key: derived }) + else dynamic.push({ file, line, template: derived, pattern: templatePattern(derived), site: template }) + } + }) + return { literal, dynamic } +} + /** Every key reference in one file's source. */ export function scanSource(text, file = '') { const literal = [] @@ -216,7 +343,9 @@ export function scanSource(text, file = '') { if (KEY_RE.test(template)) literal.push({ file, line, key: template }) continue } - dynamic.push({ file, line, template, pattern: templatePattern(template) }) + const refs = templateRefs(file, line, template) + literal.push(...refs.literal) + dynamic.push(...refs.dynamic) } } for (const m of text.matchAll(KEY_BUILDER_RE)) { @@ -274,11 +403,11 @@ export function evaluate(scan, enKeys, allowed = DYNAMIC_ALLOWED) { const broad = [] for (const d of scan.dynamic) { const n = reach(d.template, d.pattern) - const isAllowed = allowedSites.has(site(d.file, d.template)) + const isAllowed = allowedSites.has(site(d.file, d.site ?? d.template)) if (n === 0 || (n === null && !isAllowed)) unmatched.push(d) else if (n > MAX_IMPLICIT_MATCHES && !isAllowed) broad.push({ ...d, matches: n }) } - const usedSites = new Set(scan.dynamic.map((d) => site(d.file, d.template))) + const usedSites = new Set(scan.dynamic.map((d) => site(d.file, d.site ?? d.template))) const stale = allowed.filter((a) => { if (!usedSites.has(site(a.file, a.template))) return true if ((a.resolves ?? []).some((k) => !hasKey(enKeys, k))) return true diff --git a/client/tests/unit/i18n/i18nKeysScript.test.ts b/client/tests/unit/i18n/i18nKeysScript.test.ts index 2cd17348c..13aabd7b4 100644 --- a/client/tests/unit/i18n/i18nKeysScript.test.ts +++ b/client/tests/unit/i18n/i18nKeysScript.test.ts @@ -8,13 +8,14 @@ import { countMatches, evaluate, firstArgument, + interpolationResults, readEnKeys, scanSource, scanTree, templatePattern, } from '../../../scripts/i18n-keys.mjs' -// FE-I18N-KEYS-001 to FE-I18N-KEYS-013: the client key check (scripts/i18n-keys.mjs). +// FE-I18N-KEYS-001 to FE-I18N-KEYS-015: the client key check (scripts/i18n-keys.mjs). const EN = new Set(['budget.title', 'places.count', 'places.count.one', 'trips.total.other', 'trips.total.one', 'costs.filter.all']) @@ -142,4 +143,24 @@ describe('i18n key check', () => { const gone = [{ file: 'a.tsx', template: 'gone.prefix.${x}', because: 'test' }] expect(evaluate(scan, EN, gone).unmatched.map((u) => u.template)).toEqual(['gone.prefix.${x}']) }) + + it('FE-I18N-KEYS-014: checks each literal branch of a template as a key, so a typo in one fails', () => { + const scan = scanSource("t(`costs.filter.${on ? 'all' : 'alll'}`)", 'a.tsx') + expect(scan.dynamic).toEqual([]) + expect(scan.literal.map((l) => l.key)).toEqual(['costs.filter.all', 'costs.filter.alll']) + expect(evaluate(scan, EN, []).missing.map((m) => m.key)).toEqual(['costs.filter.alll']) + // Nested ternaries, parentheses and optional chaining in the condition; a literal in a comparison is no result. + expect(interpolationResults("(a?.b === 'x') ? 'p' : c ? ('q') : 'r'")).toEqual({ literals: ['p', 'q', 'r'], complete: true }) + expect(interpolationResults("fn('a.b')")).toEqual({ literals: [], complete: false }) + }) + + it('FE-I18N-KEYS-015: keeps a template with data in it a pattern and still checks its literal fallback', () => { + const scan = scanSource("t(`costs.filter.${role ?? 'nope'}`); t(`budget.${p === 'x' ? 'title' : p}`)", 'a.tsx') + expect(scan.dynamic.map((d) => d.template)).toEqual(["costs.filter.${role ?? 'nope'}", "budget.${p === 'x' ? 'title' : p}"]) + expect(scan.literal.map((l) => l.key)).toEqual(['costs.filter.nope', 'budget.title']) + expect(evaluate(scan, EN, []).missing.map((m) => m.key)).toEqual(['costs.filter.nope']) + // A derived pattern is judged under its source template's allow-list entry. + const two = scanSource("t(`wide.${a ?? 'k0'}.${b}`)", 'a.tsx') + expect(two.dynamic.map((d) => d.site)).toEqual([undefined, "wide.${a ?? 'k0'}.${b}"]) + }) })