diff --git a/.changeset/7744-chart-type-internal-only.md b/.changeset/7744-chart-type-internal-only.md new file mode 100644 index 0000000000..3d78e64162 --- /dev/null +++ b/.changeset/7744-chart-type-internal-only.md @@ -0,0 +1,23 @@ +--- +--- + +`@object-ui/plugin-charts`: documentation and a pin only — no published behaviour changes. +`normalizeSeries` returns the identical output for the identical input (measured: 336 input +cases, byte-identical JSON before and after; the probe was itself shown sensitive — a reader +reorder moves 49 of those 336). + +`normalizeSeries` resolves the per-series family as `str(raw.chartType) ?? str(raw.type)` — +the internal spelling FIRST. `chartType` is the carrier of the renderer's internal `dataKey` +series shape (`ChartRenderer`'s `series?` union declares it on that arm and on no other); +the AUTHORING face refuses it by name via `ChartDataSeriesSchema` (objectui#7694 / PR #7737). +Because the one function normalizes both shapes, an author who skips validation still gets +`chartType` honoured at runtime while the validator refuses that same key — the two faces +disagree. + +objectui#7744 ruled that the split is annotated and pinned, NOT retired at the reader: +retiring the limb would change what the internal producers (`DashboardRenderer`, `ObjectView`, +the dataset path) render, which is a reader-side decision of its own. So the limb keeps its +behaviour and gains a docblock that delegates the refusal's rule to where it lives, plus +`chartType-internal-only-7744.test.ts`, which pins the RELATIONSHIP the two files each held +half of: the key the validator refuses is the key the reader prefers. Bringing the faces into +agreement from either side now goes red and names the side that moved. diff --git a/packages/plugin-charts/src/__tests__/chartType-internal-only-7744.test.ts b/packages/plugin-charts/src/__tests__/chartType-internal-only-7744.test.ts new file mode 100644 index 0000000000..34bc6c2c84 --- /dev/null +++ b/packages/plugin-charts/src/__tests__/chartType-internal-only-7744.test.ts @@ -0,0 +1,145 @@ +/** + * 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. + */ + +/** + * objectui#7744 — `chartType` on a series is the INTERNAL spelling only, and + * the authoring face and the reader disagree about it ON PURPOSE. + * + * Two facts, each already true, each already load-bearing: + * + * - the AUTHORING face refuses `chartType` BY NAME — `ChartDataSeriesSchema` + * (`@object-ui/types`, objectui#7694 / PR #7737). Its rule, its message and + * its remedy are pinned in that package's + * `chart-series-chart-type-alias-refusal-7694.test.ts`; this file does not + * restate them, it only asks WHICH key is refused; + * - the READER honours it, and reads it FIRST — `normalizeSeries`' + * `str(raw.chartType) ?? str(raw.type)` — because the internal `dataKey` + * shape carries the per-series family under that name and + * `normalizeChartSchema` translates BOTH shapes through the one function. + * + * Neither fact is a defect, and until now NEITHER FILE PINNED THE PAIR — which + * is how the split survived to be found in the contract review of PR #7737. + * What this file pins is the RELATIONSHIP: the key the validator refuses is + * the key the reader prefers. Bring the two faces into agreement from either + * side — make the refusal accept, or reorder or delete the reader's first limb + * — and this goes red naming the side that moved. + * + * ⚠️ It is deliberately not written as "`chartType` is honoured somehow": an + * assertion loose enough to stay green under both behaviours pins nothing. + * + * ⛔ A green ablation on the reader's first limb is not a licence to delete it; + * the `normalizeSeries` docblock carries that measurement and why. + */ + +import { describe, it, expect } from 'vitest'; +import { ChartDataSeriesSchema } from '@object-ui/types/zod'; + +import { normalizeChartSchema, type NormalizedSeries } from '../normalizeChartSchema'; +import type { ChartRendererProps } from '../ChartRenderer'; + +/** + * ONE document, read by every assertion below, so the two faces are measured + * against the same input instead of against two fixtures free to drift apart. + * + * It writes BOTH spellings with DIFFERENT values, and that is what makes the + * reader's precedence observable at all: with only `chartType` written, a + * reordered reader returns the identical answer and a pin on the value alone + * stays green straight through the change it exists to catch. + */ +const AUTHORED = { name: 'margin', type: 'area', chartType: 'line' } as const; + +/** The same document with the refused spelling dropped — the accept control. */ +const AUTHORED_WITHOUT_ALIAS = { name: AUTHORED.name, type: AUTHORED.type } as const; + +/** `null` when the document parses; the refused paths otherwise. */ +const issuePathsOf = (input: unknown): string[] | null => { + const r = ChartDataSeriesSchema.safeParse(input); + return r.success ? null : r.error.issues.map((i) => i.path.join('.')); +}; + +const seriesOf = (raw: unknown): NormalizedSeries | undefined => + normalizeChartSchema({ series: [raw] }).series?.[0]; + +describe('objectui#7744 — the `chartType` split between the authoring face and the reader', () => { + it('the AUTHORING face refuses `chartType`, and that is the ONLY thing wrong with the document', () => { + expect( + issuePathsOf(AUTHORED), + 'objectui#7744: `ChartDataSeriesSchema` no longer refuses `chartType` by name on an authored ' + + 'series (objectui#7694 / PR #7737) — the AUTHORING face moved. If that is intended, the ' + + 'reader half of this split and the `normalizeSeries` docblock are the other half of the edit.', + ).toEqual(['chartType']); + }); + + it('CONTROL — the same series is ACCEPTED once `chartType` is dropped', () => { + expect( + issuePathsOf(AUTHORED_WITHOUT_ALIAS), + 'objectui#7744: the fixture is refused for something other than `chartType`, so the refusal ' + + 'asserted above is not measuring the key it names.', + ).toBeNull(); + }); + + it('the READER honours `chartType`, and takes it OVER the declared `type`', () => { + expect( + seriesOf(AUTHORED)?.chartType, + 'objectui#7744: `normalizeSeries` no longer resolves the per-series family from `chartType` ' + + 'first. `"area"` here means the limb was REORDERED; `undefined` means it was DELETED. ' + + 'Either one changes what the internal `dataKey`-shape producers render (DashboardRenderer / ' + + 'ObjectView / the dataset path) — that is a reader-side decision, not a doc fix.', + ).toBe('line'); + }); + + it('THE SPLIT — the key the validator refuses is the key the reader prefers', () => { + const [refused] = issuePathsOf(AUTHORED) ?? []; + const written = AUTHORED as unknown as Record; + + expect(refused, 'objectui#7744: nothing on this fixture is refused by name any more').toBe('chartType'); + expect( + written[refused], + 'the fixture must write the refused key and the declared key with DIFFERENT values, or the ' + + 'reader\'s precedence between them is not observable and this case pins nothing.', + ).not.toBe(written.type); + + expect( + seriesOf(AUTHORED)?.chartType, + 'objectui#7744: the reader no longer prefers the very key the authoring face refuses. The two ' + + 'faces have converged, and the `normalizeSeries` docblock now describes a split that is gone.', + ).toBe(written[refused]); + }); + + it('the internal `dataKey` shape is the limb\'s LEGITIMATE carrier — it is not the defect', () => { + expect( + seriesOf({ dataKey: 'margin', chartType: 'line' })?.chartType, + 'objectui#7744: the internal-shape producers lost their per-series family override.', + ).toBe('line'); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Compile-time half — this package's tsconfig.json includes its tests, so */ +/* `type-check` compiles these (the `spec-symbol-batch7.test.ts` precedent). */ +/* -------------------------------------------------------------------------- */ + +type Assert = T; +type HasKey = K extends keyof T ? true : false; + +type SeriesArm = NonNullable[number]; +type InternalArm = Extract; +type AuthoredArm = Extract; + +describe('objectui#7744 — `ChartRenderer` states the same split in TypeScript', () => { + it('is pinned at compile time (the assertion IS the compile)', () => { + // `series?` is a union of the two shapes. `chartType` is a member of the + // INTERNAL arm and of no other; the authored arm carries `type` instead. + // Move `chartType` onto the authored arm and these stop compiling. + type _ChartTypeIsInternal = Assert>; + type _ChartTypeIsNotAuthored = Assert extends false ? true : false>; + type _AuthoredArmDeclaresType = Assert>; + + expect(true).toBe(true); + }); +}); diff --git a/packages/plugin-charts/src/normalizeChartSchema.ts b/packages/plugin-charts/src/normalizeChartSchema.ts index 05a66dfb5b..0a2b702c01 100644 --- a/packages/plugin-charts/src/normalizeChartSchema.ts +++ b/packages/plugin-charts/src/normalizeChartSchema.ts @@ -226,8 +226,48 @@ function normalizeAxis(raw: unknown): NormalizedAxis | undefined { * Merge one series entry from either shape. * * Internal (`dataKey`) wins over spec (`name`) so a caller that already speaks - * the internal contract is untouched. `type` (spec, a ChartType) and - * `chartType` (internal) both name the per-series family in a combo chart. + * the internal contract is untouched. + * + * ## `chartType` is the INTERNAL spelling, and only that (objectui#7744) + * + * The two spellings of the per-series family are NOT two authoring keys: + * + * - `type` is the AUTHORING face's key — spec `ChartSeries.type`, declared on + * the mirror as `ChartDataSeriesSchema.type` (`'bar' | 'line' | 'area'`). + * - `chartType` is the RENDERER-INTERNAL carrier. It rides on the `dataKey`- + * shaped series that `DashboardRenderer`, `ObjectView` and the dataset path + * hand straight to `ChartRenderer`, which is where it is declared + * (`ChartRenderer.tsx:65`, on the internal series shape) and where + * {@link NormalizedSeries.chartType} is the output half of the same + * contract. On the AUTHORING face it is not a member at all: it is a named + * alias refusal — `ChartDataSeriesSchema.chartType` + * (`packages/types/src/zod/data-display.zod.ts`, objectui#7694 / PR #7737), + * pinned in that package's + * `__tests__/chart-series-chart-type-alias-refusal-7694.test.ts`. Read the + * rule and its remedy THERE. Restating either here would be a second copy of + * a rule that only one of the two files is ever edited with. + * + * ## The split this limb carries, deliberately (objectui#7744) + * + * This function normalizes BOTH shapes, and the `family` limb below cannot + * tell them apart: by the time `raw` arrives it is a bag of keys with nothing + * on it naming its producer. So an AUTHORED series that skipped validation + * still gets `chartType` honoured at runtime — and honoured OVER the declared + * `type`, which the limb reads second — while the validator refuses that same + * key by name. The two faces disagree, and that is the state this comment + * exists to make legible rather than to fix: the objectui#7744 ruling was to + * annotate and pin, NOT to retire the limb at the reader, because retiring it + * would change what the internal producers above render — a reader-side + * decision of its own. + * + * ⛔ Do not delete the limb on the strength of a green ablation. Deleting + * `str(raw.chartType) ??` left the repo fully green when it was measured + * (304 files / 5817 tests) while deleting its ` ?? str(raw.type)` sibling went + * 2 red — green there means only that no test PINNED it, never that no + * producer depends on it. Both halves of the split, and the precedence between + * them, are now pinned in `__tests__/chartType-internal-only-7744.test.ts`; + * that case is what goes red if the two faces are ever brought into agreement, + * from either side. */ function normalizeSeries(raw: unknown): NormalizedSeries | undefined { if (!isRec(raw)) { @@ -241,6 +281,8 @@ function normalizeSeries(raw: unknown): NormalizedSeries | undefined { const out: NormalizedSeries = { dataKey }; const lbl = label(raw.label); if (lbl) out.label = lbl; + // INTERNAL spelling FIRST, authored `type` second — see the docblock above. + // The order is load-bearing and pinned; neither limb is dead. const family = str(raw.chartType) ?? str(raw.type); if (family === 'bar' || family === 'line' || family === 'area') out.chartType = family; const variant = str(raw.variant);