fix(core): dashboard measures follow the display locale (#4566) - #4577
Merged
Conversation
`formatMeasure` and `formatDimensionValue` formatted every value with a bare `undefined` locale tag at all three of their `Intl` sites. `undefined` is not "the user's locale", it is the MACHINE's — neither of the repo's two locale channels. A German session read a dashboard KPI as `1,234.5` next to a grid cell rendering the same number as `1.234,5`. Both take the display locale as a new optional LAST parameter; `DatasetWidget` threads `useDisplayLocale()` into every site it formats through (KPI, grouped table measure and dimension cells, cross-tab headers and cells). English output does not move — these sites already grouped through `Intl`, so the only change is whose locale is used. Omitting the argument reproduces the previous output byte for byte. Measured: routing through `formatDisplayNumber` is behaviourally LOSSLESS (0 diffs across 32,760 combinations) but blocked by the package boundary — `@object-ui/core` is React-free and consumed by React-free packages, while `@object-ui/i18n` peer-depends on React and exports no pure-utility subpath. The surviving duplication is recorded at both ends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3)
Auto-merge armed (squash) — landing verified per the merge-queue discipline. #4575 dispatches next, gated on this landing. Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 13, 2026 12:29
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4566
formatMeasure/formatDimensionValuein@object-ui/coreformatted every value with a bareundefinedlocale tag at all three of theirIntlsites.undefinedis not "the user's locale", it is the MACHINE's — neither of the repo's two locale channels. Both take the display locale as a new optional last parameter, andDatasetWidgetthreadsuseDisplayLocale()into every site it formats through.The fork, measured before choosing
The ruling preferred (b) route through
formatDisplayNumberand retire the duplicate, with (a) as the fallback if the mapping was lossy. Both halves were measured.The behavioural mapping is LOSSLESS. A harness replayed the current implementation against a candidate routed through
formatDisplayNumberacross 32,760 combinations (26 values times 10 formats times 6 currencies times 3 percentScales times 7 locales): 0 byte-level differences. The reason is worth recording — the policy layerformatDisplayNumberadds over plainIntlis grouping suppression keyed on a field's declaredscale, and a measure has noscaleto feed it (its decimals come from a numeral format PATTERN), soshouldGroupDisplayNumberreturnstrueevery time and the two agree everywhere. The duplicate is not missing the policy; it is unreachable by it.The routing is blocked by the package boundary, not by behaviour.
@object-ui/coreis the React-free engine (AGENTS.md section 3: "No UI-lib deps. Logic only.", anddataset-format.ts's own header says purity is why it lives there).@object-ui/i18ndepends oni18next/react-i18next, peer-depends on React, and publishes no pure-utility subpath — itsexportsmap is.and./locales/*. Acoretoi18nedge would put React into the dependency closure of the React-FREE consumers of core, measured as: theobject-uiVS Code extension and@object-ui/data-objectstack, both of which take@object-ui/coreas a runtime dependency and declare no React.So this is (a), per the ruling's fallback, and the surviving duplication is recorded at BOTH ends (
dataset-format.tsandnumber-display.ts). Retiring it for real means movingformatDisplayNumberDOWN into@object-ui/coreand re-exporting from@object-ui/i18n— the right direction, since core is the lower layer — but that relocates a published export across a package boundary and is left to its own card.English does not move — the discriminator against #4553
These sites already went through
Intlwith default grouping, so the only thing that changes is whose locale is used. Contrast #4553, whereformatPercenthad never grouped and moving en1235%to1,235%WAS the fix.0.01,234.51,234.5(unchanged)0.01,234.51.234,5€1,234.501.234,50 €(sign last, U+00A0)0.0%60.8%60,8%Omitting the new argument reproduces the previous output byte for byte, which is what makes the parameter safe to land ahead of the consumers that do not thread it yet.
Consumer census — deviates from the presumed list
The dispatch presumed DatasetWidget / ObjectMetric / a pivot path. Measured reality:
DatasetWidget.tsx— 7 sites, all threaded: KPI (1025), pivot viabuildPivot(1128) and its cells (1133), grouped-table measure (1340) and dimension (1341), drill title (1332), plusbuildPivot's two header-label sites (148/149) through its own new optional parameter.ObjectMetricWidgetdoes NOT consumeformatMeasure— it goes throughMetricWidget, which already usesformatDisplayNumberwith a threaded locale. Nothing to do.useMemois involved. Every site is in the render body, so unlike [plugin-gantt] the tasks useMemo omits tenantCurrency, so tooltip currency can render stale #4542 / fix(plugin-gantt): tooltip currency re-formats when the tenant currency resolves (#4542) #4554 / fix(fields): formatPercent groups and follows the display locale — the last tooltip channel (#4553) #4565 there is no dependency array to add the locale to — stated in the test file so a reader does not hunt for the missing case.plugin-report/DatasetReportRenderer.tsx(10 sites) and app-shell'sDatasetPreview/DatasetDefaultInspector(3 sites, an excluded surface). Both still compile — the parameter is optional — and both typecheck clean here.Red-first, predicted in writing before the run
Predictions are recorded in each test file's header. Runner machine locale measured as en-US.
Reverse verification by patch, never
git stash; fix restored and confirmed byte-identical by sha256 (929c5a34...).AssertionError: expected '1,234.5' to be '1.234,5',expected '€1,234.50' to be '1.234,50 €', andexpected [ '9,876.5', '1,234.5' ] to deeply equal [ '9.876,5', '1.234,5' ]forbuildPivot.Every case pins two locales (de and en) rather than one, because a lone de assertion is not falsifiable: on a German runner it would pass before the fix too. With both pinned, at least one must fail on any machine — and that is also exactly the property the fix delivers, namely that the machine locale stops being an input.
Honestly labelled as NOT defect pins (green on both sides, and each says so in its own comment): the malformed-tag guard, the zh-CN case, the integer-verbatim cases, and the
cellIndexinvariance case.,, decimal.), so a Chinese session cannot produce the inverted-separator signal a German one does. The case is kept to record why zh is absent from the red-first set, not as coverage it cannot provide.A hazard the fix itself introduces, and closes
Threading a real tag where a literal
undefinedused to sit is not free:undefinednever throws, but a malformed tag does. Measured,en_US(underscore instead of hyphen, the likeliest tenant-config typo),!!,eandde-DE-u-nu-all throwRangeErrorfrom a baretoLocaleString, and none of the three sites had acatch— that would have taken the whole widget down. A localformatNumberInLocalemirrorsformatDisplayNumber's retry-without-locale, so a bad tag degrades to the runtime default while a bad CURRENCY still throws out of both attempts, which is what keeps the existing unknown-currency fallthrough reachable.Two behaviours deliberately preserved, both measured
IntlWOULD change it:ar-EGre-digits 1234 to its Arabic-Indic spelling, and1e21expands from1e+21to 22 digits.Intl'sstyle: 'percent're-scales by 100 and the round trip loses precision at the top of the range (en100,000,000,000,000,000,000,000%becomes99,999,999,999,999,990,000,000%— 24 of the 32,760 combinations move, all at 1e21 / MAX_SAFE_INTEGER). Since en byte-identity is this card's discriminator, that swap cannot ride along here. Its real consequence — a German list cell writing1.234,5 %where a dashboard measure writes1.234,5%— is filed as [core][fields] A percent renders as1.234,5 %in a list cell and1.234,5%as a dashboard measure — two percent conventions for one number #4576 with the decision it needs, rather than smuggled in behind a locale fix.Verification
pnpm exec vitest run --maxWorkers=2 packages/core packages/plugin-dashboard packages/i18n— 179 files, 2978 tests, all passed (includes the fix(charts): null-keyed groups render as an explicit bucket instead of silently vanishing (#4466) #4498 / fix(charts): the pivot branch buckets null first-dimension values instead of dropping the bar (#4497) #4506 chart-series null-bucket pins; no flaky: DatasetWidget.dottedDimensionTable's "issues the ONE read" case asserts a fetch AFTER waiting only on the rendered cell #4487 flake observed).type-check(bothtscpasses) green on core, i18n, plugin-dashboard — and on the downstream consumersplugin-reportandapp-shell. Consumer sweep direction: downstream consumers of the changed exports, selected by call site rather than by filter glob. app-shell's first run showed 204TS2307errors that were purely the unbuilt-dependency trap; after building its closure it is 0.123as the locale and a 3rd argument toformatDimensionValuewas rejected withTS2345andTS2554, proving tsc read the rebuilt.d.tsrather than a cached one. Probe removed.origin/mainbaseline: net zero — core 529 to 529, i18n 35 to 35, plugin-dashboard 328 to 328, 0 errors throughout. The first draft of the widget test added 4no-explicit-anywarnings; those types were tightened rather than accepted.check:phantom-deps,check:control-bytes,check:changeset-presence,check:changeset-no-majorall green; control-byte self-scan of every touched file clean..d.ts, measured both waysBuilt before and after with
dist/andtsconfig.tsbuildinfocleared between runs:@object-ui/core—formatMeasureandformatDimensionValueeach gainlocale?: string. Both are ENTRY exports (export * from './utils/dataset-format.js'), so an optional parameter on the published surface makes this minor.@object-ui/plugin-dashboard—buildPivotgainslocale?: string, but it is not on the package'sexportssurface (absent from the builtindex.d.ts), so no published declaration changes: patch.@object-ui/i18n— comment only, declarations byte-identical; deliberately absent from the changeset rather than releasing a version for a comment.Generated by Claude Code