Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions .changeset/objectchart-contract-back-to-spec-shape.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
'@objectstack/spec': minor
'@objectstack/lint': minor
---

`<ObjectChart>`'s author contract is the spec `ChartConfig` shape again (issue #3729)

#3701 trimmed `xAxis`/`yAxis`/`series` out of the `<ObjectChart>` contract
because the renderer read `xAxisKey`/`series[].dataKey` and silently dropped the
ChartConfig shapes — an honest record of the runtime gap, not the target state.
objectui#2880 closed the gap the other way round (the renderer now honors
`ChartConfig` through one normalization boundary), so the contract follows the
protocol again (ADR-0082 D1: the spec schema IS the protocol).

**Contract.** `type`, `xAxis`, `yAxis`, `series`, `subtitle`, `showDataLabels`,
`annotations` and `interaction` are published from `ChartConfigSchema`; the
internal `chartType`/`xAxisKey`/`series[].dataKey` spellings leave the author
contract. `annotations` and `interaction` gained the `.describe()` they never
had, so the generated contract stops publishing bare `object[]` with no meaning.

**The `type` exception.** `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 — an author writing `type="bar"` used to
replace `object-chart` and the block stopped resolving. The collision is created
by the flattening and is resolved there (objectui's react-page wrapper), so the
contract can publish `type` as the spec spells it. The contract generator's
blanket `type` skip is now overridable by an explicit `dataProps` allow-list,
since for this one block `type` is a real author prop.

**Lint.** `validate-react-page-props` reads the axes in the spec spelling —
`xAxis.field`, `yAxis[].field`, `series[].name` — and keeps accepting the
internal spellings silently, because dashboards and the console's own chart-view
wiring emit them. `react-chart-axis-inert` is retired: the props it warned about
are honored now, so the warning would be false. The three binding-integrity
rules from #3701 are unchanged.

**Spec.** `chart-aggregate.ts` records the constraint the whole result-column
convention rests on: an inline `aggregate` is SINGLE-MEASURE. Keying rows by the
raw field name only works because there is exactly one measure to key; two
measures over one field would collide, and resolving that needs an author-chosen
name per measure — which is what a dataset is. Widening `ChartAggregateSchema`
into a measures array would silently invalidate every axis binding these rules
validate, so the boundary is now written down rather than left to be rediscovered.

The chart taxonomy note is corrected too: grouped/stacked bar and stacked area
are absent from `ChartTypeSchema` not because they render as their base chart,
but because stacking is a property of the SERIES (`ChartSeries.stack`), not a
chart family — one `bar` family plus a series stack group expresses all three.
`ChartInteraction.zoom` is now marked declared-not-delivered in its own
description rather than reading as shipped.
15 changes: 7 additions & 8 deletions content/docs/deployment/validating-metadata.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -191,18 +191,17 @@ column, `field` names the value column (the literal `count` for a fieldless
two conventions up is the usual cause of a chart that renders axes and no bars.

```jsx
<ObjectChart objectName="invoice" chartType="bar"
<ObjectChart objectName="invoice" type="bar"
aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }}
xAxisKey="status" series={[{ dataKey: 'sum_total' }]} />
// ↑ a dataset-style measure name; the rows
// are keyed `total` → error
xAxis={{ field: 'status' }} series={[{ name: 'sum_total' }]} />
// ↑ a dataset-style measure name; the rows
// are keyed `total` → error
```

Both halves are checked: `aggregate.field` / `aggregate.groupBy` must be fields
the object declares, and `xAxisKey` / `series[].dataKey` must name a column the
aggregate actually returns (plus `<field>__comparison` when a comparison overlay
is on). `ChartConfig`'s `xAxis` / `yAxis` shapes are **not** read by this block
and are reported as inert (warning).
the object declares, and the axes — `xAxis.field`, `yAxis[].field`,
`series[].name` — must name a column the aggregate actually returns (plus
`<field>__comparison` when a comparison overlay is on).

Skipped, to keep false positives at zero: any prop whose value is not a static
literal (it comes from React state or a variable), a usage carrying a `{...spread}`,
Expand Down
10 changes: 5 additions & 5 deletions content/docs/references/ui/chart.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ Inline aggregation for an object-bound chart
| **height** | `number` | optional | Fixed height in pixels |
| **showLegend** | `boolean` | ✅ | Display legend |
| **showDataLabels** | `boolean` | ✅ | Display data labels |
| **annotations** | `{ type: Enum<'line' \| 'region'>; axis: Enum<'x' \| 'y'>; value: number \| string; endValue?: number \| string; … }[]` | optional | |
| **interaction** | `{ tooltips: boolean; zoom: boolean; brush: boolean; clickAction?: string }` | optional | |
| **annotations** | `{ type: Enum<'line' \| 'region'>; axis: Enum<'x' \| 'y'>; value: number \| string; endValue?: number \| string; … }[]` | optional | Reference lines/bands drawn over the plot: `{ type: "line" \| "region", axis: "x" \| "y", value, endValue?, color?, label?, style? }` |
| **interaction** | `{ tooltips: boolean; zoom: boolean; brush: boolean; clickAction?: string }` | optional | Interaction toggles: `{ tooltips?, brush?, zoom?, clickAction? }` |
| **aria** | `{ ariaLabel?: string; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes |


Expand Down Expand Up @@ -150,9 +150,9 @@ Type: `string`

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **tooltips** | `boolean` | ✅ | |
| **zoom** | `boolean` | ✅ | |
| **brush** | `boolean` | ✅ | |
| **tooltips** | `boolean` | ✅ | Show the hover tooltip |
| **zoom** | `boolean` | ✅ | Pan/zoom the plot — NOT implemented by the default renderer; use `brush` |
| **brush** | `boolean` | ✅ | Show the range selector under the plot |
| **clickAction** | `string` | optional | Action ID to trigger on click |


Expand Down
4 changes: 2 additions & 2 deletions content/docs/references/ui/report.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ const result = JoinedReportBlock.parse(data);
| **height** | `number` | optional | Fixed height in pixels |
| **showLegend** | `boolean` | ✅ | Display legend |
| **showDataLabels** | `boolean` | ✅ | Display data labels |
| **annotations** | `{ type: Enum<'line' \| 'region'>; axis: Enum<'x' \| 'y'>; value: number \| string; endValue?: number \| string; … }[]` | optional | |
| **interaction** | `{ tooltips: boolean; zoom: boolean; brush: boolean; clickAction?: string }` | optional | |
| **annotations** | `{ type: Enum<'line' \| 'region'>; axis: Enum<'x' \| 'y'>; value: number \| string; endValue?: number \| string; … }[]` | optional | Reference lines/bands drawn over the plot: `{ type: "line" \| "region", axis: "x" \| "y", value, endValue?, color?, label?, style? }` |
| **interaction** | `{ tooltips: boolean; zoom: boolean; brush: boolean; clickAction?: string }` | optional | Interaction toggles: `{ tooltips?, brush?, zoom?, clickAction? }` |
| **aria** | `{ ariaLabel?: string; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes |


Expand Down
11 changes: 6 additions & 5 deletions examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ import { definePage } from '@objectstack/spec/ui';
* cross-object reads declaratively (zero data code).
* (This comparison absorbed the former Account Cockpit page.)
*
* The chart's axes are bound explicitly (#3701) to show the object-bound
* result-column rule: an inline `aggregate` returns rows keyed by the RAW
* FIELD NAMES — `status` (its `groupBy`) and `total` (its `field`) — not by a
* dataset-style measure name. `os validate` now checks both halves.
* The chart is written in the spec `ChartConfig` shape (#3729) and its axes are
* bound explicitly (#3701) to show the object-bound result-column rule: an
* inline `aggregate` returns rows keyed by the RAW FIELD NAMES — `status` (its
* `groupBy`) and `total` (its `field`) — not by a dataset-style measure name.
* `os validate` checks both halves.
*
* Styling (ADR-0065): no Tailwind — inline `style={{}}` with `hsl(var(--token))`;
* data blocks and the drawer bring their own compiled styling. The drawer sets
Expand Down Expand Up @@ -119,7 +120,7 @@ function Page() {
<Stat label="Open AR" value={related.openInvoices} accent="hsl(38 92% 50%)" />
</div>

<ObjectChart objectName="showcase_invoice" chartType="bar" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxisKey="status" series={[{ dataKey: 'total', label: 'Invoice value' }]} title="Invoice value by status" showLegend={true} />
<ObjectChart objectName="showcase_invoice" type="bar" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxis={{ field: 'status' }} yAxis={[{ field: 'total', format: '$0,0' }]} series={[{ name: 'total', label: 'Invoice value' }]} title="Invoice value by status" showLegend={true} />

<RecordRelatedList objectName="showcase_account" recordId={sel} relationshipField="account" columns={['name', 'status', 'total']} limit={5} showViewAll={true} title="Invoices" />

Expand Down
1 change: 0 additions & 1 deletion packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ export {
REACT_CHART_FIELD_UNKNOWN,
REACT_CHART_AGGREGATE_INVALID,
REACT_CHART_AXIS_UNKNOWN,
REACT_CHART_AXIS_INERT,
} from './validate-react-page-props.js';
export type { ReactPropFinding, ReactPropSeverity } from './validate-react-page-props.js';
export { validatePageSourceStyling, PAGE_SOURCE_CLASSNAME } from './validate-page-source-styling.js';
Expand Down
40 changes: 34 additions & 6 deletions packages/lint/src/validate-react-page-props.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
REACT_CHART_FIELD_UNKNOWN,
REACT_CHART_AGGREGATE_INVALID,
REACT_CHART_AXIS_UNKNOWN,
REACT_CHART_AXIS_INERT,
} from './validate-react-page-props.js';

const page = (source: string) => ({ pages: [{ name: 'p', kind: 'react', source }] });
Expand Down Expand Up @@ -154,13 +153,42 @@ describe('validateReactPageProps — <ObjectChart> bindings (#3701)', () => {
expect(f).toEqual([]);
});

it('warns that the ChartConfig axis shapes are inert on this block', () => {
// ── the spec ChartConfig shape is the contract again (#3729) ────────────

it('accepts the spec axis shape', () => {
const f = validateReactPageProps(
chartPage(chart(`objectName="invoice" type="bar" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxis={{ field: 'status' }} yAxis={[{ field: 'total' }]}`)),
);
expect(f).toEqual([]);
});

it('flags a spec yAxis[].field that is not a result column', () => {
const f = validateReactPageProps(
chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} yAxis={[{ field: 'sum_total' }]}`)),
);
expect(f.some((x) => x.rule === REACT_CHART_AXIS_UNKNOWN && /sum_total/.test(x.message))).toBe(true);
});

it('flags a spec series[].name that is not a result column', () => {
const f = validateReactPageProps(
chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} yAxis={[{ field: 'total' }]}`)),
chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} series={[{ name: 'sum_total' }]}`)),
);
const inert = f.find((x) => x.rule === REACT_CHART_AXIS_INERT);
expect(inert?.severity).toBe('warning');
expect(inert?.hint).toMatch(/series=/);
const hit = f.find((x) => x.rule === REACT_CHART_AXIS_UNKNOWN);
expect(hit?.hint).toMatch(/series\[\]\.name/);
});

it('flags a spec xAxis.field bound to the VALUE column', () => {
const f = validateReactPageProps(
chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxis={{ field: 'total' }}`)),
);
expect(f.some((x) => x.rule === REACT_CHART_AXIS_UNKNOWN && /VALUE column/.test(x.message))).toBe(true);
});

it('still accepts the internal spelling — dashboards emit it', () => {
const f = validateReactPageProps(
chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxisKey="status" series={[{ dataKey: 'total' }]}`)),
);
expect(f).toEqual([]);
});

// ── false-positive guards ───────────────────────────────────────────────
Expand Down
64 changes: 37 additions & 27 deletions packages/lint/src/validate-react-page-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
// (e.g. `onSucces` → `onSuccess`) → warning. We do NOT flag arbitrary
// unknown props (the contract's data props are a curated subset) — only the
// likely typos, to keep false positives near zero.
// - <ObjectChart>'s data BINDINGS, by reading the attribute VALUES (#3701).
// See the block comment above `checkObjectChart` for why that block, and
// why only now.
// - <ObjectChart>'s data BINDINGS, by reading the attribute VALUES (#3701,
// retargeted to the spec shape in #3729). See the block comment above
// `checkObjectChart`.
//
// Reading values is opt-in per block and per prop: everything below evaluates
// only STATIC literals (`objectName="invoice"`, an `aggregate={{…}}` object
Expand Down Expand Up @@ -185,13 +185,20 @@ function attrValue(tsc: typeof ts, sf: ts.SourceFile, attr: ts.JsxAttribute): un
//
// * `aggregate.field` / `aggregate.groupBy` are RAW FIELD names → check them
// against the object's declared fields.
// * `xAxisKey` / `series[].dataKey` are RESULT COLUMN names → check them
// against what this aggregate produces.
// * the axes are RESULT COLUMN names → check them against what this
// aggregate produces.
//
// The axes are read in the SPEC spelling — `xAxis.field`, `yAxis[].field`,
// `series[].name` (#3729). #3701 had to read `xAxisKey`/`series[].dataKey`
// because those were the only spellings the renderer honored; objectui#2880
// made it honor ChartConfig, so the gate follows the protocol again. The
// internal spellings are still accepted, silently: dashboards and the console's
// own chart-view wiring emit them, and they remain a valid (if unpublished)
// way to write the same binding.

export const REACT_CHART_FIELD_UNKNOWN = 'react-chart-field-unknown';
export const REACT_CHART_AGGREGATE_INVALID = 'react-chart-aggregate-invalid';
export const REACT_CHART_AXIS_UNKNOWN = 'react-chart-axis-unknown';
export const REACT_CHART_AXIS_INERT = 'react-chart-axis-inert';

const CHART_FUNCTIONS = ['count', 'sum', 'avg', 'min', 'max'] as const;

Expand Down Expand Up @@ -261,21 +268,6 @@ function checkObjectChart(
const push = (severity: ReactPropSeverity, rule: string, message: string, hint: string) =>
findings.push({ severity, rule, where, path, message, hint });

// The ChartConfig axis shapes reach this block but are never read by it —
// it consumes `xAxisKey` + `series[].dataKey`. Silently inert props are what
// ADR-0078 forbids, so say so rather than let the chart render blank.
for (const inert of ['xAxis', 'yAxis'] as const) {
if (!values.has(inert)) continue;
push(
'warning',
REACT_CHART_AXIS_INERT,
`<ObjectChart> prop "${inert}" is not read by this block — it renders nothing.`,
inert === 'xAxis'
? 'Use xAxisKey="<result column>" (the aggregate\'s groupBy).'
: 'Use series={[{ dataKey: "<result column>" }]} (the aggregate\'s field).',
);
}

// Inline `data` wins over the aggregate query: the columns then come from
// the author's own rows, which this rule cannot see. Nothing further to say.
if (values.has('data')) return;
Expand Down Expand Up @@ -353,23 +345,41 @@ function checkObjectChart(
);
};

axisRef(strOf(values.get('xAxisKey')), 'xAxisKey');
// The category axis: spec `xAxis: { field }`, the report surface's bare
// string, or the internal `xAxisKey`. All three name the same column.
const xAxisRaw = values.get('xAxis');
const categoryAxis =
strOf(values.get('xAxisKey')) ??
strOf(xAxisRaw) ??
(isRec(xAxisRaw) ? strOf(xAxisRaw.field) : undefined);
const categoryProp = values.has('xAxisKey') ? 'xAxisKey' : 'xAxis.field';
axisRef(categoryAxis, categoryProp);

// The value axes: spec `yAxis: [{ field }]` (or a single object / bare
// string) and spec `series: [{ name }]` / internal `series: [{ dataKey }]`.
const yAxisRaw = values.get('yAxis');
const yAxisList = Array.isArray(yAxisRaw) ? yAxisRaw : yAxisRaw !== undefined ? [yAxisRaw] : [];
for (const a of yAxisList) {
axisRef(strOf(a) ?? (isRec(a) ? strOf(a.field) : undefined), 'yAxis[].field');
}

const series = values.get('series');
if (Array.isArray(series)) {
for (const s of series) {
if (isRec(s)) axisRef(strOf(s.dataKey), 'series[].dataKey');
if (!isRec(s)) continue;
const dataKey = strOf(s.dataKey);
axisRef(dataKey ?? strOf(s.name), dataKey ? 'series[].dataKey' : 'series[].name');
}
}

// A category axis bound to anything but the groupBy is always wrong, and the
// check above lets it through when it happens to equal the VALUE column.
const xAxisKey = strOf(values.get('xAxisKey'));
if (xAxisKey && keys.category && xAxisKey !== keys.category && xAxisKey === keys.value) {
if (categoryAxis && keys.category && categoryAxis !== keys.category && categoryAxis === keys.value) {
push(
'error',
REACT_CHART_AXIS_UNKNOWN,
`xAxisKey "${xAxisKey}" is the aggregate's VALUE column, not its category column.`,
`The category axis is keyed by groupBy — use xAxisKey="${keys.category}".`,
`${categoryProp} "${categoryAxis}" is the aggregate's VALUE column, not its category column.`,
`The category axis is keyed by groupBy — bind it to "${keys.category}".`,
);
}
}
Expand Down
11 changes: 9 additions & 2 deletions packages/spec/scripts/build-react-blocks-contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,15 @@ function dataProps(schema: any, allow?: string[]): Prop[] {
}
const props = js?.properties ?? {};
const required: string[] = Array.isArray(js?.required) ? js.required : [];
const SKIP = new Set(['aria', 'type', 'id', 'className', 'style']);
let entries = Object.entries(props).filter(([name]) => !SKIP.has(name));
// Envelope/plumbing props that are never part of an author contract. `type`
// is the awkward one: for most blocks it IS the SDUI component discriminator,
// but `ChartConfig.type` is the chart family and a real author prop — so an
// explicit `dataProps` allow-list wins over the skip (#3729).
const SKIP = new Set(['aria', 'id', 'className', 'style']);
const allowed = new Set(allow ?? []);
let entries = Object.entries(props).filter(
([name]) => !(SKIP.has(name) || (name === 'type' && !allowed.has('type'))),
);
if (allow && allow.length) {
const order = new Map(allow.map((n, i) => [n, i]));
entries = entries.filter(([n]) => order.has(n)).sort((a, b) => order.get(a[0])! - order.get(b[0])!);
Expand Down
14 changes: 14 additions & 0 deletions packages/spec/src/ui/chart-aggregate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@
* `chart-measure-unknown` catches. The two paths key their rows differently
* because only one of them has an author-chosen name to key by.
*
* ## The constraint this convention rests on: inline aggregate is SINGLE-MEASURE
*
* Keying rows by the raw field name only works because an inline `aggregate`
* computes exactly ONE measure — one `function` over one `field`. Two measures
* over the same field (`sum(amount)` and `avg(amount)`) would collide on the
* column name, and the only way out would be a derived name (`sum_amount`) —
* i.e. re-inventing what a dataset `DatasetMeasure.name` already is.
*
* So do not "just add" a measures array here. Multi-measure IS the dataset
* path's job, and the boundary between the two is exactly this: a chart that
* needs more than one measure needs an author-chosen name per measure, which
* means it needs a dataset (ADR-0021 Level B). Widening `ChartAggregateSchema`
* would silently invalidate every axis binding this convention validates.
*
* ## No business logic here (Prime Directive #2)
*
* The helpers below are pure contract derivations — they map a declaration to
Expand Down
Loading
Loading