feat(charts): ObjectChart honors the spec ChartConfig author shape (#2880)

.changeset/objectchart-honors-chartconfig.md

@@ -0,0 +1,50 @@
+---
+"@object-ui/plugin-charts": minor
+"@object-ui/components": patch
+---
+
+feat(charts): ObjectChart honors the spec `ChartConfig` author shape (objectui#2880 / framework#3729)
+
+`ChartConfigSchema` is the chart protocol, but the renderer only ever read a
+Recharts-flavoured internal shape — `chartType`, `xAxisKey`, `series[].dataKey`.
+Everything an author wrote in the SPEC shape reached the renderer and was
+silently dropped, which is exactly what ADR-0078 forbids. framework#3725
+documented the gap by trimming the published contract down to the props that
+actually worked; this closes it the other way round.
+
+**S1 — one normalization boundary.** `normalizeChartSchema` translates the
+author shape into the internal pipeline contract in a single place, rather than
+scattering `??` fallbacks through the render tree (framework PD #12: one
+translation is a contract mapping, N fallbacks are a second dialect):
+
+- `type` → `chartType`, `xAxis: { field }` → `xAxisKey`, `series: [{ name }]` →
+  `series: [{ dataKey }]`
+- the report surface's bare-string `xAxis`/`yAxis` resolve too
+- `yAxis: [{ field }]` alone plots, with no `series` declared
+- **internal props win**, so `DashboardRenderer`, `ObjectView` and the dataset
+  path are byte-for-byte unaffected — there is no migration
+
+**The `type` collision.** `ChartConfig.type` is the chart family, but on any
+surface that flattens chart config into a props bag `type` is already the SDUI
+envelope's component discriminator. Spreading props last let an author's
+`type="bar"` replace `object-chart` so the block stopped resolving; stamping the
+discriminator last ate the author's value instead. The react-page wrapper now
+keeps both: the discriminator wins the `type` slot and the author's value is
+preserved beside it as `specType`, which the normalizer reads back.
+
+**S2 — axis presentation.** `ChartAxis.format` drives the tick formatter (via
+`Intl.NumberFormat`, no new dependency), `min`/`max` pin the domain,
+`logarithmic` swaps the scale, `title` labels the axis, and `showGridLines` is
+honored. A second `yAxis` entry (or `position: 'right'`) turns on the secondary
+axis that `series[].yAxis` binds to — in combo charts an explicit binding now
+beats the family-derived bar→left/line→right guess. `showLegend` is honored,
+and `title`/`subtitle` render above the plot instead of only titling the
+drill-down drawer.
+
+**S3 — `series[].stack`, `annotations`, `interaction`.** Stacking passes the
+author's group name through as Recharts' `stackId`. Annotations render as
+`ReferenceLine` (`type: 'line'`) / `ReferenceArea` (`type: 'region'`) with the
+declared axis, colour, style and label. `interaction.tooltips: false` suppresses
+the hover card and `interaction.brush: true` adds the range selector;
+`showDataLabels` prints values on the marks. `interaction.zoom` has no Recharts
+primitive behind it and is deliberately still unimplemented rather than faked.

packages/components/src/renderers/layout/react-page.tsx

@@ -59,8 +59,21 @@ function buildComponentScope(dataSource: unknown): Record<string, React.Componen
     const name = toPascal(tag);
     if (seen.has(name)) continue;
     seen.add(name);
-    const Wrapper: React.FC<any> = ({ children: _children, ...props }) =>
-      React.createElement(SchemaRenderer as any, { schema: { type: tag, dataSource, ...props } });
+    // `type` is the SDUI envelope's component discriminator, but it is ALSO a
+    // legitimate prop name in a block's own spec schema — `ChartConfig.type` is
+    // the chart family. Flattening props into the schema bag collides the two:
+    // spreading last let an author's `type="bar"` replace `object-chart` and
+    // the block stopped resolving; stamping the discriminator last silently ate
+    // the author's value instead. Neither is acceptable (ADR-0078), so the
+    // discriminator wins the `type` slot and the author's value is preserved
+    // beside it under `specType` for the block to read
+    // (objectui#2880 / framework#3729).
+    const Wrapper: React.FC<any> = ({ children: _children, ...props }) => {
+      const specType = typeof props.type === 'string' && props.type !== tag ? props.type : undefined;
+      return React.createElement(SchemaRenderer as any, {
+        schema: { dataSource, ...props, ...(specType ? { specType } : {}), type: tag },
+      });
+    };
     Wrapper.displayName = name;
     scope[name] = Wrapper;
   }

packages/plugin-charts/src/AdvancedChartImpl.specConfig.test.tsx

@@ -0,0 +1,164 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * framework#3729 / objectui#2880 — the spec's `ChartConfig` presentation props
+ * are honored, not silently dropped.
+ *
+ * `showLegend`, `title`/`subtitle`, `yAxis[].min/max/format/logarithmic`,
+ * `series[].stack`, `series[].yAxis`, `annotations` and `interaction` were all
+ * declared by `ChartConfigSchema`, reached this renderer, and did nothing —
+ * exactly the silently-inert metadata ADR-0078 forbids. These pin the wiring
+ * at the DOM/prop level so a future refactor cannot quietly drop them again.
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, cleanup, screen } from '@testing-library/react';
+
+// Recharts' ResponsiveContainer measures via ResizeObserver, which reports 0×0
+// under the headless DOM, so nothing paints. Fix its size.
+vi.mock('recharts', async () => {
+  const actual = await vi.importActual<any>('recharts');
+  return {
+    ...actual,
+    ResponsiveContainer: ({ children }: any) =>
+      React.cloneElement(children, { width: 480, height: 320 }),
+  };
+});
+
+import AdvancedChartImpl from './AdvancedChartImpl';
+
+afterEach(cleanup);
+
+const DATA = [
+  { status: 'open', total: 120, rate: 0.4 },
+  { status: 'paid', total: 80, rate: 0.9 },
+];
+
+const base = {
+  chartType: 'bar' as const,
+  data: DATA,
+  xAxisKey: 'status',
+  series: [{ dataKey: 'total' }],
+  isAnimationActive: false,
+};
+
+describe('AdvancedChartImpl — spec ChartConfig chrome', () => {
+  it('renders title and subtitle above the plot', () => {
+    render(<AdvancedChartImpl {...base} title="Invoice value" subtitle="by status" />);
+    expect(screen.getByText('Invoice value')).toBeTruthy();
+    expect(screen.getByText('by status')).toBeTruthy();
+  });
+
+  it('adds no wrapper chrome when neither is set', () => {
+    const { container } = render(<AdvancedChartImpl {...base} />);
+    // ChartFrame is a passthrough with no title/subtitle, so the container's
+    // first child is still the chart itself.
+    expect(container.querySelector('.text-muted-foreground')).toBeNull();
+  });
+
+  it('honors showLegend: false', () => {
+    const { container: withLegend } = render(<AdvancedChartImpl {...base} />);
+    const legendCount = withLegend.querySelectorAll('.recharts-legend-wrapper').length;
+    cleanup();
+    const { container: without } = render(<AdvancedChartImpl {...base} showLegend={false} />);
+    expect(without.querySelectorAll('.recharts-legend-wrapper').length).toBeLessThan(legendCount);
+  });
+});
+
+describe('AdvancedChartImpl — spec ChartAxis', () => {
+  it('formats y ticks with the declared format instead of the compact default', () => {
+    const { container } = render(
+      <AdvancedChartImpl {...base} yAxes={[{ field: 'total', format: '$0,0.00' }]} />,
+    );
+    // Recharts 3 draws tick labels in their own z-index layer rather than
+    // inside the axis group, so read every <text> and look for the currency
+    // symbol — the compact default would render a bare "120".
+    const texts = Array.from(container.querySelectorAll('text')).map((t) => t.textContent ?? '');
+    expect(texts.some((t) => t.includes('$'))).toBe(true);
+  });
+
+  it('renders an axis title', () => {
+    render(<AdvancedChartImpl {...base} yAxes={[{ field: 'total', title: 'Revenue' }]} />);
+    expect(screen.getByText('Revenue')).toBeTruthy();
+  });
+
+  it('draws a second y-axis when two are declared', () => {
+    const { container: single } = render(<AdvancedChartImpl {...base} />);
+    const singleCount = single.querySelectorAll('.recharts-yAxis').length;
+    cleanup();
+    const { container: dual } = render(
+      <AdvancedChartImpl
+        {...base}
+        series={[{ dataKey: 'total' }, { dataKey: 'rate', yAxis: 'right' }]}
+        yAxes={[{ field: 'total' }, { field: 'rate', position: 'right' }]}
+      />,
+    );
+    expect(dual.querySelectorAll('.recharts-yAxis').length).toBeGreaterThan(singleCount);
+  });
+});
+
+describe('AdvancedChartImpl — spec series.stack', () => {
+  /** X offsets of every drawn bar (Recharts 3 draws bars as <path>). */
+  const barXs = (container: HTMLElement) =>
+    Array.from(container.querySelectorAll('.recharts-bar-rectangle path')).map((p) => p.getAttribute('x'));
+
+  it('stacks series sharing a stack group', () => {
+    // Recharts stacks by `stackId`. Stacked: both series of a category share
+    // one column band → 2 distinct x for 2 categories × 2 series. Grouped:
+    // the bars sit side by side → 4 distinct x.
+    const { container: stacked } = render(
+      <AdvancedChartImpl
+        {...base}
+        series={[
+          { dataKey: 'total', stack: 'g' },
+          { dataKey: 'rate', stack: 'g' },
+        ]}
+      />,
+    );
+    const stackedXs = barXs(stacked);
+    expect(stackedXs.length).toBe(4);
+    expect(new Set(stackedXs).size).toBe(2);
+    cleanup();
+
+    const { container: grouped } = render(
+      <AdvancedChartImpl {...base} series={[{ dataKey: 'total' }, { dataKey: 'rate' }]} />,
+    );
+    expect(new Set(barXs(grouped)).size).toBe(4);
+  });
+});
+
+describe('AdvancedChartImpl — spec annotations', () => {
+  it('draws a reference line for a line annotation', () => {
+    const { container } = render(
+      <AdvancedChartImpl {...base} annotations={[{ type: 'line', axis: 'y', value: 100, label: 'Target' }]} />,
+    );
+    expect(container.querySelectorAll('.recharts-reference-line').length).toBeGreaterThan(0);
+    expect(screen.getByText('Target')).toBeTruthy();
+  });
+
+  it('draws a reference area for a region annotation', () => {
+    const { container } = render(
+      <AdvancedChartImpl {...base} annotations={[{ type: 'region', axis: 'y', value: 50, endValue: 100 }]} />,
+    );
+    expect(container.querySelectorAll('.recharts-reference-area').length).toBeGreaterThan(0);
+  });
+
+  it('draws nothing extra when no annotation is declared', () => {
+    const { container } = render(<AdvancedChartImpl {...base} />);
+    expect(container.querySelectorAll('.recharts-reference-line').length).toBe(0);
+  });
+});
+
+describe('AdvancedChartImpl — spec interaction', () => {
+  it('adds the brush when interaction.brush is on', () => {
+    const { container } = render(<AdvancedChartImpl {...base} interaction={{ brush: true }} />);
+    expect(container.querySelectorAll('.recharts-brush').length).toBeGreaterThan(0);
+  });
+
+  it('omits the brush by default', () => {
+    const { container } = render(<AdvancedChartImpl {...base} />);
+    expect(container.querySelectorAll('.recharts-brush').length).toBe(0);
+  });
+});