Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-detail,i18n): RecordComments and PointInTimeRestore resolve their copy from the packs - #7172
Conversation
…e their copy from the packs (#7163)
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-warren
commented
Sep 1, 2026
PM review — |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7163
Measured on
origin/mainheadd8ec8d6d4(PR #7162's landing sha — the card's line numbers predate it, so everything below was re-derived, not inherited). Final commit42f6c1480; every gate result quoted here was run on that tree withgit status --porcelainempty.The card's "byte-identical" premise is FALSE — verified mechanically
The title and both PM comments say the two
formatTimestampcopies are byte-identical. They are not:RecordCommentshas a fourthd agobranch and falls through totoLocaleDateString();PointInTimeRestorestops at hours and falls through totoLocaleString(). A revision list wants the time of day; a comment list does not. (The card body said as much — "the same, minus the days branch"; only the title and the dispatch generalised it.)This decides the sharing question by measurement, per the scope ruling. Sharing is declined on two independent grounds: the two helpers have genuinely different tails, so unifying them would silently restyle one surface; and a shared helper needs a new export, which is Clause ② with the quota exhausted. Reported rather than routed around. The duplication is now three-way (
ActivityTimeline,RecordComments, and this near-copy) and is filed separately as a finding.What shipped
RecordComments.tsx— pure lookup swap, no new key, no copy change. The file was already wired to the packs (11t('detail.…')refs); only its helper was hardcoded. It now resolvesdetail.justNow/minutesAgo/hoursAgo/daysAgo. Each mapping was checked for exactness, not adopted on the card's word:enpack value'just now'detail.justNowjust now`${diffMins}m ago`detail.minutesAgo{{count}}m ago`${diffHours}h ago`detail.hoursAgo{{count}}h ago`${diffDays}d ago`detail.daysAgo{{count}}d agoPointInTimeRestore.tsx— swept WHOLE, not partially wired. It used no translation hook at all (0 refs), so per the ruling it was finished in one pass rather than becoming a third half-done component: card title, empty state, field-count line, preview heading, snapshot heading, both(empty)placeholders, the snapshot dash, the restore confirmation, and all three buttons. 17 statict()call sites; zero English literals remain (each removed anchor greps to 0).Ten new keys land in all ten packs and in
DETAIL_DEFAULT_TRANSLATIONS. Three existing keys are reused rather than forked:detail.cancel,detail.activityEmptyValue(the keyActivityTimelinealready uses for an absent old/new value in a field-change diff — the same concept), anddetail.emptyValue.Two shape rules observed:
{{when}}hole, not a sentence assembled around a JSX expression.t(KEYS[n])—ActivityTimelinehas 18 more hardcoded English literals — the empty state was one of a set, and 12 of them already have pack keys sitting unused #7149 measured that a key visible only as a map value reads as unreferenced to the i18n scanners.One deliberate copy change, called out rather than quietly adopted: the snapshot panel's null placeholder was an EN DASH (
–, U+2013) written inline; it now resolvesdetail.emptyValue, which is an EM DASH (—, U+2014) in all ten packs — the glyph the rest of the detail package already uses.Pack verification method, with both controls
Read out of the pack objects by walking the nested path (
packages/i18n/src/locales/index.ts->builtInLocales), never a dotted-key grep — a grep fordetail.justNowreturns a false zero against theenpack that defines it, and thedetailnamespace is one of five in these packs that carry ajustNowrow.locales loaded: en,zh,ja,ko,de,fr,es,pt,ru,ar (count=10)detail.back10/10,detail.noActivity10/10detail.zzzAbsentControl71630/10Reachability finding (this splits the card, as #7149's did)
PointInTimeRestorehas zero in-repo consumers. Repo-wide, excludingnode_modulesanddist, it appears only in its own file and twice in the package barrel (index.tsx:126export,:154type export). Nothing renders it — notDetailView, notapps/console.RecordComments, by contrast, is rendered byDetailViewat two call sites (:1479,:1705).So the user-visible defect today is
RecordComments;PointInTimeRestoreis public API a downstream consumer can mount but nothing in this repo does. It was still swept whole rather than deferred: it is exported, so it is shippable surface, and finishing it now is what stops the #7142 shape from recurring.Nothing pinned the old literals — attributed, not counted
Repo-wide
grep -Fon'just now'and'm ago'returns hits across five packages. Attributed rather than counted: every test assertion among them belongs topackages/collaboration(a different package and namespace,useCollaborationTranslation) or toActivityTimeline.remainingLiterals.i18n.test.tsx(#7162's suite, asserting on a different component). The twoplugin-detailhits inRecordMetaFooter.tsxare comments, not literals — that file is already a correctt()call site. ThePointInTimeRestorecopy has exactly one hit per literal: the file itself. No test pinned either file's English.Evidence
Ablation — direction predicted before running, mutation proven on disk, restore proven by state.
Predicted: reverting only the two
.tsxfiles tod8ec8d6d4(packs, defaults map and the new suites kept) turns the zh/ar assertions red while everyenand provider-less assertion stays green by construction, since eachenpack value is byte-identical to the literal being restored. Predicted 9 red / 8 green.Observed, exactly:
A(HEAD)=49c08e56…vsA(disk)=b5ce3ad9…,B(HEAD)=0db4d714…vsB(disk)=cac43cf7…; injected markert('detail.justNow')0 in both, restored markerreturn 'just now';1 in both,Revision Historyliteral back to 1.git diff HEADproduced 0 lines,git status --porcelain0 lines, and both on-disk hashes match their HEAD blobs exactly. The script carriedtrap restore EXIT INT TERMwith absolute paths resolved fromgit rev-parse --show-toplevel, and the restore leg namesHEADexplicitly (a baregit checkout -- pathrestores from the index, which the mutation had already written).enand provider-less legs of both suites, the two{{count}}-interpolation guards, and the two raw-key guards (a reverted file renders literals, not keys, so those cannot discriminate).Resolution path stated, since an ablation is only valid if it is measured: the suites resolve source, not
dist/. The components are same-package relative imports, and the root vitest config aliases@object-ui/i18ntopackages/i18n/src(vitest.config.mts:275). No rebuild leg is required for this ablation, and the green run was measured against source too. The dependency closure was built anyway before the first run (pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-detail^...' build, exit 0).Green runs (vitest paths root-relative per #3378, canonical invocation through the root config):
Gates — exit codes captured by redirect-then-capture, never read across a pipe; each verdict is the gate's own printed line:
check:i18n-keysexit 0 — "Every in-scope call-site key resolves against the en pack (2865 keys), every literal inline defaultValue matches the value the pack serves, every call site passes exactly the arguments that value has holes for…"check:i18n-driftexit 0 — "Compared the ten locale packs at d8ec8d6 (merge-base with origin/main) with the working tree: 0 en value(s) changed (10 key(s) added, 0 removed…)"check:i18n-dead-keysexit 0 (report, not a gate)changeset:checkexit 0 — "All workspace packages are in the changeset fixed group." / "No changeset declares amajorbump."check-changeset-presence.mjsexit 0 — "15 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)"check-changeset-overwrite.mjsexit 0 — "No pre-existing changeset was modified or deleted."check:control-bytesexit 0 — "OK (scanned 5947 tracked text file(s); skipped 85 binary)"; plus a directgrep -naPcontrol-char scan over all 16 changed files: 0 hitstype-check(plugin-detail + i18n) exit 0 — "Scope: 2 of 47 workspace projects" with both script names echoed, so not a zero-match silent passlint(plugin-detail + i18n, plain per-packageeslint ., no--no-inline-config) exit 0 — warnings only, all pre-existing; the three onPointInTimeRestore.tsxare theRevisionEntryanyfields at lines 25/27, which this diff does not touchNot NOT-MEASURED:
type-checkrunstsc -p tsconfig.test.json, and--listFilesconfirms both new suites are actually in that program (1 hit each; controlzzzNoSuchFile.tsx= 0) — so the green genuinely covers the new test files rather than excluding them.The
dequote-pairing census did not move. The ⚠ carried forward from #7149 (a pack value with a quoted span shiftsde-quote-pairing-3876.test.ts, 55 -> 58) does not apply: the German values added here contain no quote characters by construction, the census stays at 58, and that file is untouched — verified by running it green rather than by assuming.Scope
Lane held:
packages/plugin-detail/src/{RecordComments,PointInTimeRestore}.tsx, their new suites, and the ten locale packs.ActivityTimeline.tsxwas read-only reference and is unmodified. No fenced package was entered.content/docs/releases/untouched.Generated by Claude Code