feat(i18n): add per-instance language selection mechanism#1442
feat(i18n): add per-instance language selection mechanism#1442thomasbeaudry wants to merge 1 commit into
Conversation
Adds the mechanism for supporting additional UI languages (e.g. Spanish) without shipping any translations. Deployments choose which languages are active; the language toggle and instrument rendering derive from that set. - Store activeLanguages in SetupState (default ['en', 'fr']); expose via the setup API and admin settings, which autosaves language changes. - Augment libui's UserConfig.LanguageOptions with es in the web and gateway apps so the UI can resolve to Spanish; instrument content types are left at en|fr (partial translations are accepted), and sites that pass the resolved UI language into instrument APIs narrow it back to a supported language. - Derive Navbar/Sidebar language toggle options from activeLanguages and hide the toggle when only one language is active. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
30bab68 to
87ab4f0
Compare
gdevenyi
left a comment
There was a problem hiding this comment.
Review — per-instance language selection
The mechanism is the right shape: store the active set on SetupState, derive the toggle options from it, hide the toggle when only one language is active. Hiding rather than disabling the toggle is a nice touch, and disabling the last remaining checkbox is the correct guard.
Two things make this hard to accept as-is: the perimeter isn't validated, and the type widening was absorbed by casting at ~8 call sites rather than fixed once.
Blocking
-
activeLanguages: z.array(z.string())validates nothing.PATCH /v1/setup {"activeLanguages": ["klingon"]}is accepted and persisted.Object.entries(ALL_LANGUAGES).filter(([key]) => activeLanguages.includes(key))then yields{},Object.keys(...).length > 1is false, and the language toggle disappears from the navbar and sidebar for every user on the instance, with no way to restore it through the UI. AGENTS.md is explicit here — strict data validation at the boundary, and fail loudly on an undeclared policy. This should bez.array($Language).min(1)with$Language = z.enum(['en', 'es', 'fr'])exported as the single source of truth. -
The casts move the problem instead of solving it. Widening
Languageto includeesbroke indexing into the{ en, fr }objects in$BrandingConfigandGroupSettings, and the fix wasas undefined | { [key: string]: string }at four spots inLoginBrandingPanel, one inlogin.tsx, one inStartSessionForm, plusresolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en'repeated five times. AGENTS.md: "no casting at call sites", "shape is never repeated", "concentrate the cost". The actual defect is that those schema fields are typed{ en: string; fr: string }instead of being keyed byLanguage. Fixing them once inpackages/schemasmakes every one of these call sites type-check unmodified. (#1439 introduces a$LocalizedStringthat is close to what's needed — worth converging on one, inschemas/core.) -
No tests. AGENTS.md requires unit tests and e2e tests for new changes; this has neither, and both manual boxes in the test plan are unchecked. Minimum viable set: a
$UpdateSetupStateDataschema test for the accepted/rejected language arrays, aSetupServicetest thatactiveLanguagesround-trips (the pattern is right there in #1441'ssetup.service.spec.ts), and an e2e that deactivates a language and asserts the toggle disappears.
Should fix
-
A failed autosave still reports "All changes saved" —
onSettledruns on error. Same issue as #1441; see inline. -
Deactivating a language strands users who had it selected.
i18n.changeLanguageis called only for the admin flipping the checkbox. Every other user whose persisted language was just removed keeps rendering in it, and the toggle no longer offers a way out. Needs a reconciliation on app load: if the resolved language isn't inactiveLanguages, switch to the first active one. -
['en', 'fr']is hardcoded as the default in four places here (setup.service.ts,Navbar,Sidebar,settings.tsx) and five more in #1439. IfSetupServiceguarantees a non-empty array — which it already tries to — no client needs a fallback at all. -
The gateway carries a fifth hardcoded language list and doesn't consult
activeLanguages, so?lang=esworks there even when Spanish is deactivated instance-wide. -
ALL_LANGUAGES: { [key: string]: string }— a loose record over a closed key set, which AGENTS.md rules out and which is what forces the?? codefallbacks in #1439's consumers. -
The libui bump isn't here. The PR notes this depends on libui#107 for libui's own Spanish strings. Until that lands and is released, selecting Español leaves every libui-owned string (form submit/reset buttons, search placeholder, date pickers, notification titles) in English. Should this merge before the bump, or wait?
Coordination with the other open PRs
| Conflict | Files |
|---|---|
| #1442 × #1441 | apps/web/src/routes/_app/admin/settings.tsx (both replace the staged-Save block with incompatible autosave layouts — #1441 adds a Settings section and SettingSection wrappers, this adds a Languages card) and apps/web/src/components/SaveStatus.tsx (add/add) |
| #1442 × #1439 | apps/web/src/components/SaveStatus.tsx (add/add); apps/web/src/utils/languages.ts, activeLanguages in schema.prisma / setup.service.ts / $SetupState are duplicated verbatim between the two |
Where the two autosave implementations differ, #1441's is correct: it destructures mutate (referentially stable) and depends on [mutate], whereas this one depends on [updateSetupStateMutation], which React Query returns fresh every render.
| }); | ||
|
|
||
| const $SetupState = z.object({ | ||
| activeLanguages: z.array(z.string()).optional(), |
There was a problem hiding this comment.
z.array(z.string()) accepts any string, and this is the perimeter — $UpdateSetupStateData is what UpdateSetupStateDto validates the PATCH body against.
PATCH /v1/setup {"activeLanguages": ["klingon"]} is stored as-is. Navbar/Sidebar then compute Object.entries(ALL_LANGUAGES).filter(([key]) => activeLanguages.includes(key)) → {} → Object.keys(...).length > 1 is false → the language toggle vanishes for every user, and the admin settings page shows all three boxes unchecked with no way back through the UI. [] is likewise accepted here and only papered over by a ?.length check in the service.
AGENTS.md: "Strict data validation at the boundary, trusting inside" and "Correctness is structural, not vigilant".
const LANGUAGES = ['en', 'es', 'fr'] as const;
const $Language = z.enum(LANGUAGES);
const $ActiveLanguages = z.array($Language).min(1);Exporting LANGUAGES/$Language also gives ALL_LANGUAGES, the gateway's VALID_LANGUAGES, and #1439's MAIL_LANGUAGE one thing to derive from instead of four independent copies.
| (data: Parameters<typeof updateSetupStateMutation.mutate>[0]) => { | ||
| setSaveState('saving'); | ||
| updateSetupStateMutation.mutate(data, { | ||
| onSettled: () => { |
There was a problem hiding this comment.
onSettled runs on failure too, so a rejected save still shows the green check and "All changes saved". Because useUpdateSetupStateMutation doesn't disable the default error notification, the user gets a red error toast and a green "saved" pill simultaneously — and for the language checkboxes specifically, the box stays flipped even though nothing was stored.
onSuccess for the saved state, onError for a visible failure (and revert the optimistic checkbox).
Same issue in #1441, which rewrites this same block.
| }; | ||
| }); | ||
| }, | ||
| [updateSetupStateMutation] |
There was a problem hiding this comment.
React Query returns a new mutation result object on every render — only mutate/mutateAsync are referentially stable. Depending on [updateSetupStateMutation] means this useCallback re-creates autosave every render, so the memo does nothing, and it becomes a real bug the moment autosave is used in an effect dependency array.
#1441 gets this right in the same file:
// `mutate` is referentially stable across renders, so callbacks that depend on `autosave` stay stable too.
const { mutate } = updateSetupStateMutation;
const autosave = useCallback((data) => { ... }, [mutate]);Worth adopting that version wholesale when these two branches are reconciled.
| : activeLanguages.filter((l) => l !== code); | ||
| if (updated.length > 0) { | ||
| autosave({ activeLanguages: updated }); | ||
| if (!updated.includes(i18n.resolvedLanguage)) { |
There was a problem hiding this comment.
Two problems with switching the language here.
It only moves the admin. Every other user whose persisted language was just deactivated keeps rendering in it — t() still resolves their strings, and the toggle no longer lists it, so they can't switch away either. There's no reconciliation anywhere on app load. Something like this in the _app route (or wherever setupStateQuery is first read) would close it:
if (!activeLanguages.includes(i18n.resolvedLanguage)) i18n.changeLanguage(activeLanguages[0]);updated[0] is arbitrary. [...activeLanguages, code] appends, so the array's order reflects the sequence of toggles, not any canonical order. Deactivating English can drop the admin into Spanish or French depending on click history. Sorting against the canonical LANGUAGES tuple (or just preferring 'en' when present) makes it deterministic.
Minor: the if (updated.length > 0) guard silently does nothing when it fails. The disabled={isLastActive} prop already prevents that path, so the guard is either dead or masking a case where it isn't — worth deciding which.
| // older $BrandingConfig will silently drop newer branding fields on read. | ||
| const branding = $BrandingConfig.nullable().safeParse(savedOptions?.branding ?? null); | ||
| return { | ||
| activeLanguages: savedOptions?.activeLanguages?.length ? savedOptions.activeLanguages : ['en', 'fr'], |
There was a problem hiding this comment.
This is the first of four copies of ['en', 'fr'] in this PR (also Navbar, Sidebar, settings.tsx) and #1439 adds five more. AGENTS.md: "One source of truth; everything derived."
Export DEFAULT_ACTIVE_LANGUAGES from schemas/setup and use it here. Then, since this method already guarantees a non-empty array, $SetupState.activeLanguages can drop .optional() and every client-side ?? ['en', 'fr'] disappears — the clients stop needing to know the default exists.
| createdAt DateTime @default(now()) @db.Date | ||
| updatedAt DateTime @updatedAt @db.Date | ||
| id String @id @default(auto()) @map("_id") @db.ObjectId | ||
| activeLanguages String[] |
There was a problem hiding this comment.
A required scalar list added to a model with existing documents. #1441 adds its SetupState field as Int?; this one has neither ? (not permitted on lists) nor @default([]).
Prisma's MongoDB connector should return [] for a list field absent from a stored document, so this probably works — but "probably" is doing real work in a field that gates the language toggle for the whole instance. @default([]) costs nothing and removes the question for every instance created before this deploys.
Also worth noting initApp creates SetupState without this field, so the very first document an instance writes exercises exactly that path.
| } | ||
| } | ||
|
|
||
| const VALID_LANGUAGES = ['en', 'es', 'fr']; |
There was a problem hiding this comment.
Third hardcoded language list in this PR (after ALL_LANGUAGES and LanguageOptions), and #1439 adds a fourth (MAIL_LANGUAGE). Import the tuple from schemas and derive.
Two behavioural notes:
-
The gateway ignores
activeLanguages.?lang=esrenders Spanish even on an instance where Spanish has been deactivated. Since the emailed assignment links in feat(mail): add outgoing email, group email templates, and assignment emails #1439 are built as${assignment.url}?lang=${language}, that's reachable in practice. -
SSR/hydration.
detectLanguage()readswindow.location.searchat module scope; the guard makes the server always resolve'en'while the client can resolve'es'. Worth confirming the first paint doesn't flash English or trip a hydration mismatch — a?lang=link is precisely the case where the two disagree.
Minor: the two /* eslint-disable */ lines at the top disable those rules for the whole file rather than the declare module block that needs them.
| setInstrument( | ||
| translateInstrument( | ||
| instrument, | ||
| resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en' |
There was a problem hiding this comment.
resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en' appears five times in this PR — here, useInstrumentInfoQuery, group/manage.tsx, InteractiveContent, and useInterpretedInstrument. AGENTS.md: "Shape is never repeated."
One named helper says what it means and gives you a single place to change the policy:
/** Instruments are authored in en/fr only; other UI languages fall back to English. */
export const toInstrumentLanguage = (language: Language): 'en' | 'fr' =>
language === 'fr' ? 'fr' : 'en';The policy itself is also worth surfacing: a user who has selected Español gets instruments silently rendered in English with no indication that a translation is missing. That's defensible, but right now it's an implicit consequence of five scattered ternaries rather than a stated decision.
| const instanceDetails = branding?.instanceDetails?.[lang]?.trim() || null; | ||
| /* eslint-disable @typescript-eslint/prefer-nullish-coalescing -- blank strings must fall back, so `||` not `??` */ | ||
| const instanceName = | ||
| (branding?.instanceName as undefined | { [key: string]: string })?.[lang]?.trim() || DEFAULT_INSTANCE_NAME; |
There was a problem hiding this comment.
These casts (four in this file, plus login.tsx and StartSessionForm.tsx) are the symptom, not the fix. AGENTS.md: "no casting at call sites" and "Concentrate the cost: complex type machinery belongs in a small number of utilities."
The root cause is that $BrandingConfig types instanceName/instanceTagline/instanceDetails and resourceLinks[].label as literal { en, fr } objects, so they can't be indexed once Language widens. Keying them by Language in packages/schemas fixes all six sites without touching them:
const $LocalizedString = z.partialRecord($Language, z.string().nullish());That also makes tl = (obj) => obj[lang] ?? obj.en ?? '' unnecessary and — importantly — makes adding a fourth language a compile-time task rather than a runtime fallback to ''. Today the cast means a missing language silently renders as an empty instance name.
Note #1439 adds a $LocalizedString in schemas/mail for the same reason; one shared definition in schemas/core would serve both PRs.
|
|
||
| const handleChangeLanguageEvent = useCallback( | ||
| (event: CustomEvent<Language>) => { | ||
| (event: CustomEvent<string>) => { |
There was a problem hiding this comment.
Widening a react-core event contract from CustomEvent<Language> to CustomEvent<string> to work around a downstream type error loses the checking for every other consumer of the changeLanguage event — including instrument authors, since this is the interactive-task bridge.
The runtime guard on the next line already narrows to 'en' | 'fr', so the type can stay Language and the guard keeps doing its job. If the incompatibility is with document.addEventListener's global CustomEventMap (which is where the widening pressure comes from), that map is the thing to update — not this handler's signature.
While here: console.error on an unsupported language is invisible to the participant, who just sees the language not change. Given AGENTS.md's "fail loudly on an undeclared policy", a user-visible signal (or not offering the option at all) would be better.
Summary
esto theLanguagetype inruntime-coreand libuiUserConfig.LanguageOptionsactiveLanguagestoSetupStateschema, Prisma model, and API serviceLanguageToggleoptions fromactiveLanguagesin Navbar and Sidebar (hide toggle when only one language is active)?lang=query paramNo translated strings are included — missing translations fall back to
defaultLanguage(English).Depends on DouglasNeuroInformatics/libui#107 for libui's own Spanish translations. The libui version bump should be done when that PR is merged and released.
Test plan
?lang=esparam🤖 Generated with Claude Code