diff --git a/.changeset/10608-object-chart-legacy-axis-keys-retired.md b/.changeset/10608-object-chart-legacy-axis-keys-retired.md new file mode 100644 index 0000000000..52ca9e35d9 --- /dev/null +++ b/.changeset/10608-object-chart-legacy-axis-keys-retired.md @@ -0,0 +1,45 @@ +--- +'@object-ui/types': minor +--- + +feat(types)!: `ObjectChartSchema.xAxisField` / `yAxisFields` / `aggregation` are retired on the `object-chart` node, each refused by name with the spec spelling as its remedy + +⚠️ Breaking, marked `minor` under this repo's version-alignment rule (a `major` +in the fixed group would move all of it off the `@objectstack` major). An +`object-chart` node that authors any of the three keys now FAILS to validate, +and a TypeScript literal typed as `ObjectChartSchema` that sets one no longer +compiles. + +The three are the LIST-VIEW chart block's vocabulary. The list-view relays +translate that block into `aggregate` / `xAxisKey` / `series` before they +compose an `object-chart` node, and nothing on the node's own render path +reads them: a node written with `xAxisField: 'status'` and +`yAxisFields: ['amount']` over static rows reached the chart with no category +key and no series, and on the inline `objectName` path it drew the +"no category axis" refusal. Yet both published faces declared the keys, so the +node type-checked and parsed green through `objectui validate` — every signal +said the axis was bound. + +Each key is now a `?: never` tombstone on the TypeScript face and a +`retirementTombstone()` on the zod mirror (ADR-0049; `BaseSchema` is +`.passthrough()`, so deleting the declarations would have KEPT an authored +value in silence rather than refused it). The refusal is an `invalid_type` +issue at the key's own path whose message names the key and the spelling to +write instead: + +- `xAxisField` → `xAxis: { field: 'status' }` (on the inline `objectName` + path the category is `aggregate.groupBy`); +- `yAxisFields` → `yAxis: [{ field: 'amount' }]`, one entry per value axis + (on the inline `objectName` path the measure is `aggregate.field`, and a + dataset-bound chart selects `values` by name); +- `aggregation` → `aggregate: { field, function, groupBy }` with the spec's + `function` vocabulary (a dataset-bound chart takes its aggregation from the + dataset's measures). + +Not touched: the list-view chart carriers keep these names — they are a +different node and they read them. A one-time producer census, recorded on the +card's pull request and not re-derived here, found no producer writing the +three on an `object-chart` node — in this repository, in the objectstack +showcase, or in hotcrm. + +Pinned in `packages/types/src/__tests__/object-chart-legacy-axis-keys-retired-10608.test.ts`. diff --git a/packages/types/src/__tests__/object-chart-legacy-axis-keys-retired-10608.test.ts b/packages/types/src/__tests__/object-chart-legacy-axis-keys-retired-10608.test.ts new file mode 100644 index 0000000000..0ba5fa7d55 --- /dev/null +++ b/packages/types/src/__tests__/object-chart-legacy-axis-keys-retired-10608.test.ts @@ -0,0 +1,235 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Retirement pin — `ObjectChartSchema.xAxisField` / `yAxisFields` / + * `aggregation` are REFUSED on the `object-chart` node, each by name with the + * spec spelling as its remedy (objectui#10608, ADR-0049 enforce-or-remove). + * + * ## The failure this pin exists to prevent + * + * The three are the LIST-VIEW chart block's vocabulary. The list-view relays + * translate that block into `aggregate` / `xAxisKey` / `series` before they + * compose an `object-chart` node, and nothing on the node's own render path + * reads them — so both published faces declared three keys that type-checked, + * parsed green through `objectui validate`, and drew no category axis and no + * series. An author (or an AI author reading the `.d.ts`) had every signal that + * `xAxisField: 'status'` was the way to bind the axis, and got an empty chart. + * The render reading and the producer census are recorded on the card's PR, as + * a one-time measurement; this file is the instrument that keeps the contract. + * + * ## Why tombstones and not deletions + * + * `BaseSchema` is `.passthrough()` on the zod side and carries an index + * signature on the TS side, so an UNDECLARED key is not refused, it is KEPT. + * Deleting the three members would hand the authored spelling exactly the + * silent no-op this card closes; block (d) pins that consequence on a + * misspelling, so the reason is a reading and not prose. `?: never` + + * `retirementTombstone()` is this package's convention — the precedent this + * follows most closely is `ChatbotSchema.userAvatar` / `height` + * (`chatbot-dark-keys-retired-7703.test.ts`): a dead spelling beside a live + * one, refused with the live one as the remedy. + * + * ## How refusals are asserted + * + * By the issue ENVELOPE — `code` and `path`, and that it is the ONLY issue, so + * a refusal cannot ride on some other failure of the same document — at the + * public door `safeValidateSchema` (what `objectui validate` / `objectui check` + * run) and on the mirror. The message is asserted to carry the remedy's own + * spelling, which IS the contract here (triage's execution note: tombstones + * naming the spec spelling); nothing else of its wording is pinned. + * + * ## The lit controls + * + * Block (b): the spec spelling of the same intent parses on the same node, at + * the same door. Block (e): the LIST-VIEW carrier that does read these names + * (the `options.chart` bag) still accepts them — the retirement is scoped to + * this node, and that boundary is pinned rather than stated. + * + * The `@ts-expect-error` directives in block (f) are REAL enforcement: this + * package type-checks its tests through `tsconfig.test.json`, so re-widening a + * member fails the build on the unused directive (TS2578). A green `vitest` + * run is NOT evidence about them — type assertions are erased before it runs. + */ + +import { describe, it, expect } from 'vitest'; +import { ChartAggregateSchema as SpecChartAggregateSchema } from '@objectstack/spec/ui'; +import type { ObjectChartSchema } from '../objectql'; +import { ObjectChartSchema as ObjectChartMirror } from '../zod/objectql.zod'; +import { ListViewSchema, safeValidateSchema } from '../zod/index.zod'; + +const CHART = { type: 'object-chart', chartType: 'bar' } as const; +const chart = (extra: Record) => ({ ...CHART, ...extra }); + +/** + * The retired keys, each with the values an author would plausibly have written + * (every arm of the retired declaration, so a partial re-widening shows up) and + * the remedy's own spelling, which the refusal must carry. + */ +const RETIRED = { + xAxisField: { + values: ['status'], + remedy: "xAxis: { field: 'status' }", + }, + yAxisFields: { + values: [['amount'], ['amount', 'count']], + remedy: "yAxis: [{ field: 'amount' }]", + }, + aggregation: { + values: ['cardinality', 'sum', 'avg', 'min', 'max'], + remedy: 'aggregate: { field, function, groupBy }', + }, +} as const; + +type RetiredKey = keyof typeof RETIRED; +const RETIRED_KEYS = Object.keys(RETIRED) as RetiredKey[]; + +type Issue = { code: string; path: PropertyKey[]; message: string }; +const issuesOf = (r: { success: boolean; error?: { issues: Issue[] } }): Issue[] => r.error?.issues ?? []; + +const mirrorShape = (ObjectChartMirror as unknown as { shape: Record }).shape; + +/* ── (a) each key is refused by name, with the spec spelling as the remedy ── */ + +describe.each(RETIRED_KEYS)('objectui#10608 (a) — `%s` is RETIRED on `object-chart`', (key) => { + const { values, remedy } = RETIRED[key]; + + it.each(values.map((v) => [JSON.stringify(v), v] as const))( + 'refuses `%s` at the public door and on the mirror: ONE `invalid_type` issue at the key, naming the remedy', + (_label, value) => { + for (const r of [safeValidateSchema(chart({ [key]: value })), ObjectChartMirror.safeParse(chart({ [key]: value }))]) { + expect(r.success, `an authored \`${key}: ${JSON.stringify(value)}\` was ACCEPTED`).toBe(false); + const issues = issuesOf(r); + expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([{ code: 'invalid_type', path: [key] }]); + expect(issues[0]!.message).toContain(`\`${key}\``); + expect(issues[0]!.message).toContain(remedy); + } + }, + ); + + it('the refusal message and the published `.describe()` metadata are ONE string', () => { + const r = ObjectChartMirror.safeParse(chart({ [key]: values[0] })); + expect(issuesOf(r)[0]!.message).toBe(mirrorShape[key]!.description); + }); + + it('stays DECLARED on the mirror — a tombstone, not a deletion (see block d)', () => { + expect(Object.keys(mirrorShape)).toContain(key); + }); +}); + +/* ── (b) LIT CONTROL: the spec spelling of the same intent parses ─────────── */ + +describe('objectui#10608 (b) — LIT CONTROL: each remedy parses on the same node, at the same door', () => { + it.each([ + ['`xAxis: { field }` for `xAxisField`', { xAxis: { field: 'status' } }], + ['`yAxis: [{ field }]` for `yAxisFields`', { yAxis: [{ field: 'amount' }] }], + ['`aggregate: { field, function, groupBy }` for `aggregation`', { objectName: 'task', aggregate: { field: 'amount', function: 'sum', groupBy: 'status' } }], + ])('%s', (_label, extra) => { + expect(issuesOf(safeValidateSchema(chart(extra)))).toEqual([]); + expect(issuesOf(ObjectChartMirror.safeParse(chart(extra)))).toEqual([]); + }); + + it('all three remedies together, on one node', () => { + const node = chart({ + objectName: 'task', + aggregate: { field: 'amount', function: 'sum', groupBy: 'status' }, + xAxis: { field: 'status' }, + yAxis: [{ field: 'amount' }], + }); + expect(safeValidateSchema(node).success).toBe(true); + }); +}); + +/* ── (c) the `aggregation` remedy's vocabulary is the spec's, read live ───── */ + +describe('objectui#10608 (c) — the `function` vocabulary the `aggregation` refusal lists is the spec\'s own', () => { + it('equals `ChartAggregateSchema.shape.function`\'s options, read off the installed spec', () => { + const message = mirrorShape.aggregation!.description!; + const listed = message.slice(message.indexOf('`function` one of')).split(';')[0]!.match(/`([a-z_]+)`/g)! + .map((t) => t.slice(1, -1)) + .filter((t) => t !== 'function'); + const specFns = (SpecChartAggregateSchema.shape.function as unknown as { options: string[] }).options; + expect(listed.length).toBeGreaterThan(0); + expect([...listed].sort()).toEqual([...specFns].sort()); + }); +}); + +/* ── (d) why a tombstone: an undeclared key would be KEPT, not refused ────── */ + +describe('objectui#10608 (d) — CONTROL: an undeclared spelling is KEPT by `.passthrough()`', () => { + it('a misspelled `xAxisFeld` parses green and survives the parse — what a deletion would have left', () => { + const r = ObjectChartMirror.safeParse(chart({ xAxisFeld: 'status' })); + expect(r.success).toBe(true); + expect((r.data as Record).xAxisFeld).toBe('status'); + }); +}); + +/* ── (e) the boundary: the LIST-VIEW carrier keeps these names ────────────── */ + +describe('objectui#10608 (e) — CONTROL: the list-view carrier still takes these names', () => { + /* + * The carrier the authoring door still admits them on is the legacy + * `options.chart` bag — the one `resolveListChartBinding` reads beside + * `chart` and translates before it composes an `object-chart` node. (The + * spec's own `chart` block is dataset-only and refuses them already; that is + * the spec's ruling, not this card's, and is not pinned here.) + */ + it('a `chart` list view authoring `xAxisField` / `yAxisFields` / `aggregation` in `options.chart` parses, on both doors', () => { + const view = { + type: 'list-view', + objectName: 'task', + columns: ['name'], + viewType: 'chart', + options: { chart: { chartType: 'bar', xAxisField: 'status', yAxisFields: ['estimate'], aggregation: 'sum' } }, + }; + expect(issuesOf(ListViewSchema.safeParse(view))).toEqual([]); + expect(issuesOf(safeValidateSchema(view))).toEqual([]); + }); +}); + +/* ── (f) the TS twin carries the same contract ───────────────────────────── */ + +type Equal = + (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false; +type Expect = T; + +/** `Equal`, not `extends`: an UNDECLARED member reads `any` through the index signature. */ +export type assertionXAxisFieldRetired = Expect>; +export type assertionYAxisFieldsRetired = Expect>; +export type assertionAggregationRetired = Expect>; +/** The helper can FAIL — synthetic control (an undeclared key reads `any`). */ +export type assertionEqualCanFail = Expect, false>>; + +describe('objectui#10608 (f) — the TS twin refuses what the mirror refuses', () => { + it('each retired key is a compile error — checked by `tsc -p tsconfig.test.json`', () => { + // Not `as const`: a readonly literal would fail every line below for a + // reason of its own, and each `@ts-expect-error` would pass without testing. + const base = { type: 'object-chart' as const, chartType: 'bar' as const }; + const refused: ObjectChartSchema[] = [ + // @ts-expect-error — retired: write `xAxis: { field }` + { ...base, xAxisField: 'status' }, + // @ts-expect-error — retired: write `yAxis: [{ field }]` + { ...base, yAxisFields: ['amount'] }, + // @ts-expect-error — retired: write `aggregate: { field, function, groupBy }` + { ...base, aggregation: 'sum' }, + ]; + expect(refused).toHaveLength(3); + }); + + it('CONTROL — the spec spelling compiles on the same node', () => { + const node: ObjectChartSchema = { + type: 'object-chart', + chartType: 'bar', + objectName: 'task', + aggregate: { field: 'amount', function: 'sum', groupBy: 'status' }, + xAxis: { field: 'status' }, + yAxis: [{ field: 'amount' }], + }; + expect(node.xAxis?.field).toBe('status'); + }); +}); diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index 06024d7291..60314327f5 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -4202,10 +4202,11 @@ export type KanbanConditionalFormattingRule = * `FilterArray` in its react-blocks prop table, and this component's * registry `inputs` advertises `{ name: 'filter', type: 'array' }`. * - `xAxisKey` — INTERNAL (relay-composed). `ChartRendererProps` calls it - * "Internal binding. Authors write the spec `xAxis: { field }`"; the - * author-facing spelling ON THIS NODE is `xAxisField` above. All five - * producers COMPUTE it (`dims[0]`, `chartCategoryKey(...)`), none forwards - * an authored value, and it is absent from the registry `inputs`. + * "Internal binding. Authors write the spec `xAxis: { field }`", and that + * spec object ({@link ObjectChartSchema.xAxis}) is the author-facing + * spelling ON THIS NODE. All five producers COMPUTE it (`dims[0]`, + * `chartCategoryKey(...)`), none forwards an authored value, and it is + * absent from the registry `inputs`. * - `series` — INTERNAL (relay-composed). The `{ dataKey }` shape below is * the renderer's internal contract; the spec's author-facing * `ChartSeriesSchema` REFUSES `dataKey` by name (`dataKey` → `name` @@ -4272,6 +4273,22 @@ export type KanbanConditionalFormattingRule = * them — as reads or as exceptions — and neither was moved. Their pin is * `__tests__/object-chart-axis-config-10518.test.ts`; the render path was * measured once with a real registry probe, recorded on the card's PR. + * + * ## Three list-view spellings, RETIRED on this node (objectui#10608, ADR-0049) + * + * `xAxisField`, `yAxisFields` and `aggregation` are the LIST-VIEW chart + * block's vocabulary (`chart.xAxisField` and its siblings on a `chart` list + * view). The list-view relays translate that block into `aggregate` / + * `xAxisKey` / `series` before they compose this node, so on this node the + * three were declared and read by nothing: a node written with them drew no + * category axis and no series. Each is now a `?: never` tombstone whose zod + * twin refuses it by name with the spec spelling as the remedy — `xAxis: + * { field }`, `yAxis: [{ field }]`, `aggregate: { field, function, groupBy }`. + * The measurement and the producer census are on the card's PR; the pin is + * `__tests__/object-chart-legacy-axis-keys-retired-10608.test.ts`. + * + * ⛔ The LIST-VIEW carriers keep these names: they are a different node, they + * read them, and nothing here touches them. */ export interface ObjectChartSchema extends BaseSchema { type: 'object-chart'; @@ -4280,12 +4297,39 @@ export interface ObjectChartSchema extends BaseSchema { /** Chart type. Includes donut / horizontal-bar / column — all rendered by * AdvancedChartImpl (previously only reachable by passing an untyped string). */ chartType: 'bar' | 'column' | 'horizontal-bar' | 'line' | 'area' | 'pie' | 'donut' | 'scatter'; - /** Field for X axis (categories) — legacy inline path */ - xAxisField?: string; - /** Fields for Y axis (values) — legacy */ - yAxisFields?: string[]; - /** Aggregation function — legacy */ - aggregation?: 'cardinality' | 'sum' | 'avg' | 'min' | 'max'; + /** + * RETIRED (objectui#10608, ADR-0049) — the list-view chart block's spelling, + * read by no `object-chart` reader. Write the spec's {@link xAxis} object, + * `xAxis: { field: 'status' }`; on the inline `objectName` path the category + * is `aggregate.groupBy`. + * + * A tombstone rather than a deletion: `BaseSchema`'s index signature and the + * mirror's `.passthrough()` would otherwise KEEP an authored value in silence. + * + * @deprecated Not a key this node reads. Write `xAxis: { field }`. + */ + xAxisField?: never; + /** + * RETIRED (objectui#10608, ADR-0049) — the list-view chart block's spelling, + * read by no `object-chart` reader. Write the spec's {@link yAxis} list, + * `yAxis: [{ field: 'amount' }]` — one entry per value axis, and each entry's + * `field` is a plotted column when no `series` is written. On the inline + * `objectName` path the measure is `aggregate.field`; a dataset-bound chart + * selects {@link values} by name. + * + * @deprecated Not a key this node reads. Write `yAxis: [{ field }]`. + */ + yAxisFields?: never; + /** + * RETIRED (objectui#10608, ADR-0049) — the list-view chart block's spelling, + * read by no `object-chart` reader. Write the spec's {@link aggregate}, + * `aggregate: { field, function, groupBy }`, whose `function` is the spec's + * own vocabulary; a dataset-bound chart takes its aggregation from the + * dataset's measures. + * + * @deprecated Not a key this node reads. Write `aggregate: { field, function, groupBy }`. + */ + aggregation?: never; /** Semantic-layer dataset name (ADR-0021, #1890) */ dataset?: string; /** Dataset dimension names */ @@ -4389,9 +4433,9 @@ export interface ObjectChartSchema extends BaseSchema { aggregate?: ChartAggregate; /** * INTERNAL (relay-composed) — the category column the renderer binds the x - * axis to. Authors write `xAxisField` above (or the spec's {@link xAxis} - * object below, whose `field` `normalizeChartSchema` resolves); the five - * producers of an `object-chart` node compute this key. + * axis to. Authors write the spec's {@link xAxis} object below, whose `field` + * `normalizeChartSchema` resolves (the retired {@link xAxisField} above is + * refused); the five producers of an `object-chart` node compute this key. * * Typed `string` from `ChartRendererProps.schema.xAxisKey`, the read this * value ends at. @@ -4564,9 +4608,8 @@ export interface ObjectChartSchema extends BaseSchema { * single axis object or a bare column name, which the normalizer tolerates, * is refused by the mirror and does not compile here. * - * ⚠️ Declared beside {@link yAxisFields} above, a separate legacy - * vocabulary. Whether that key is live on this node is reported on - * objectui#10518 for its own ruling; this member settles nothing about it. + * The list-view spelling {@link yAxisFields} above is RETIRED on this node + * (objectui#10608): this list is the one author-facing value-axis spelling. */ yAxis?: SpecChartAxis[]; } diff --git a/packages/types/src/zod/objectql.zod.ts b/packages/types/src/zod/objectql.zod.ts index 69a991aca8..7c93158af6 100644 --- a/packages/types/src/zod/objectql.zod.ts +++ b/packages/types/src/zod/objectql.zod.ts @@ -2462,9 +2462,33 @@ export const ObjectChartSchema = BaseSchema.extend({ // instead bind to a semantic-layer dataset (ADR-0021, #1890). objectName: z.string().optional().describe('ObjectQL object name (legacy inline path)'), chartType: z.enum(['bar', 'column', 'horizontal-bar', 'line', 'area', 'pie', 'donut', 'scatter']).describe('Chart type'), - xAxisField: z.string().optional().describe('X axis field (legacy inline path)'), - yAxisFields: z.array(z.string()).optional().describe('Y axis fields (legacy)'), - aggregation: z.enum(['cardinality', 'sum', 'avg', 'min', 'max']).optional().describe('Aggregation (legacy)'), + // ── objectui#10608: three list-view spellings, RETIRED on this node ── + // + // `xAxisField` / `yAxisFields` / `aggregation` are the LIST-VIEW chart + // block's vocabulary; the list-view relays translate that block before they + // compose this node, and no `object-chart` reader consumes the three. Kept + // declared and unwritable (ADR-0049): `BaseSchema` is `.passthrough()`, so a + // deleted arm would KEEP an authored value in silence instead of refusing it. + // Each refusal names the spec spelling as its remedy. The TS twin in + // `../objectql.ts` carries the ground; the pin is + // `../__tests__/object-chart-legacy-axis-keys-retired-10608.test.ts`. + xAxisField: retirementTombstone( + 'RETIRED (objectui#10608, ADR-0049) — `xAxisField` is the list-view chart block\'s spelling and no ' + + '`object-chart` reader consumes it: a node written with it draws no category axis. Write the spec\'s ' + + '`xAxis: { field: \'status\' }` instead; on the inline `objectName` path the category is `aggregate.groupBy`.', + ), + yAxisFields: retirementTombstone( + 'RETIRED (objectui#10608, ADR-0049) — `yAxisFields` is the list-view chart block\'s spelling and no ' + + '`object-chart` reader consumes it: a node written with it plots no series. Write the spec\'s ' + + '`yAxis: [{ field: \'amount\' }]` instead, one entry per value axis; on the inline `objectName` path the ' + + 'measure is `aggregate.field`, and a dataset-bound chart selects `values` by name.', + ), + aggregation: retirementTombstone( + 'RETIRED (objectui#10608, ADR-0049) — `aggregation` is the list-view chart block\'s spelling and no ' + + '`object-chart` reader consumes it: a node written with it aggregates nothing. Write the spec\'s ' + + '`aggregate: { field, function, groupBy }` instead, `function` one of `count`, `sum`, `avg`, `min`, ' + + '`max`; a dataset-bound chart takes its aggregation from the dataset\'s measures.', + ), // ADR-0021 semantic-layer binding: dimensions/measures selected BY NAME from a // dataset, queried via the governed queryDataset path. dataset: z.string().optional().describe('Semantic-layer dataset name (ADR-0021)'), @@ -2512,7 +2536,7 @@ export const ObjectChartSchema = BaseSchema.extend({ // authoring door here and at the react-page publish gate are one shape. aggregate: stripImportedDefaults(SpecChartAggregateSchema).optional() .describe('AUTHORABLE — inline aggregation for the legacy objectName path. @objectstack/spec ChartAggregateSchema ({ field?, function, groupBy }), the same schema the react-page publish gate parses: function and groupBy are REQUIRED, field is optional because only count counts rows rather than a column, and unknown keys are refused rather than dropped'), - xAxisKey: z.string().optional().describe('INTERNAL (relay-composed) — the category column the renderer binds the x axis to. Authors write xAxisField (or the spec xAxis: { field } one layer down); all five producers compute this key'), + xAxisKey: z.string().optional().describe('INTERNAL (relay-composed) — the category column the renderer binds the x axis to. Authors write the spec xAxis: { field }; all five producers compute this key'), series: z.array(z.object({ dataKey: z.string().describe('Result column this series plots'), label: z.string().optional().describe('Series display label'),