diff --git a/client/CLAUDE.md b/client/CLAUDE.md index 23e0a4313..ccff8474f 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 passes when its fixed parts match an en key, or through `DYNAMIC_ALLOWED` with a reason. 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 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 a1bbfcfe1..820eebcbb 100644 --- a/client/scripts/i18n-keys.mjs +++ b/client/scripts/i18n-keys.mjs @@ -22,8 +22,16 @@ * 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 * t(`budget.category.${id}`), or a prefix with the rest appended - * (t('costs.filter.' + f)), cannot be resolved; it passes when its fixed - * parts match at least one en key, and otherwise only through DYNAMIC_ALLOWED. + * (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 + * on its own only when that pattern is narrow: it matches at least one en key + * and at most MAX_IMPLICIT_MATCHES of them, the size of an enum such as the + * docsync error codes. A pattern that matches nothing fails. A wider one + * (`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. * A key held in a variable (`t(opt.label)`) is out of reach unless the table * names it in a *Key property. * @@ -44,19 +52,77 @@ const HERE = dirname(fileURLToPath(import.meta.url)) const SRC = join(HERE, '..', 'src') const SHARED_SCRIPTS = join(HERE, '..', '..', 'shared', 'scripts') +/** How many en keys a template may reach and still pass without an entry below. */ +export const MAX_IMPLICIT_MATCHES = 30 + /** - * Template literal keys whose fixed parts match no en key, each with the - * reason it still resolves and the keys it resolves to, which must exist. - * Keep this short: every entry is a key the check cannot follow. + * Template keys the check cannot bound by itself: wider than + * MAX_IMPLICIT_MATCHES, or without a fixed dotted prefix. Each names the file + * it sits in and why the interpolated value can only be a real key; one with + * no fixed prefix also lists the keys it resolves to, which must exist. An + * entry nothing uses any more fails, so the list cannot rot. */ export const DYNAMIC_ALLOWED = [ { + file: 'components/Trips/TripShareDialog.tsx', template: '${opt.label}Hint', because: 'TripShareDialog appends Hint to each SHARE_OPTIONS label (useTripShare.ts); share.optTravelOnlyHint ' + 'and share.optHideImagesHint exist, and the template has no fixed part to match them by', resolves: ['share.optTravelOnlyHint', 'share.optHideImagesHint'], }, + ...[ + 'help.ctx.${id}.${part}', + 'help.ctx.${id}.bullet.${n}', + 'help.guide.${id}.${part}', + 'help.guide.${id}.step.${n}', + 'help.guide.${id}.tip.${n}', + ].map((template) => ({ + file: 'help/registry.ts', + template, + because: + 'a key builder over the registered help contexts and guides; src/help/registry.test.ts resolves every ' + + 'key of every registered entry against en', + })), + { + file: 'mobile/screens/dashboard/MUserMenu.tsx', + template: 'settings.${mode}', + because: "mode is a ThemeMode from resolveMode(), 'light' | 'dark' | 'auto', and settings.light/dark/auto exist", + }, + ...['components/Studio/StudioElementsPanel.tsx', 'components/Studio/StudioInspector.tsx'].map((file) => ({ + file, + template: 'journey.studio.${key}', + because: 'key comes from an `as const` tuple of frame and stroke styles written beside the call, typed literals', + })), + ...['mobile/screens/trip/sheets/MReservationSheet.tsx', 'mobile/screens/trip/sheets/MTransportFormSheet.tsx'].map( + (file) => ({ + file, + template: 'reservations.${s}', + because: "s iterates the literal ['pending', 'confirmed'] as const; reservations.pending/confirmed exist", + }), + ), + ...['components/Admin/AdminPluginsPanel.tsx', 'mobile/screens/admin/MAdminPluginsPanel.tsx'].map((file) => ({ + file, + template: 'admin.plugins.perm.${perm}', + because: + 'PermLabel translates a permission only when PERM_KEYS (generated PLUGIN_PERMISSIONS) holds it and shows ' + + 'the raw code otherwise; the plugin-facts gate keeps the permission keys in step with en', + })), + ...['components/Settings/PhotoProvidersSection.tsx', 'mobile/screens/settings/MPhotoProvidersSection.tsx'].flatMap( + (file) => + ['memories.${field.label}', 'memories.${field.hint}'].map((template) => ({ + file, + template, + because: + 'field label and hint names come from the provider field rows PhotoProviderSeeder writes, each a ' + + 'memories.* key in en', + })), + ), + { + file: 'components/Tours/planner/TourPlannerPanels.tsx', + template: 'tours.planner.${role}', + because: "roleLabel takes role: 'start' | 'via' | 'end', and tours.planner.start/via/end exist", + }, ] const KEY_RE = /^[a-z][a-zA-Z0-9_]*(?:\.[a-zA-Z0-9_:-]+)+$/ @@ -121,12 +187,15 @@ export function literalsIn(arg) { return { quoted, templates } } -/** A template literal's key pattern: its fixed parts, anything in between. Null when it has no fixed dotted prefix. */ +/** + * A template literal's key pattern: its fixed parts, one key segment (no dot) + * for each interpolation. Null when it has no fixed dotted prefix. + */ export function templatePattern(template) { const parts = template.split(/\$\{[^}]*\}/) if (!/^[a-z][a-zA-Z0-9_]*\./.test(parts[0] ?? '')) return null const escape = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') - return new RegExp(`^${parts.map(escape).join('.+')}$`) + return new RegExp(`^${parts.map(escape).join('[^.]+')}$`) } /** Every key reference in one file's source. */ @@ -174,27 +243,55 @@ export async function readEnKeys() { export const hasKey = (enKeys, key) => enKeys.has(key) || enKeys.has(`${key}.other`) +const base = (k) => k.replace(/\.(?:zero|one|two|few|many|other)$/, '') + +/** How many en keys a pattern reaches, a plural group counted once. */ +export function countMatches(pattern, enKeys) { + const reached = new Set() + for (const k of enKeys) { + if (pattern.test(k)) reached.add(k) + else if (pattern.test(base(k))) reached.add(base(k)) + } + return reached.size +} + /** * The verdict over a scan: literal keys en lacks, template keys no en key - * matches (unless allowed), allow-list entries nothing uses any more, and - * the en keys nothing reaches. + * matches, templates too wide to pass without an allow-list entry, + * allow-list entries that no file uses any more or that the check no longer + * needs, and the en keys nothing reaches. */ export function evaluate(scan, enKeys, allowed = DYNAMIC_ALLOWED) { const missing = scan.literal.filter(({ key }) => !hasKey(enKeys, key)) - const allowedTemplates = new Set(allowed.map((a) => a.template)) - const unmatched = scan.dynamic.filter( - ({ template, pattern }) => - !allowedTemplates.has(template) && !(pattern && [...enKeys].some((k) => pattern.test(k))), - ) - const usedTemplates = new Set(scan.dynamic.map((d) => d.template)) - const stale = allowed.filter((a) => !usedTemplates.has(a.template) || a.resolves.some((k) => !hasKey(enKeys, k))) - const reached = new Set([...scan.literal.map((l) => l.key), ...allowed.flatMap((a) => a.resolves)]) + const site = (file, template) => `${file}\n${template}` + const allowedSites = new Set(allowed.map((a) => site(a.file, a.template))) + const counts = new Map() + const reach = (template, pattern) => { + if (!counts.has(template)) counts.set(template, pattern ? countMatches(pattern, enKeys) : null) + return counts.get(template) + } + const unmatched = [] + const broad = [] + for (const d of scan.dynamic) { + const n = reach(d.template, d.pattern) + const isAllowed = allowedSites.has(site(d.file, 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 stale = allowed.filter((a) => { + if (!usedSites.has(site(a.file, a.template))) return true + if ((a.resolves ?? []).some((k) => !hasKey(enKeys, k))) return true + // An entry for a template narrow enough to pass by itself only lengthens the list. + const n = reach(a.template, templatePattern(a.template)) + return n !== null && n <= MAX_IMPLICIT_MATCHES + }) + const reached = new Set([...scan.literal.map((l) => l.key), ...allowed.flatMap((a) => a.resolves ?? [])]) const patterns = scan.dynamic.map((d) => d.pattern).filter(Boolean) - const base = (k) => k.replace(/\.(?:zero|one|two|few|many|other)$/, '') const unused = [...enKeys].filter( (k) => !reached.has(k) && !reached.has(base(k)) && !patterns.some((p) => p.test(k) || p.test(base(k))), ) - return { missing, unmatched, stale, unused } + return { missing, unmatched, broad, stale, unused } } export function scanTree(root = SRC) { @@ -216,15 +313,24 @@ if (isCli) { try { const scan = scanTree() const enKeys = await readEnKeys() - const { missing, unmatched, stale, unused } = evaluate(scan, enKeys) + const { missing, unmatched, broad, stale, unused } = evaluate(scan, enKeys) for (const { file, line, key } of missing) { console.error(`FAIL ${file}:${line}: '${key}' is not a key in shared/src/i18n/en`) } for (const { file, line, template } of unmatched) { console.error(`FAIL ${file}:${line}: \`${template}\` matches no key in shared/src/i18n/en`) } - for (const { template } of stale) { - console.error(`FAIL DYNAMIC_ALLOWED lists \`${template}\`, which no file uses any more: remove the entry`) + for (const { file, line, template, matches } of broad) { + console.error( + `FAIL ${file}:${line}: \`${template}\` reaches ${matches} en keys, more than ${MAX_IMPLICIT_MATCHES}: ` + + 'narrow the prefix, or add a DYNAMIC_ALLOWED entry saying what bounds the value', + ) + } + for (const { file, template } of stale) { + console.error( + `FAIL DYNAMIC_ALLOWED lists \`${template}\` in ${file}, which that file no longer uses, whose keys en ` + + 'lacks, or which is narrow enough to pass without it: remove or fix the entry', + ) } if (process.argv.includes('--unused')) for (const key of unused.sort()) console.log(`unused ${key}`) console.log( @@ -232,7 +338,7 @@ if (isCli) { `reference(s) checked against ${enKeys.size} en keys; ${unused.length} en key(s) reached by neither ` + '(information, --unused lists them)', ) - if (missing.length || unmatched.length || stale.length) process.exit(1) + if (missing.length || unmatched.length || broad.length || stale.length) process.exit(1) } catch (err) { console.error(`FAIL ${err.message}`) process.exit(1) diff --git a/client/tests/unit/i18n/i18nKeysScript.test.ts b/client/tests/unit/i18n/i18nKeysScript.test.ts index b240d3180..2cd17348c 100644 --- a/client/tests/unit/i18n/i18nKeysScript.test.ts +++ b/client/tests/unit/i18n/i18nKeysScript.test.ts @@ -4,6 +4,8 @@ import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { DYNAMIC_ALLOWED, + MAX_IMPLICIT_MATCHES, + countMatches, evaluate, firstArgument, readEnKeys, @@ -12,7 +14,7 @@ import { templatePattern, } from '../../../scripts/i18n-keys.mjs' -// FE-I18N-KEYS-001 to FE-I18N-KEYS-010: the client key check (scripts/i18n-keys.mjs). +// FE-I18N-KEYS-001 to FE-I18N-KEYS-013: 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']) @@ -52,6 +54,8 @@ describe('i18n key check', () => { expect(dynamic.map((d) => d.template)).toEqual(['costs.filter.${f}', 'costs.filter.${…}', 'help.guide.${id}.title']) expect(templatePattern('costs.filter.${f}')!.test('costs.filter.all')).toBe(true) expect(templatePattern('${opt.label}Hint')).toBeNull() + // An interpolation is one key segment: costs.filter.${f} does not reach a nested costs.filter.x.y. + expect(templatePattern('costs.filter.${f}')!.test('costs.filter.x.y')).toBe(false) }) it('FE-I18N-KEYS-004: fails a literal key en lacks and accepts a plural group by its general form', () => { @@ -63,7 +67,7 @@ describe('i18n key check', () => { it('FE-I18N-KEYS-005: fails a template no en key matches unless it is allowed with the keys it resolves to', () => { const scan = scanSource('t(`costs.filter.${f}`); t(`gone.prefix.${x}`); t(`${opt.label}Hint`)') expect(evaluate(scan, EN, []).unmatched.map((u) => u.template)).toEqual(['gone.prefix.${x}', '${opt.label}Hint']) - const allowed = [{ template: '${opt.label}Hint', because: 'test', resolves: ['budget.title'] }] + const allowed = [{ file: '', template: '${opt.label}Hint', because: 'test', resolves: ['budget.title'] }] const result = evaluate(scan, EN, allowed) expect(result.unmatched.map((u) => u.template)).toEqual(['gone.prefix.${x}']) expect(result.stale).toEqual([]) @@ -71,10 +75,13 @@ describe('i18n key check', () => { it('FE-I18N-KEYS-006: refuses an allow-list entry nothing uses or whose keys en lacks', () => { const scan = scanSource('t(`${opt.label}Hint`)') - const unused = [{ template: '${gone}', because: 'test', resolves: ['budget.title'] }] + const unused = [{ file: '', template: '${gone}', because: 'test', resolves: ['budget.title'] }] expect(evaluate(scan, EN, unused).stale).toEqual(unused) - const wrong = [{ template: '${opt.label}Hint', because: 'test', resolves: ['share.nope'] }] + const wrong = [{ file: '', template: '${opt.label}Hint', because: 'test', resolves: ['share.nope'] }] expect(evaluate(scan, EN, wrong).stale).toEqual(wrong) + const elsewhere = [{ file: 'other.tsx', template: '${opt.label}Hint', because: 'test', resolves: ['budget.title'] }] + expect(evaluate(scan, EN, elsewhere).stale).toEqual(elsewhere) + expect(evaluate(scan, EN, elsewhere).unmatched.map((u) => u.template)).toEqual(['${opt.label}Hint']) }) it('FE-I18N-KEYS-007: reports en keys neither a literal nor a pattern reaches', () => { @@ -82,10 +89,11 @@ describe('i18n key check', () => { expect(evaluate(scan, EN, []).unused).toEqual(['trips.total.other', 'trips.total.one']) }) - it('FE-I18N-KEYS-008: every allow-list entry carries a reason and the keys it resolves to', () => { + it('FE-I18N-KEYS-008: every allow-list entry names its file and a reason, and a prefix-less one its keys', () => { for (const entry of DYNAMIC_ALLOWED) { + expect(entry.file).toMatch(/\.tsx?$/) expect(entry.because.length).toBeGreaterThan(20) - expect(entry.resolves.length).toBeGreaterThan(0) + if (templatePattern(entry.template) === null) expect('resolves' in entry && entry.resolves.length).toBeGreaterThan(0) } }) @@ -101,9 +109,37 @@ describe('i18n key check', () => { it('FE-I18N-KEYS-010: the client source names no key en lacks today', async () => { const enKeys = await readEnKeys() - const { missing, unmatched, stale } = evaluate(scanTree(), enKeys) + const { missing, unmatched, broad, stale } = evaluate(scanTree(), enKeys) expect(missing).toEqual([]) expect(unmatched).toEqual([]) + expect(broad).toEqual([]) expect(stale).toEqual([]) }) + + // A table of en keys wider than the implicit bound: wide.k0 to wide.k. + const WIDE = new Set([...EN, ...Array.from({ length: MAX_IMPLICIT_MATCHES + 1 }, (_, i) => `wide.k${i}`)]) + + it('FE-I18N-KEYS-011: counts a plural group once when measuring what a template reaches', () => { + expect(countMatches(templatePattern('trips.${x}')!, EN)).toBe(1) + expect(countMatches(templatePattern('wide.${x}')!, WIDE)).toBe(MAX_IMPLICIT_MATCHES + 1) + }) + + it('FE-I18N-KEYS-012: fails a template wider than the bound unless its file has an entry for it', () => { + const scan = scanSource('t(`wide.${x}`); t(`costs.filter.${f}`)', 'a.tsx') + const { broad, unmatched } = evaluate(scan, WIDE, []) + expect(broad.map((b) => [b.template, b.matches])).toEqual([['wide.${x}', MAX_IMPLICIT_MATCHES + 1]]) + expect(unmatched).toEqual([]) + const otherFile = [{ file: 'b.tsx', template: 'wide.${x}', because: 'test' }] + expect(evaluate(scan, WIDE, otherFile).broad).toHaveLength(1) + const ownFile = [{ file: 'a.tsx', template: 'wide.${x}', because: 'test' }] + expect(evaluate(scan, WIDE, ownFile)).toMatchObject({ broad: [], stale: [] }) + }) + + it('FE-I18N-KEYS-013: refuses an entry for a narrow template, and an allowed template that matches nothing', () => { + const scan = scanSource('t(`costs.filter.${f}`); t(`gone.prefix.${x}`)', 'a.tsx') + const narrow = [{ file: 'a.tsx', template: 'costs.filter.${f}', because: 'test' }] + expect(evaluate(scan, EN, narrow).stale).toEqual(narrow) + 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}']) + }) })