From d9dedb5cb4ae95537ddbcb42c72753b910b63bea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 19:22:10 +0000 Subject: [PATCH 1/2] fix(charts): refuse a cartesian chart that declared no series binding at all A bar / horizontal-bar / line / area / combo chart handed rows but no `series` prop (`series === undefined`) drew axes, grid and legend around zero marks and said nothing. It now renders the objectui#4683 `no-plottable-series` placeholder with its own sentence ("none was declared") and a console warning naming the row keys. Pie, donut, funnel, radar and scatter fall back to a `value` column and keep drawing. The pin that recorded the old silence is rewritten to assert the refusal. Refs objectui#4695 Co-Authored-By: Claude --- .../plugin-charts/src/AdvancedChartImpl.tsx | 70 ++++++++--- ...rtImpl.unprojectedSeriesDimension.test.tsx | 116 +++++++++++++++++- 2 files changed, 164 insertions(+), 22 deletions(-) diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index 1df66ab866..83f001c949 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -2401,23 +2401,41 @@ const SERIES_ONLY_CHART_TYPES: ReadonlySet = new Set([ * and BOTH authoring causes, and the console warning carries the diagnostic pair * (`xAxisKey` + the keys the rows actually carry) exactly as the model does. * - * ## Why `[]` and not "no series at all" - * - * `Array.isArray(series) && series.length === 0` — a binding that was COMPUTED - * and came out empty, which is what both `buildChartSeries` call paths hand - * over. `series === undefined` means no binding was ever computed (a caller that - * never went through the helper); those charts are left byte-for-byte as they - * were. + * ## Both "empty" and "never declared" refuse — and they say different things + * + * Two shapes reach here with rows and nothing to plot, and both refuse + * (objectui#4695, maintainer ruling A): + * + * - **`'empty'`** — `Array.isArray(series) && series.length === 0`: a binding + * was COMPUTED and came out empty, which is what both `buildChartSeries` + * call paths hand over for the objectui#4683 shape above. + * - **`'undeclared'`** — `series === undefined`: no binding was ever computed. + * The caller never went through the helper, and the schema declared no + * `series`, no `categories` and no y-axis field for `normalizeChartSchema` + * to derive one from. `AdvancedChartImplInner` defaults `series` to `[]`, + * so this used to draw the very same frame of zero marks, silently — the + * author (often an AI) wrote a chart with nothing to draw and was never + * told. That is an authoring mistake, so it is refused, not tolerated. + * + * The distinction has to be read HERE, on the raw props: past the inner + * component's `series = []` default the two are the same array. They share one + * placeholder and one `data-chart-error` code (the chart cannot plot a series + * either way) and differ only in the sentence, because the fix differs — + * project a dimension or select a measure for `'empty'`, declare a series at + * all for `'undeclared'`. + * + * Only {@link SERIES_ONLY_CHART_TYPES} are refused, for the reason given on + * that set: every other family falls back to a `value` column and draws. */ -function hasNoPlottableSeries(props: AdvancedChartImplProps): boolean { +type NoPlottableSeries = 'empty' | 'undeclared'; + +function hasNoPlottableSeries(props: AdvancedChartImplProps): NoPlottableSeries | null { const chartType = props.chartType === 'column' ? 'bar' : (props.chartType ?? 'bar'); const rows = Array.isArray(props.data) ? props.data : []; - return ( - SERIES_ONLY_CHART_TYPES.has(chartType) && - rows.length > 0 && - Array.isArray(props.series) && - props.series.length === 0 - ); + if (!SERIES_ONLY_CHART_TYPES.has(chartType) || rows.length === 0) return null; + if (props.series === undefined) return 'undeclared'; + if (Array.isArray(props.series) && props.series.length === 0) return 'empty'; + return null; } /** @@ -2461,7 +2479,7 @@ export default function AdvancedChartImpl(props: AdvancedChartImplProps) { // category key at all cannot plot an axis, which is the more fundamental // failure and the more specific message. Without this gate such a chart would // also warn about its series, printing two diagnoses for one cause. - const noPlottableSeries = !missingCategoryKey && hasNoPlottableSeries(props); + const noPlottableSeries = missingCategoryKey ? null : hasNoPlottableSeries(props); const xAxisKey = props.xAxisKey ?? 'name'; const firstRowKeys = React.useMemo( () => Object.keys((Array.isArray(props.data) ? props.data[0] : undefined) ?? {}), @@ -2487,6 +2505,17 @@ export default function AdvancedChartImpl(props: AdvancedChartImplProps) { // axis the chart did plot, and the keys its rows actually carry. In the // objectui#4683 shape that second half is the tell — bucket rows carrying // the category and NOTHING else say the group column was never written. + // For the objectui#4695 shape the row keys are the other half of the fix: + // they list the columns a series could have named. + if (noPlottableSeries === 'undeclared') { + console.warn( + `[chart] no series binding was declared for the category axis "${xAxisKey}" — rendering an ` + + `explanatory placeholder instead of an empty frame. Row keys: ${JSON.stringify(firstRowKeys)}. ` + + `A bar / line / area / combo chart draws only the columns its \`series\` (or \`categories\`, ` + + `or a y-axis \`field\`) names (objectui#4695).`, + ); + return; + } console.warn( `[chart] no series to plot against the category axis "${xAxisKey}" — rendering an ` + `explanatory placeholder instead of an empty frame. Row keys: ${JSON.stringify(firstRowKeys)}. ` + @@ -2504,7 +2533,16 @@ export default function AdvancedChartImpl(props: AdvancedChartImplProps) { ); } - if (noPlottableSeries) { + if (noPlottableSeries === 'undeclared') { + return ( + + This chart cannot plot any series: none was declared to draw against its{' '} + {xAxisKey} axis. + + ); + } + + if (noPlottableSeries === 'empty') { return ( This chart cannot plot any series: no measure or group reached its{' '} diff --git a/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx index 81953ebba9..31d1a188d7 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx @@ -20,7 +20,11 @@ * - an unprojected group key REFUSES — nothing exists to draw or to label; * - an ordinary (and a PARTIALLY projected) pivot renders unchanged. * - * Every fixture goes through `buildChartSeries` rather than hand-writing the + * objectui#4695 extends the refusal to the neighbouring shape — NO series + * binding at all (`series === undefined`) — with its own sentence; the last + * `describe` pins that arm, its family boundary and its reverse control. + * + * Every #4683 fixture goes through `buildChartSeries` rather than hand-writing the * props, because the defect lives in the composition the two consumers perform * (`ObjectChart`, `DatasetWidget`) and not in either half alone. */ @@ -221,11 +225,12 @@ describe('AdvancedChartImpl — an unprojected SECOND dimension refuses (objectu warn.mockRestore(); }); - it('leaves a caller that computed NO series binding at all untouched', () => { + it('refuses a caller that computed NO series binding at all, in its own sentence (objectui#4695)', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); // `series === undefined` is "no binding was ever computed", not "a binding - // came out empty" — a different sentence, and not this card's. Such charts - // stay byte-for-byte as they were. + // came out empty". This used to draw the same silent frame of zero marks; + // maintainer ruling A on objectui#4695 refuses it too, through the SAME + // placeholder and code, with a sentence that says which of the two it is. const { container } = render( , ); - expect(refusal(container)).toBeNull(); - expect(warn).not.toHaveBeenCalled(); + const undeclared = refusal(container); + expect(undeclared, 'renders the explanatory placeholder').not.toBeNull(); + expect(undeclared?.getAttribute('role')).toBe('status'); + expect(undeclared?.textContent).toContain('status'); + expect(container.querySelector('svg')).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0] ?? '')).toContain('objectui#4695'); + const undeclaredText = undeclared?.textContent; + cleanup(); + + // Told apart from the computed-and-empty refusal: same code, different + // sentence — the fix differs (declare a series vs project / select one). + const { container: empty } = render( + , + ); + const computedEmpty = refusal(empty); + expect(computedEmpty).not.toBeNull(); + expect(computedEmpty?.textContent).not.toBe(undeclaredText); + expect(String(warn.mock.calls[1]?.[0] ?? '')).toContain('objectui#4683'); warn.mockRestore(); }); @@ -345,3 +373,79 @@ describe('AdvancedChartImpl — what the refusal must NOT swallow (objectui#4683 warn.mockRestore(); }); }); + +describe('AdvancedChartImpl — a chart that declared NO series binding at all (objectui#4695)', () => { + const ROWS = [{ status: 'Backlog', est_hours: 12 }, { status: 'Done', est_hours: 30 }]; + + it('refuses on every family whose marks come only from `series`', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + for (const chartType of ['bar', 'horizontal-bar', 'line', 'area', 'combo', 'column']) { + const { container } = render( + , + ); + expect(refusal(container), `${chartType} refuses`).not.toBeNull(); + expect(container.querySelector('svg'), `${chartType} draws no empty frame`).toBeNull(); + cleanup(); + } + warn.mockRestore(); + }); + + it('keeps drawing the families that fall back to a `value` column', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + for (const chartType of ['pie', 'donut', 'funnel', 'radar']) { + const { container } = render( + , + ); + expect(refusal(container), `${chartType} draws`).toBeNull(); + expect(container.querySelector('svg'), `${chartType} paints`).not.toBeNull(); + cleanup(); + } + const { container: scatter } = render( + , + ); + expect(refusal(scatter), 'scatter draws').toBeNull(); + expect(scatter.querySelector('svg'), 'scatter paints').not.toBeNull(); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('reverse control: a bar chart that DECLARES its series renders normally', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container } = render( + , + ); + expect(refusal(container)).toBeNull(); + expect(container.querySelector('[data-chart-error]')).toBeNull(); + expect(container.querySelectorAll('.recharts-rectangle')).toHaveLength(2); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('says nothing about an undeclared chart with no rows to plot at all', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // Same rows.length > 0 precondition as the computed-and-empty arm: the empty + // RESULT is the chart's own empty state, not an authoring mistake. + const { container } = render( + , + ); + expect(refusal(container)).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +}); From 54f2e1d956e5f977c6de21576a567814418bab50 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 19:39:47 +0000 Subject: [PATCH 2/2] test(charts): re-point the objectui#8650 no-series pins at the refusal; add changeset Two objectui#8650 cases pinned a bar chart with no series binding as "mounts and plots nothing". Under the objectui#4695 ruling that shape now refuses, so both wait for the `no-plottable-series` placeholder instead; the positive reading is kept (re-adding the retired read would plot and never refuse). Co-Authored-By: Claude --- .changeset/4695-no-series-binding-refusal.md | 18 ++++++++++++ ...nderer.foreignDialectRetired-8650.test.tsx | 28 +++++++++++++------ 2 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 .changeset/4695-no-series-binding-refusal.md diff --git a/.changeset/4695-no-series-binding-refusal.md b/.changeset/4695-no-series-binding-refusal.md new file mode 100644 index 0000000000..61edfef4d3 --- /dev/null +++ b/.changeset/4695-no-series-binding-refusal.md @@ -0,0 +1,18 @@ +--- +'@object-ui/plugin-charts': patch +--- + +fix(charts): a cartesian chart that declares no series at all now says so instead of drawing an empty frame (objectui#4695) + +A `bar` / `horizontal-bar` / `line` / `area` / `combo` (and `column`) chart handed +data rows but no series binding — no `series`, no `categories`, no y-axis `field`, +so `series` reaches the renderer as `undefined` — drew axes, grid, tooltip and +legend around zero marks and said nothing. It now renders the same placeholder +objectui#4683 introduced for a computed-but-empty series list +(`data-chart-error="no-plottable-series"`, `role="status"`), with its own sentence: +"none was declared" rather than "no measure or group reached" the axis, and a +console warning that lists the row keys a series could name. + +Unchanged: a chart with no rows; pie, donut, funnel, radar and scatter charts, +which fall back to a `value` column and keep drawing; and every chart that +declares its series. diff --git a/packages/plugin-charts/src/ChartRenderer.foreignDialectRetired-8650.test.tsx b/packages/plugin-charts/src/ChartRenderer.foreignDialectRetired-8650.test.tsx index df3fae3049..5b60e5047c 100644 --- a/packages/plugin-charts/src/ChartRenderer.foreignDialectRetired-8650.test.tsx +++ b/packages/plugin-charts/src/ChartRenderer.foreignDialectRetired-8650.test.tsx @@ -110,6 +110,18 @@ const expectCategoryAxisRefusal = async (c: HTMLElement) => { expect(c.querySelector('.recharts-surface')).toBeNull(); }; +/** + * The positive reading for a retired SERIES alias: wait for the refusal + * `AdvancedChartImpl` renders when a bar chart declares no series at all + * (objectui#4695). Same shape and same reasoning as the axis helper above. + */ +const expectNoSeriesRefusal = async (c: HTMLElement) => { + await waitFor(() => + expect(c.querySelector('[data-chart-error="no-plottable-series"]')).not.toBeNull(), + ); + expect(c.querySelector('.recharts-surface')).toBeNull(); +}; + const renderChart = (schema: Record) => render().container; @@ -161,7 +173,8 @@ describe('objectui#8650 — `categories` is NOT retired: the declared key still // `categories: 'revenue'` (a string, not a list) reached `.map` on a string // in the retired branch and threw during render. The normalizer answers // "no series" instead, which is the honest reading of an off-contract - // value: the chart mounts, and plots nothing. + // value: the chart mounts, and — since objectui#4695 — says it has no + // series to plot rather than drawing an empty frame. const container = renderChart({ type: 'chart', chartType: 'bar', @@ -170,8 +183,7 @@ describe('objectui#8650 — `categories` is NOT retired: the declared key still categories: 'revenue', isAnimationActive: false, }); - await waitFor(() => expect(container.querySelector('.recharts-surface')).toBeTruthy()); - expect(marks(container)).toEqual({ bars: 0, lines: 0 }); + await expectNoSeriesRefusal(container); }); }); @@ -210,10 +222,10 @@ describe('objectui#8650 — the foreign dialect is retired', () => { isAnimationActive: false, }); // The axis IS bound here, so this case isolates the series half — and the - // reading is positive: a bar with an empty series list reaches the plot - // surface with no refusal, so `plotted` waits for that surface to ARRIVE - // rather than for a fixed window to elapse, and then counts. Re-add the - // `value` read and a bar appears on it. - expect(await plotted(container)).toEqual({ bars: 0, lines: 0 }); + // reading is positive: with no series declared, a bar chart refuses + // (objectui#4695), so this waits for that refusal to ARRIVE rather than for + // a fixed window to elapse. Re-add the `value` read and a bar plots + // instead, no refusal ever arrives, and the `waitFor` reddens on timeout. + await expectNoSeriesRefusal(container); }); });