fix(charts): refuse a cartesian chart that declared no series binding at all (#4695)

.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.

packages/plugin-charts/src/AdvancedChartImpl.tsx

@@ -2401,23 +2401,41 @@ const SERIES_ONLY_CHART_TYPES: ReadonlySet<string> = 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 (
+      <ChartRefusal code="no-plottable-series" className={props.className}>
+        This chart cannot plot any series: none was declared to draw against its{' '}
+        <code className="font-mono">{xAxisKey}</code> axis.
+      </ChartRefusal>
+    );
+  }
+
+  if (noPlottableSeries === 'empty') {
     return (
       <ChartRefusal code="no-plottable-series" className={props.className}>
         This chart cannot plot any series: no measure or group reached its{' '}

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(
       <AdvancedChartImpl
         chartType="bar"
@@ -234,8 +239,31 @@ describe('AdvancedChartImpl — an unprojected SECOND dimension refuses (objectu
         isAnimationActive={false}
       />,
     );
-    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(
+      <AdvancedChartImpl
+        chartType="bar"
+        data={[{ status: 'Backlog', est_hours: 12 }]}
+        xAxisKey="status"
+        series={[]}
+        isAnimationActive={false}
+      />,
+    );
+    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(
+        <AdvancedChartImpl chartType={chartType as any} data={ROWS} xAxisKey="status" isAnimationActive={false} />,
+      );
+      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(
+        <AdvancedChartImpl
+          chartType={chartType as any}
+          data={[{ name: 'Backlog', value: 12 }, { name: 'Done', value: 30 }]}
+          xAxisKey="name"
+          isAnimationActive={false}
+        />,
+      );
+      expect(refusal(container), `${chartType} draws`).toBeNull();
+      expect(container.querySelector('svg'), `${chartType} paints`).not.toBeNull();
+      cleanup();
+    }
+    const { container: scatter } = render(
+      <AdvancedChartImpl
+        chartType="scatter"
+        data={[{ x: 12, value: 30 }, { x: 20, value: 45 }]}
+        xAxisKey="x"
+        isAnimationActive={false}
+      />,
+    );
+    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(
+      <AdvancedChartImpl
+        chartType="bar"
+        data={ROWS}
+        xAxisKey="status"
+        series={[{ dataKey: 'est_hours' }]}
+        isAnimationActive={false}
+      />,
+    );
+    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(
+      <AdvancedChartImpl chartType="bar" data={[]} xAxisKey="status" isAnimationActive={false} />,
+    );
+    expect(refusal(container)).toBeNull();
+    expect(warn).not.toHaveBeenCalled();
+    warn.mockRestore();
+  });
+});

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<string, unknown>) =>
   render(<ChartRenderer schema={schema as any} />).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);
   });
 });