From 1ebc98bce6e569fa8219c49983a12829036f66f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 10:59:48 +0000 Subject: [PATCH] fix(i18n): fail on an inline defaultValue that contradicts its en value (#3810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `t(key, { defaultValue: 'English' })` renders that text only when i18next misses the key. Where `en` defines the key the pack value always wins, so the inline string is dead code — and 43 of them said something else than the sentence users read, which misleads every later reader and becomes the visible copy the day the key is renamed. `scripts/check-i18n-call-site-keys.mjs` (#3530) already parsed both halves; it now compares them and fails on any byte of difference. Hard rule, no baseline: the repo-wide census found 43 sites in 19 files out of 851 literal inline defaults, and all 43 are aligned to the pack in this commit, so there is no debt for a ratchet to hold. A defaultValue on a key `en` does NOT define stays legal and stays the `missing-key` rule's business alone — the two classes are disjoint by construction and pinned that way. Every alignment moved the call site, never `en.ts`: pack values are what users read, and editing one would oblige the same edit in nine other packs (check-i18n-en-drift.mjs, #3650). Six of the 43 differed only in an ellipsis (`...` vs U+2026). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../inline-default-value-drift-gate-3810.md | 39 ++++ .../src/pages/auth/ForgotPasswordPage.tsx | 8 +- apps/console/src/pages/auth/LoginPage.tsx | 4 +- apps/console/src/pages/auth/RegisterPage.tsx | 10 +- .../src/pages/auth/VerifyEmailPromptPage.tsx | 4 +- .../app-shell/src/console/home/HomeLayout.tsx | 2 +- .../app-shell/src/console/home/HomePage.tsx | 8 +- .../app-shell/src/console/home/HomeRail.tsx | 2 +- .../organizations/OrganizationsPage.tsx | 18 +- packages/app-shell/src/layout/AppHeader.tsx | 2 +- packages/app-shell/src/layout/AppSidebar.tsx | 2 +- packages/app-shell/src/layout/ChatDock.tsx | 2 +- .../app-shell/src/layout/InboxPopover.tsx | 2 +- .../app-shell/src/views/InterfaceListPage.tsx | 2 +- .../app-shell/src/views/RecordDetailView.tsx | 4 +- .../app-shell/src/views/ReportConfigPanel.tsx | 4 +- packages/app-shell/src/views/ReportView.tsx | 2 +- packages/plugin-detail/src/DetailView.tsx | 2 +- .../src/renderers/record-reference-rail.tsx | 4 +- .../src/components/ViewSettingsPopover.tsx | 4 +- .../check-i18n-call-site-keys.test.ts | 153 ++++++++++++- scripts/check-i18n-call-site-keys.mjs | 215 ++++++++++++++++-- scripts/check-i18n-en-drift.mjs | 7 +- 23 files changed, 429 insertions(+), 71 deletions(-) create mode 100644 .changeset/inline-default-value-drift-gate-3810.md diff --git a/.changeset/inline-default-value-drift-gate-3810.md b/.changeset/inline-default-value-drift-gate-3810.md new file mode 100644 index 0000000000..d6951b2f18 --- /dev/null +++ b/.changeset/inline-default-value-drift-gate-3810.md @@ -0,0 +1,39 @@ +--- +"@object-ui/app-shell": patch +"@object-ui/plugin-detail": patch +"@object-ui/plugin-list": patch +"@object-ui/console": patch +--- + +Align 43 inline `defaultValue` strings with the `en` pack, and make the call-site gate enforce it (objectui#3810) + +`t(key, { defaultValue: 'English text' })` only renders that text when i18next +**misses** the key. Where the key exists in `packages/i18n/src/locales/en.ts` the +pack value always wins, so the inline string is dead code — and 43 of those dead +strings said something different from the sentence users actually read. + +`scripts/check-i18n-call-site-keys.mjs` (objectui#3530) now compares the two +whenever a call site carries a literal `defaultValue` for a key `en` defines, and +fails on any byte of difference. It is a hard rule with **no baseline**: the +repo-wide census measured 43 sites in 19 files out of 851 literal inline defaults, +and all 43 are aligned here, so there is no debt for a ratchet to hold. A +`defaultValue` on a key that is *not* yet in `en` stays legal — that transition +runs for months (objectui#3546) and belongs to the existing `missing-key` rule, +which keeps reporting it alone. + +Every fix moved the CALL SITE to the pack's wording. `en.ts` is untouched: its +values are what users read today, and changing one would oblige the same change in +the nine other packs (`scripts/check-i18n-en-drift.mjs`, objectui#3650). Six of the +43 differed only in an ellipsis (`...` against U+2026) — invisible in review, which +is how they survived three i18n gates that are each blind to this class by +construction. + +The visible effect is confined to hosts that render these components with **no** +`I18nProvider` and no initialised i18next instance. There, react-i18next's +not-ready `t` returns the `defaultValue`, so the inline string was the rendered +one; it now matches what a provider-backed app has always shown. Inside the +console — provider mounted — nothing users see changes. The clearest converging +examples: the workspaces screen was written as "Organizations" at nine call sites +while every user has been reading "Workspaces"; the forgot-password success line +was written as "If an account exists, a reset link has been sent." while the pack +asserts "We've sent a password reset link to {{email}}." diff --git a/apps/console/src/pages/auth/ForgotPasswordPage.tsx b/apps/console/src/pages/auth/ForgotPasswordPage.tsx index 6a243c90d5..8ca4083d5a 100644 --- a/apps/console/src/pages/auth/ForgotPasswordPage.tsx +++ b/apps/console/src/pages/auth/ForgotPasswordPage.tsx @@ -31,20 +31,20 @@ export function ForgotPasswordPage() { linkComponent={RouterLink} title={t('auth.forgotPassword.title', { defaultValue: 'Reset your password' })} description={t('auth.forgotPassword.description', { - defaultValue: "Enter your email and we'll send you a reset link.", + defaultValue: "Enter your email address and we'll send you a link to reset your password", })} labels={{ emailLabel: t('auth.forgotPassword.emailLabel', { defaultValue: 'Email' }), emailPlaceholder: t('auth.forgotPassword.emailPlaceholder', { defaultValue: 'name@example.com' }), - submitButton: t('auth.forgotPassword.submitButton', { defaultValue: 'Send reset link' }), + submitButton: t('auth.forgotPassword.submitButton', { defaultValue: 'Send Reset Link' }), submittingButton: t('auth.forgotPassword.submittingButton', { - defaultValue: 'Sending…', + defaultValue: 'Sending...', }), successTitle: t('auth.forgotPassword.successTitle', { defaultValue: 'Check your email', }), successDescription: t('auth.forgotPassword.successDescription', { - defaultValue: 'If an account exists, a reset link has been sent.', + defaultValue: "We've sent a password reset link to {{email}}. Please check your inbox.", }), backToSignInText: t('auth.forgotPassword.backToSignInText', { defaultValue: 'Back to sign in', diff --git a/apps/console/src/pages/auth/LoginPage.tsx b/apps/console/src/pages/auth/LoginPage.tsx index 09d2f91c5b..f44d513d8b 100644 --- a/apps/console/src/pages/auth/LoginPage.tsx +++ b/apps/console/src/pages/auth/LoginPage.tsx @@ -366,7 +366,7 @@ function LoginFormCard({ linkComponent={RouterLink} errorMessages={{ INVALID_EMAIL_OR_PASSWORD: t('auth.login.errors.invalidCredentials', { - defaultValue: 'Invalid email or password', + defaultValue: 'Invalid email or password. Please try again.', }), EMAIL_NOT_VERIFIED: t('auth.login.errors.emailNotVerified', { defaultValue: 'Please verify your email address before signing in.', @@ -383,7 +383,7 @@ function LoginFormCard({ passwordPlaceholder: t('auth.login.passwordPlaceholder', { defaultValue: 'Enter your password' }), forgotPasswordText: t('auth.login.forgotPasswordText', { defaultValue: 'Forgot password?' }), submitButton: t('auth.login.submitButton', { defaultValue: 'Sign In' }), - submittingButton: t('auth.login.submittingButton', { defaultValue: 'Signing in…' }), + submittingButton: t('auth.login.submittingButton', { defaultValue: 'Signing in...' }), noAccountText: t('auth.login.noAccountText', { defaultValue: "Don't have an account?" }), signUpText: t('auth.login.signUpText', { defaultValue: 'Sign up' }), phoneLabel: t('auth.login.phoneLabel', { defaultValue: 'Phone number' }), diff --git a/apps/console/src/pages/auth/RegisterPage.tsx b/apps/console/src/pages/auth/RegisterPage.tsx index 4b1df69de1..3b9cf94f9d 100644 --- a/apps/console/src/pages/auth/RegisterPage.tsx +++ b/apps/console/src/pages/auth/RegisterPage.tsx @@ -151,9 +151,9 @@ export function RegisterPage() {

{t('auth.verifyEmail.resendFailed', { - defaultValue: 'Failed to resend verification email', + defaultValue: 'Cannot resend verification email', })}

{resendError}

diff --git a/packages/app-shell/src/console/home/HomeLayout.tsx b/packages/app-shell/src/console/home/HomeLayout.tsx index a9fa2e752f..ff8f4f71e2 100644 --- a/packages/app-shell/src/console/home/HomeLayout.tsx +++ b/packages/app-shell/src/console/home/HomeLayout.tsx @@ -62,7 +62,7 @@ export function HomeLayout({ children }: HomeLayoutProps) { chat (the dock maximized; Home has no shell to dock a rail into). */} {showChatbot && ( )} diff --git a/packages/app-shell/src/console/home/HomePage.tsx b/packages/app-shell/src/console/home/HomePage.tsx index a73a97aed0..c998302a01 100644 --- a/packages/app-shell/src/console/home/HomePage.tsx +++ b/packages/app-shell/src/console/home/HomePage.tsx @@ -305,7 +305,7 @@ export function HomePage() { if (loading) { return (
-
{t('home.loading', { defaultValue: 'Loading workspace…' })}
+
{t('home.loading', { defaultValue: 'Loading workspace...' })}
); } @@ -327,12 +327,12 @@ export function HomePage() { */} {isAdmin ? ( - {t('home.welcome', { product: getRuntimeConfig().branding.productName, defaultValue: 'Welcome to {{product}}' })} + {t('home.welcome', { product: getRuntimeConfig().branding.productName, defaultValue: 'Build your business system with AI' })} {buildAvailable ? t('home.welcomeAdminDescription', { defaultValue: - 'Describe your business in one sentence — AI generates the objects, screens, APIs and agent tools. Or set things up yourself from the Administration menu on the left.', + 'Describe your business in one sentence — AI generates the objects, screens, APIs and agent tools. Or set things up yourself from the menu on the left.', }) : askAvailable ? t('home.welcomeAdminDescriptionNoBuild', { @@ -358,7 +358,7 @@ export function HomePage() { {t('home.noAppsDescription', { defaultValue: - 'There are no applications available to you yet. Please contact your workspace administrator.', + 'Your workspace is being set up — apps your admin shares with you will show up here.', })} diff --git a/packages/app-shell/src/console/home/HomeRail.tsx b/packages/app-shell/src/console/home/HomeRail.tsx index a47bdf4dce..79d93eea20 100644 --- a/packages/app-shell/src/console/home/HomeRail.tsx +++ b/packages/app-shell/src/console/home/HomeRail.tsx @@ -188,7 +188,7 @@ export function HomeContinue({ items, onOpen, t }: { items: RecentItem[]; onOpen export function HomeActivity({ items, onViewAll, t }: { items: ActivityItem[]; onViewAll: () => void; t: TFn }) { const { language } = useObjectTranslation(); return ( - + {items.length === 0 ? (

{t('layout.activityFeed.empty', { defaultValue: 'No recent activity' })} diff --git a/packages/app-shell/src/console/organizations/OrganizationsPage.tsx b/packages/app-shell/src/console/organizations/OrganizationsPage.tsx index f848754e45..2b76b2ab02 100644 --- a/packages/app-shell/src/console/organizations/OrganizationsPage.tsx +++ b/packages/app-shell/src/console/organizations/OrganizationsPage.tsx @@ -175,11 +175,11 @@ export function OrganizationsPage() {

- {t('organizations.heading', { defaultValue: 'Your Organizations' })} + {t('organizations.heading', { defaultValue: 'Your Workspaces' })}

{t('organizations.subtitle', { - defaultValue: 'Select an organization to continue, or create a new one.', + defaultValue: 'Select a workspace to continue, or create a new one.', })}

@@ -191,7 +191,7 @@ export function OrganizationsPage() { value={query} onChange={(e) => setQuery(e.target.value)} placeholder={t('organizations.searchPlaceholder', { - defaultValue: 'Search for an organization', + defaultValue: 'Search for a workspace', })} className="pl-9" data-testid="organizations-search" @@ -200,7 +200,7 @@ export function OrganizationsPage() { {canCreateOrg && ( )}
@@ -208,24 +208,24 @@ export function OrganizationsPage() { {orgList.length === 0 ? ( - {t('organizations.emptyTitle', { defaultValue: 'No organizations yet' })} + {t('organizations.emptyTitle', { defaultValue: 'No workspaces yet' })} {t('organizations.emptyDescription', { - defaultValue: 'Create your first organization to get started.', + defaultValue: 'Create your first workspace to get started.', })} {canCreateOrg && ( )} ) : filtered.length === 0 ? (
{t('organizations.noMatches', { - defaultValue: 'No organizations match your search.', + defaultValue: 'No workspaces match your search.', })}
) : ( @@ -251,7 +251,7 @@ export function OrganizationsPage() {
{org.name}
{isActive - ? t('organizations.current', { defaultValue: 'Current organization' }) + ? t('organizations.current', { defaultValue: 'Current workspace' }) : org.slug}
diff --git a/packages/app-shell/src/layout/AppHeader.tsx b/packages/app-shell/src/layout/AppHeader.tsx index 594722766b..82fb3cd7ad 100644 --- a/packages/app-shell/src/layout/AppHeader.tsx +++ b/packages/app-shell/src/layout/AppHeader.tsx @@ -749,7 +749,7 @@ export function AppHeader({ <> - {t('organizations.title', { defaultValue: 'Organizations' })} + {t('organizations.title', { defaultValue: 'Workspaces' })} )} diff --git a/packages/app-shell/src/layout/AppSidebar.tsx b/packages/app-shell/src/layout/AppSidebar.tsx index f7046d0455..02937540d6 100644 --- a/packages/app-shell/src/layout/AppSidebar.tsx +++ b/packages/app-shell/src/layout/AppSidebar.tsx @@ -644,7 +644,7 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri - {t('layout.appSwitcher.systemConsole', { defaultValue: 'System' })} + {t('layout.appSwitcher.systemConsole', { defaultValue: 'System Console' })} diff --git a/packages/app-shell/src/layout/ChatDock.tsx b/packages/app-shell/src/layout/ChatDock.tsx index f76f38adce..95e3034f25 100644 --- a/packages/app-shell/src/layout/ChatDock.tsx +++ b/packages/app-shell/src/layout/ChatDock.tsx @@ -560,7 +560,7 @@ export function ChatDockLauncher({ onExpand, className }: ChatDockLauncherProps) onClick={onExpand} data-testid="chat-dock-launcher" aria-label={t('console.ai.dock.open', { defaultValue: 'Open assistant' })} - title={t('console.ai.dock.open', { defaultValue: 'Open assistant (⌘⇧I)' })} + title={t('console.ai.dock.open', { defaultValue: 'Open assistant' })} className={cn( 'fixed right-0 top-1/2 z-40 hidden h-16 w-7 -translate-y-1/2 rounded-l-md rounded-r-none border-r-0 bg-background shadow-md md:inline-flex', className, diff --git a/packages/app-shell/src/layout/InboxPopover.tsx b/packages/app-shell/src/layout/InboxPopover.tsx index 0ef8d0f9a1..705008eb99 100644 --- a/packages/app-shell/src/layout/InboxPopover.tsx +++ b/packages/app-shell/src/layout/InboxPopover.tsx @@ -312,7 +312,7 @@ export function InboxPopover({ - {t('sidebar.activityFeed', { defaultValue: 'Activity' })} + {t('sidebar.activityFeed', { defaultValue: 'Activity feed' })} diff --git a/packages/app-shell/src/views/InterfaceListPage.tsx b/packages/app-shell/src/views/InterfaceListPage.tsx index d9210e071f..a4e819aa4f 100644 --- a/packages/app-shell/src/views/InterfaceListPage.tsx +++ b/packages/app-shell/src/views/InterfaceListPage.tsx @@ -433,7 +433,7 @@ export function InterfaceListPage({ page, className, onConfigChange, reserveEdit
- {t('empty.objectNotFound', { defaultValue: 'Source object not found' })} + {t('empty.objectNotFound', { defaultValue: 'Object Not Found' })} {t('empty.interfacePageSourceMissing', { defaultValue: 'This interface page references "{{name}}", which is not available.', diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index 35609bf272..fa4a2ff530 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -1793,7 +1793,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri history: { entries: historyEntries ?? [], loading: historyLoading && historyEntries === null, - unknownUserText: t('detail.unknownUser', { defaultValue: 'Unknown user' }), + unknownUserText: t('detail.unknownUser', { defaultValue: 'Unknown' }), }, }), // Approvals tab (#3461) — only when the record actually has requests, @@ -1958,7 +1958,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // because the page is not focused or running over http://). try { await navigator.clipboard.writeText(window.location.href); - toast.success(t('detail.linkCopied', { defaultValue: 'Link copied' })); + toast.success(t('detail.linkCopied', { defaultValue: 'Link copied to clipboard' })); } catch (err: any) { toast.error( t('detail.linkCopyFailed', { defaultValue: 'Failed to copy link' }) + diff --git a/packages/app-shell/src/views/ReportConfigPanel.tsx b/packages/app-shell/src/views/ReportConfigPanel.tsx index bacdc973fc..e655519620 100644 --- a/packages/app-shell/src/views/ReportConfigPanel.tsx +++ b/packages/app-shell/src/views/ReportConfigPanel.tsx @@ -140,11 +140,11 @@ export function ReportConfigPanel({ className="hidden sm:flex w-[440px] shrink-0 flex-col border-l bg-background h-full" data-testid="report-config-panel" role="complementary" - aria-label={t('report.editor.title', { defaultValue: 'Edit report' })} + aria-label={t('report.editor.title', { defaultValue: 'Title' })} >
- {t('report.editor.title', { defaultValue: 'Edit report' })} + {t('report.editor.title', { defaultValue: 'Title' })}
diff --git a/packages/plugin-list/src/components/ViewSettingsPopover.tsx b/packages/plugin-list/src/components/ViewSettingsPopover.tsx index a73a20994a..8ae5879359 100644 --- a/packages/plugin-list/src/components/ViewSettingsPopover.tsx +++ b/packages/plugin-list/src/components/ViewSettingsPopover.tsx @@ -213,7 +213,7 @@ export function ViewSettingsPopover(props: ViewSettingsPopoverProps) { {showColor && setRowColorConfig && (
setRowColorConfig(undefined) : undefined} clearLabel={t('list.clear', { defaultValue: 'Clear' })} defaultOpen={!!rowColorConfig} @@ -299,7 +299,7 @@ export function ViewSettingsPopover(props: ViewSettingsPopoverProps) { {showHideFields && hiddenFields && updateHiddenFields && (
0 ? () => updateHiddenFields(new Set()) : undefined} clearLabel={t('list.showAll', { defaultValue: 'Show all' })} diff --git a/scripts/__tests__/check-i18n-call-site-keys.test.ts b/scripts/__tests__/check-i18n-call-site-keys.test.ts index 74a7ca893e..0a89c44459 100644 --- a/scripts/__tests__/check-i18n-call-site-keys.test.ts +++ b/scripts/__tests__/check-i18n-call-site-keys.test.ts @@ -36,6 +36,14 @@ import { * calls reach i18next, 1074 reach a module-local `engine.*` table, 41 are * not translators at all. The synthetic-repo tests pin each classification * independently of what today's `main` happens to contain. + * + * 3. objectui#3810 added a third: the gate now reads `en` VALUES, and a call + * site's own inline `defaultValue` must repeat the value byte for byte + * whenever the key exists. Both halves are pinned — the extractor against + * the evaluated module (values, not only keys), and the rule's verdicts + * against synthetic repos, including the cases it deliberately declines to + * judge. That last group is the one worth reading before widening the rule: + * each abstention is a decision, not an oversight. */ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); @@ -78,6 +86,15 @@ function leafPaths(node: unknown, prefix = ''): string[] { : [prefix]; } +/** The same walk, carrying each leaf's STRING — what the drift rule compares. */ +function leafEntries(node: unknown, prefix = ''): Array<[string, string]> { + return node !== null && typeof node === 'object' + ? Object.entries(node as Record).flatMap(([k, v]) => + leafEntries(v, prefix ? `${prefix}.${k}` : k), + ) + : [[prefix, String(node)]]; +} + /** Materialises `{ 'packages/x/src/a.tsx': '…' }` into a throwaway repo root. */ function repoWith(files: Record): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'check-i18n-keys-')); @@ -92,9 +109,10 @@ function repoWith(files: Record): string { /** A minimal `en` pack for the synthetic repos below. */ const EN_FIXTURE = `const en = { - common: { save: 'Save', cancel: 'Cancel' }, + common: { save: 'Save', cancel: 'Cancel', loading: 'Loading...' }, detail: { showEmptyRelated_one: '+ {{count}} empty', showEmptyRelated_other: '+ {{count}} empty' }, grid: { column: { label: 'Label', width: 'Width' } }, + confirm: { purge: 'Deleting resets it to the shipped baseline. ' + 'Continue?' }, } as const; export default en; `; @@ -388,6 +406,139 @@ export function describe(t: TranslateFn): string { return t('legacy.helper'); } }); }); +describe('an inline defaultValue on a key that EXISTS must match the en value (objectui#3810)', () => { + /** Findings of `reason`, rendered as `key: expected -> actual`. */ + function driftOf(root: string): string[] { + return analyze(root) + .findings.filter((f: { reason: string }) => f.reason === 'default-value-drift') + .map((f: { detail: string; expected: string; actual: string }) => `${f.detail}: ${f.expected} -> ${f.actual}`) + .sort(); + } + + it('is silent when the call site copies the pack value byte for byte', () => { + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.save', { defaultValue: 'Save' }); }; +`, + }); + const { findings, counters } = analyze(root); + expect(findings).toEqual([]); + expect(counters.matchingDefaultValues).toBe(1); + }); + + it('reports the dead string that says something else, with both texts', () => { + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.save', { defaultValue: 'Save changes' }); }; +`, + }); + expect(driftOf(root)).toEqual(['common.save: Save -> Save changes']); + }); + + it('catches a difference of one character — the ellipsis families are the whole reason', () => { + // `Loading...` (three periods) against `Loading` + U+2026 renders the same to + // a reader skimming a diff, which is how six of the 43 sites survived. Both + // sides are written as escapes here for the same reason. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.loading', { defaultValue: 'Loading\\u2026' }); }; +`, + }); + expect(driftOf(root)).toEqual(['common.loading: Loading... -> Loading\u2026']); + }); + + it('folds a concatenated en value before comparing, so a wrapped sentence is judged', () => { + const both = 'Deleting resets it to the shipped baseline. Continue?'; + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { + const { t } = useObjectTranslation(); + return [t('confirm.purge', { defaultValue: '${both}' }), t('confirm.purge', { defaultValue: 'Continue?' })]; +}; +`, + }); + // The matching one is silent; only the half-sentence is reported. + expect(driftOf(root)).toEqual([`confirm.purge: ${both} -> Continue?`]); + }); + + it('leaves a key en does not define to the missing-key rule, and reports it ONCE', () => { + // The two classes must stay disjoint: a missing key with an inline default is + // objectui#3517's shape, and saying "and it drifts" about a key with no value + // would be both noise and false. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.reset', { defaultValue: 'Reset' }); }; +`, + }); + expect(analyze(root).findings.map((f: { reason: string }) => f.reason)).toEqual(['missing-key']); + }); + + it('counts rather than judges a computed default — there is no text to compare', () => { + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (label: string) => { + const { t } = useObjectTranslation(); + return [t('common.save', { defaultValue: label }), t('common.cancel', { defaultValue: \`Go \${label}\` })]; +}; +`, + }); + const { findings, counters } = analyze(root); + expect(findings).toEqual([]); + expect(counters.computedDefaultValues).toBe(2); + expect(counters.literalDefaultValues).toBe(0); + }); + + it('counts rather than judges a plural family and a several-literal key', () => { + // `detail.showEmptyRelated` resolves through `_one`/`_other`: there is no one + // form to compare against, and picking one would be an invention. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (flag: boolean) => { + const { t } = useObjectTranslation(); + return [ + t('detail.showEmptyRelated', { count: 2, defaultValue: 'anything' }), + t(flag ? 'common.save' : 'common.cancel', { defaultValue: 'anything' }), + t('grid.column', { returnObjects: true, defaultValue: 'anything' }), + ]; +}; +`, + }); + const { findings, counters } = analyze(root); + expect(findings).toEqual([]); + expect(counters.unjudgedDefaultValues).toBe(3); + }); + + it('reads the same values the module loader evaluates, not just the same keys', () => { + // The key half of this pin has existed since objectui#3530; the value half + // is what the drift rule rests on. A parser that folded a concatenation + // wrongly, or dropped an escape, would accuse correct call sites. + const parsed = collectEnKeys(repoRoot); + const runtime = new Map(leafEntries(realEn)); + const disagreeing = [...runtime] + .filter(([key, value]) => parsed.values.has(key) && parsed.values.get(key) !== value) + .map(([key]) => key); + const unread = [...runtime.keys()].filter((key) => !parsed.values.has(key)); + expect(disagreeing, `${disagreeing.length} key(s) parsed to a different string`).toEqual([]); + expect(unread, 'every leaf in en today is a static string, so none should be unreadable').toEqual([]); + expect(parsed.values.size).toBeGreaterThan(2000); + }); + + it('main carries no drift, which is why this rule has no baseline', () => { + // objectui#3810 measured 43 sites in 19 files and aligned all of them in the + // same PR. A finding here is a NEW divergence: fix the call site, not the + // pack. If this ever has to be waived, that decision needs a baseline + // section and an issue — not an edit to `en.ts` to make the red go away. + expect(driftOf(repoRoot)).toEqual([]); + }); +}); + describe('the baseline is a ratchet', () => { const finding = (reason: string, detail: string) => ({ reason, detail, file: 'f.tsx', line: 1, column: 1 }); diff --git a/scripts/check-i18n-call-site-keys.mjs b/scripts/check-i18n-call-site-keys.mjs index bcfdeff5b8..1cd64aa7da 100644 --- a/scripts/check-i18n-call-site-keys.mjs +++ b/scripts/check-i18n-call-site-keys.mjs @@ -1,9 +1,12 @@ #!/usr/bin/env node /** - * Every key a component asks `t()` for must EXIST in the `en` locale pack. + * Every key a component asks `t()` for must EXIST in the `en` locale pack — and + * where the call site also writes an inline `defaultValue`, that dead string + * must say the same thing the pack does (objectui#3810). * * Run: node scripts/check-i18n-call-site-keys.mjs (also `pnpm check:i18n-keys`) - * Exit: 0 = every in-scope call-site key resolves (or is baselined), 1 = it does not + * Exit: 0 = every in-scope call-site key resolves (or is baselined) and no + * inline default contradicts its `en` value, 1 = otherwise * * ## The gap this closes (objectui#3530) * @@ -27,15 +30,23 @@ * * All three read the same ten packs, and each is blind to what the next owns: * - * - THIS gate: call site -> `en`. Does the key a component asks for exist? + * - THIS gate: call site -> `en`. Does the key a component asks for exist, and + * does the call site's own inline `defaultValue` agree with the `en` value? + * Both directions of one question — what this call site will render. * - `packages/i18n/src/__tests__/all-locales-key-parity.test.ts`: pack vs pack - * KEY SETS, plus placeholder shape. - * - `scripts/check-i18n-en-drift.mjs` (objectui#3650): the only one that reads - * VALUES, and only as an event — when an `en` string CHANGES, the nine - * translations must change in the same PR (or be waived). Neither key gate - * can see a value go stale: objectui#3582 and objectui#3625 were eight packs - * serving a retired sentence at full key parity, the second one in idiomatic - * native script that every value-shaped heuristic also reads as healthy. + * KEY SETS, plus placeholder shape. Never reads a value. + * - `scripts/check-i18n-en-drift.mjs` (objectui#3650): `en` vs the other nine + * packs, as an event — when an `en` string CHANGES, the nine translations + * must change in the same PR (or be waived). Neither pack gate can see a + * value go stale: objectui#3582 and objectui#3625 were eight packs serving a + * retired sentence at full key parity, the second one in idiomatic native + * script that every value-shaped heuristic also reads as healthy. + * + * The `default-value-drift` class below is the fourth blind spot in that + * partition, and it was blind to all three by construction (objectui#3810): + * this gate asked only whether the key existed, parity reads no values at all, + * and en-drift fires on CHANGE — and in every one of the 43 sites the `en` value + * had not moved for months. The call site was the thing that disagreed. * * ## What is IN scope, and why the answer is not "every `t(`" * @@ -78,7 +89,7 @@ * every one of them a component that was handed the metadata-admin table's `t` * by its parent, and not one of them a real finding. * - * ## Two failure classes + * ## Three failure classes * * 1. `missing-key` — a literal key with no leaf in `en`. i18next plural suffixes * (`_one`, `_other`, …) count as defining the base key, and a key passed with @@ -87,6 +98,33 @@ * whose static head matches NO `en` key at all. Then every possible expansion * is missing, whatever the substitution evaluates to. This is the only claim * about a dynamic key that is true without knowing the value. + * 3. `default-value-drift` (objectui#3810) — the key EXISTS and the call site + * still carries a literal `t(key, { defaultValue: 'other English' })` whose + * text differs from the `en` value. i18next uses `defaultValue` only on a + * miss, so with the key present the pack always wins and that string is + * structurally dead code — dead code that states, at the call site, a + * different sentence from the one users read. Two costs, both measured on + * `main`: the reader (and the AI writing the next edit) is misled — + * `ForgotPasswordPage` claimed `If an account exists, a reset link has been + * sent.` while the pack asserts `We've sent a password reset link to + * {{email}}.`, a materially different privacy claim — and on the day the key + * is renamed or dropped, rendering silently falls back to that other + * sentence, in a diff where nobody expected copy to change. + * + * Classes 1 and 3 are disjoint by construction: drift is judged only when the + * key resolves to an `en` leaf this file could read as a string, so a call + * site is never reported twice, and each report reads on its own. Deliberately + * NOT judged (counted instead): a computed default (`defaultValue: label`), a + * call whose key is dynamic or denotes several literals, a `returnObjects` + * subtree, and the plural families — `t('detail.showEmptyRelated')` resolves + * through `_one`/`_other`, and there is no single form to compare against. + * + * The rule is HARD from day one — no baseline section, unlike classes 1-2. + * That is a measurement, not an aspiration: the first full run found 43 sites + * in 19 files, all of them aligned in the same PR (objectui#3810), so there + * is no debt for a ratchet to hold. A `defaultValue` written on a key that is + * NOT yet in `en` stays legal — that transition period runs for months + * (objectui#3546) and is class 1's business, not this one's. * * ## Dynamic keys: the explicit policy * @@ -111,7 +149,8 @@ * ## The baseline * * `scripts/i18n-call-site-key-baseline.json` lists the keys already missing on - * `main` when this gate landed, each with the issue tracking its fix. It is a + * `main` when this gate landed, each with the issue tracking its fix — classes 1 + * and 2 only; class 3 has no baseline and never needed one. It is a * ratchet, not an allowlist: a key that is NOT in it fails, and an entry that no * longer fires (key added to `en`, or its last call site deleted) ALSO fails, so * the file can only shrink. Fixing the debt means adding the key to @@ -173,13 +212,40 @@ const PLURAL_SUFFIXES = ['_zero', '_one', '_two', '_few', '_many', '_other']; // ── the `en` pack ──────────────────────────────────────────────────────────── /** - * Dotted leaf paths of `packages/i18n/src/locales/en.ts`, read from its AST. + * Read a node as a static string, or return `null` if it is not one. + * + * `'a' + 'b'` counts: `en.ts` wraps one long sentence that way + * (`objectActions.resetPackageSetConfirm`), and a leaf this returns `null` for + * is a leaf the `default-value-drift` rule cannot judge — so folding the + * concatenation here is what keeps that one key inside the checked surface + * instead of silently outside it. + */ +function staticString(node, source) { + const inner = unwrapExpression(node); + if (!inner) return null; + if (ts.isStringLiteral(inner) || ts.isNoSubstitutionTemplateLiteral(inner)) return inner.text; + if (ts.isBinaryExpression(inner) && inner.operatorToken.kind === ts.SyntaxKind.PlusToken) { + const left = staticString(inner.left, source); + const right = staticString(inner.right, source); + return left === null || right === null ? null : left + right; + } + return null; +} + +/** + * Dotted leaf paths of `packages/i18n/src/locales/en.ts`, read from its AST, + * plus the leaf VALUES — the strings the app actually renders. * * Parsed rather than imported so the gate needs no build step and no TS loader. * `scripts/__tests__/check-i18n-call-site-keys.test.ts` pins this extraction - * against the real module evaluated by vitest, so the two cannot drift. + * against the real module evaluated by vitest, keys AND values, so the two + * cannot drift. + * + * `values` holds only leaves that read as a static string. Every leaf is in + * `leaves` either way: the key rules judge existence and need no value, and the + * value rule declines to judge what it could not read rather than guessing. * - * @returns {{ leaves: Set, branches: Set }} + * @returns {{ leaves: Set, branches: Set, values: Map }} */ export function collectEnKeys(root) { const file = join(root, 'packages/i18n/src/locales/en.ts'); @@ -209,6 +275,7 @@ export function collectEnKeys(root) { const leaves = new Set(); const branches = new Set(); + const values = new Map(); const walk = (object, prefix) => { for (const prop of object.properties) { if (!ts.isPropertyAssignment(prop)) { @@ -228,11 +295,13 @@ export function collectEnKeys(root) { walk(value, path); } else { leaves.add(path); + const text = staticString(value, source); + if (text !== null) values.set(path, text); } } }; walk(literal, ''); - return { leaves, branches }; + return { leaves, branches, values }; } // ── source walk ────────────────────────────────────────────────────────────── @@ -448,6 +517,30 @@ function literalKeysOf(argument, source) { return { keys, dynamic }; } +/** + * The inline `defaultValue` an options argument carries (objectui#3810). + * + * `{ present: false }` — no `defaultValue` property at all. + * `{ present: true, text }` — a static string this rule can compare. + * `{ present: true, text: null }` — written, but computed (a template with a + * substitution, a variable, a ternary). Not + * comparable, so it is counted, never failed. + */ +function inlineDefaultValue(node, source) { + for (const argument of node.arguments.slice(1)) { + const inner = unwrapExpression(argument); + if (!inner || !ts.isObjectLiteralExpression(inner)) continue; + for (const property of inner.properties) { + if (!ts.isPropertyAssignment(property)) continue; + const name = + ts.isIdentifier(property.name) || ts.isStringLiteral(property.name) ? property.name.text : null; + if (name !== 'defaultValue') continue; + return { present: true, text: staticString(property.initializer, source) }; + } + } + return { present: false, text: null }; +} + /** The literal head of a template key, i.e. everything before the first `${`. */ function staticHead(argument) { const inner = unwrapExpression(argument); @@ -464,7 +557,7 @@ function staticHead(argument) { * @returns {{ findings: Array, counters: Record, enKeyCount: number }} */ export function analyze(root) { - const { leaves, branches } = collectEnKeys(root); + const { leaves, branches, values } = collectEnKeys(root); const resolvesLeaf = (key) => leaves.has(key) || PLURAL_SUFFIXES.some((suffix) => leaves.has(key + suffix)); // Materialised once, not inside the predicate: spreading a 2.6k-entry Set per // candidate head is the shape that made `all-locales-key-parity` quadratic @@ -487,6 +580,10 @@ export function analyze(root) { skippedLocalTable: 0, skippedNotATranslator: 0, skippedMethodCall: 0, + literalDefaultValues: 0, + matchingDefaultValues: 0, + computedDefaultValues: 0, + unjudgedDefaultValues: 0, }; for (const file of collectSourceFiles(root)) { @@ -601,6 +698,32 @@ export function analyze(root) { if (resolvesLeaf(key) || (returnsObjects && branches.has(key))) counters.resolvedKeys += 1; else findings.push({ reason: 'missing-key', ...at, detail: key }); } + + // objectui#3810 — the inline default is dead code the moment the key + // exists, so it must not say something else. Deliberately narrow: one + // literal key, whose `en` leaf is a plain string. A key `en` does not + // define is `missing-key`'s territory and is NOT reported here too; + // keeping the two classifications disjoint is what lets either output + // be read on its own. + const inlineDefault = inlineDefaultValue(node, source); + if (inlineDefault.present && inlineDefault.text === null) { + counters.computedDefaultValues += 1; + } else if (inlineDefault.present) { + counters.literalDefaultValues += 1; + const key = !dynamic && keys.length === 1 ? keys[0] : null; + const enValue = key !== null && !returnsObjects ? values.get(key) : undefined; + if (enValue === undefined) { + // No single static key, or a key whose `en` leaf this parser could + // not read as a string — including the plural families, where + // there is no one form to compare against. Counted, never failed. + counters.unjudgedDefaultValues += 1; + } else if (enValue === inlineDefault.text) { + counters.matchingDefaultValues += 1; + } else { + findings.push({ reason: 'default-value-drift', ...at, detail: key, expected: enValue, actual: inlineDefault.text }); + } + } + if (dynamic) { counters.dynamicKeySites += 1; const head = staticHead(argument); @@ -681,8 +804,23 @@ const HINTS = { '`createSafeTranslation(...)` bound to a name outside the `use*Translation` /' + ' `use*Translate` / `use*T` convention. Every call through it would leave this' + ' gate\'s checked surface silently — rename it to the convention.', + 'default-value-drift': + 'The key EXISTS in `en`, so i18next serves the pack value and this inline' + + ' `defaultValue` never renders — but it says something else, which misleads every' + + ' later reader of this component and becomes the visible copy the day the key is' + + ' renamed (objectui#3810). Fix it at the CALL SITE: copy the `en` value in' + + ' byte-for-byte, ellipsis and capitalisation included. Do NOT edit' + + ' `packages/i18n/src/locales/en.ts` to match the call site — the pack value is what' + + ' users read today, and changing it makes `scripts/check-i18n-en-drift.mjs` demand' + + ' the same change in the other nine packs. If the pack value is genuinely the wrong' + + ' copy for this spot, that is a copy change in its own PR, or the call site is asking' + + ' for the wrong key.', }; +/** JSON-quoted, with every non-ASCII byte escaped — `...` vs `…` must be visible. */ +const quote = (text) => + JSON.stringify(text).replace(/[^\x20-\x7e]/g, (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, '0')}`); + const invokedDirectly = process.argv[1] && resolve(process.argv[1]) === resolve(fileURLToPath(import.meta.url)); if (invokedDirectly) { @@ -710,26 +848,55 @@ if (invokedDirectly) { `${counters.skippedLocalTable} module-local table, ${counters.skippedNotATranslator} not a translator, ` + `${counters.skippedMethodCall} method call.`, ); + console.log( + `Inline defaults: ${counters.literalDefaultValues} literal ` + + `(${counters.matchingDefaultValues} match their en value, ${counters.unjudgedDefaultValues} not comparable), ` + + `${counters.computedDefaultValues} computed (report-only).`, + ); + + // Split by class before printing: the two key classes say "en does not define + // this", the drift class says "en defines it differently", and one paragraph + // cannot honestly introduce both. + const drift = unexpected.filter((finding) => finding.reason === 'default-value-drift'); + const keyFindings = unexpected.filter((finding) => finding.reason !== 'default-value-drift'); if (unexpected.length === 0 && stale.length === 0) { - console.log(`Every in-scope call-site key resolves against the en pack (${enKeyCount} keys).`); + console.log( + `Every in-scope call-site key resolves against the en pack (${enKeyCount} keys), and every` + + ' literal inline defaultValue matches the value the pack serves.', + ); process.exit(0); } - if (unexpected.length > 0) { - const distinct = new Set(unexpected.map((f) => `${f.reason} :: ${f.detail}`)); + if (keyFindings.length > 0) { + const distinct = new Set(keyFindings.map((f) => `${f.reason} :: ${f.detail}`)); console.error( - `\n${unexpected.length} call site${unexpected.length === 1 ? '' : 's'} reference${unexpected.length === 1 ? 's' : ''} ` + + `\n${keyFindings.length} call site${keyFindings.length === 1 ? '' : 's'} reference${keyFindings.length === 1 ? 's' : ''} ` + `a key the en pack does not define (${distinct.size} distinct):`, ); - for (const finding of unexpected) { + for (const finding of keyFindings) { console.error(` ${finding.file}:${finding.line}:${finding.column} [${finding.reason}] ${finding.detail}`); } - for (const reason of Object.keys(HINTS)) { - if (unexpected.some((finding) => finding.reason === reason)) console.error(`\n${reason}: ${HINTS[reason]}`); + } + + if (drift.length > 0) { + const distinct = new Set(drift.map((finding) => finding.detail)); + console.error( + `\n${drift.length} inline defaultValue${drift.length === 1 ? '' : 's'} contradict${drift.length === 1 ? 's' : ''} ` + + `the en value of a key that EXISTS (${distinct.size} distinct key${distinct.size === 1 ? '' : 's'}) — ` + + 'the pack value is what renders, so the call site is stating a sentence nobody sees:', + ); + for (const finding of drift) { + console.error(` ${finding.file}:${finding.line}:${finding.column} [${finding.reason}] ${finding.detail}`); + console.error(` en renders: ${quote(finding.expected)}`); + console.error(` call site: ${quote(finding.actual)}`); } } + for (const reason of Object.keys(HINTS)) { + if (unexpected.some((finding) => finding.reason === reason)) console.error(`\n${reason}: ${HINTS[reason]}`); + } + if (stale.length > 0) { console.error( `\n${stale.length} baseline entr${stale.length === 1 ? 'y is' : 'ies are'} stale — the defect is gone, so the` + diff --git a/scripts/check-i18n-en-drift.mjs b/scripts/check-i18n-en-drift.mjs index e0bea426a7..093610b015 100644 --- a/scripts/check-i18n-en-drift.mjs +++ b/scripts/check-i18n-en-drift.mjs @@ -224,9 +224,10 @@ function packLiteral(source, label) { * how a long sentence is wrapped across source lines: `en`'s * `objectActions.resetPackageSetConfirm` is written that way today, and the * folded text is what i18next serves, so it is what must be compared. (The - * value extractor's counterpart in `check-i18n-call-site-keys.mjs` treats such - * a node as an opaque leaf and is right to — it only needs the KEY. This gate - * needs the text, so it has to fold.) + * counterpart in `check-i18n-call-site-keys.mjs` folds it the same way and for + * the same reason, since objectui#3810 gave that gate a value rule too: a leaf + * neither can read as a string stays a key-only leaf there, judged for + * existence and not for text.) */ function stringValueOf(node) { const value = unwrap(node);