fix(charts): null-keyed groups render as an explicit bucket instead of silently vanishing (#4466) - #4498
Merged
Merged
Conversation
…f silently vanishing (#4466) `buildChartSeries`' single-dimension branch passed rows through verbatim, so a row whose category VALUE is null reached recharts with a null category and drew no mark. The partial case is the sharpest: two groups in, ONE bar out — the dominant null-keyed group (51 of 53 events) dropped while the y-axis scale still accommodated it. With every group null it drew axes and gridlines with zero marks and no empty state: the shipped first-boot state of System Overview's "Events by User". The mapping lives in the shared series layer so dashboard widgets and standalone ObjectChart get one answer. The label flows from the renderer through the i18n channel (`chart.nullCategory`), because core is React-free. `findChartSeriesRow` reads the bucket label back to its raw-null row so the newly visible bar keeps its drill-through. `hasNoCategoryKey` (framework#4033) keeps meaning "key absent" — the bucket never adds the key to a row that lacks it.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 21:41
This was referenced Aug 12, 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.
Closes #4466
The defect
buildChartSeries' single-dimension branch (packages/core/src/utils/chart-series.ts) passed rows through verbatim, so a row whose category value isnullreached recharts with a null category and drew no mark.hasNoCategoryKeyinAdvancedChartImplnever fired, because'user_id' in rowis true — the key is present, its value is null.The visible outcome was not an empty chart but a quietly wrong one. Measured, both levels, pre-fix:
[{user_id: null, event_count: 51}, {user_id: 'Dev Admin', event_count: 2}]drew exactly ONE bar. The dominant group (51 of 53 events) was dropped while the y-axis scale still accommodated it, so the chart understated its own data and the axis proved the data had been there.[{user_id: null, event_count: 50}]drew axes, gridlines and the axis title with zero bar rectangles and no empty state. This is the shipped first-boot state of the built-in System Overview board's "Events by User": every seededsys_audit_logrow is written withuser_id = NULL.The fix
The mapping lives in the shared series layer, so dashboard widgets and standalone
ObjectChartget one answer instead of a per-chart patch in the recharts wrapper. It also resolves the two-answers disagreement the card names: an empty result set keeps the designed empty state, a non-empty result always draws bars — the null bucket included.@object-ui/coregainsNULL_CATEGORY_LABELandChartSeriesOptions;buildChartSeriesandfindChartSeriesRoweach take an optional trailingoptions. Purely additive — see the.d.tsdiff below.chart.nullCategory, en(None)/ zh(未指定), in all ten packs, following the measured sibling convention for a parenthesised bucket label (report.allLabel(All)/report.emptyLabel(Empty), translated in every pack).@object-ui/coreis React-free and cannot read the locale bundle, so the renderer passes the resolved string down — the same divisiondimensionOptionTranslatoralready uses, one layer down in the same file. The English constant is the floor for a provider-less host, not the mechanism.null, sofindChartSeriesRowreads the bucket label back to that row. Without it the one bar this fix made visible would resolve to-1and its drill-through would silently no-op —DatasetWidget'shandleChartDrillreturns early on a negative index.hasNoCategoryKeyis untouched and now documented against this. A row that does not carry the category key AT ALL is a different defect (a dimension grouped by but never projected, framework#4033) and keeps its explanatory placeholder. The bucket deliberately never ADDS the key to such a row, which is what keeps that guard's only signal alive. Key absent → the placeholder; key present with a null value → the bucket. Asserted in both directions.Red-first
Written before the fix, run against unfixed code. Verbatim, at both levels the defect was proven at.
Series transform (unit,
@object-ui/core):Rendered DOM (
@object-ui/plugin-charts,.recharts-bar-rectanglecounts through the real transform):The same two counts reproduce end-to-end through a dataset-bound
ObjectChart(reverse-verified by taking the core fix out withgit checkout origin/main -- ...after the fix was green):Post-fix all of it is green: 2 bars for the partial case with
(None)andDev Adminon the axis, 1 labelled bar for the all-null case.Must-not-change (green on both sides)
expect(r.data).toBe(rows)).data-chart-error="missing-category-key"placeholder with nosvgdrawn.null.Verification
Repo-root vitest, paths relative to the repo root.
pnpm exec vitest run packages/core/ packages/i18n/ packages/plugin-charts/ packages/plugin-dashboard/ packages/plugin-report/ packages/react/→ 241 files, 3727 tests passed.plugin-dashboardreaches the fix through core with no file of its own edited, and no test there pinned the dropped-row behaviour.tsc --noEmitandtsc -p tsconfig.test.jsonfor@object-ui/coreand@object-ui/i18n,tsc --noEmitfor@object-ui/plugin-charts— all exit 0, afterpnpm --filter '@object-ui/plugin-charts^...' build..d.ts: a deliberate typo in the new option is rejected by the consumer's own typecheck, so the build closure is genuinely being read and not a cached artifact.check-i18n-call-site-keys.mjs,check-i18n-en-drift.mjs(1 key(s) added),check-control-bytes.mjs,check-changeset-presence.mjs,check-changeset-no-major.mjs,check-changeset-fixed.mjs— all exit 0. ESLint: 0 errors on every touched file..d.tsdiffAdditive in one direction only, which is what sets the grading:
Old call sites compile and behave identically against the new types (both parameters optional); new call sites do not compile against the old ones. The public surface therefore grows, so the changeset is
@object-ui/core: minorwith@object-ui/plugin-chartsand@object-ui/i18npatch — per the ruling's own "minor only if a public surface grows", and following #4431's precedent for core gaining a shared executor. Never major.Generated by Claude Code