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
20 changes: 20 additions & 0 deletions .changeset/8086-chart-series-type-arm.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@object-ui/plugin-charts': minor
---

`ChartRendererProps.schema.series`: the `dataKey` arm now declares `type?: string`, the
per-series family override the `name` arm already declares, with the same member type
(objectui#8086). The TypeScript face now states what `ChartDataSeriesSchema` (where
`dataKey` and `name` are each independently optional beside `type`) and the renderer
(every entry goes through `normalizeSeries`, objectui#7681) accept. No other arm changes:
`chartType` stays on the `dataKey` arm alone, and wins when an entry writes both.

The accept set only widens. A value typed as the `dataKey` arm may now carry `type`, and
an entry narrowed with `'dataKey' in entry` may read it; nothing that compiled before is
refused. `ChartRendererProps` is not re-exported by name from the package entry: it
reaches consumers as the props type of the exported `ChartRenderer` component. An object
literal written against the whole `series` union was already accepted before this
change, because `type` is known to the `name` arm; the gap was at the arm.

The `ObjectChartSchema.series` docblock in `@object-ui/types` is restated to match: that
copy of the internal arm does not carry `type`, and its type is unchanged.
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* 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#8086 — the `dataKey` arm of `ChartRendererProps.schema.series`
* declares the per-series family override `type`, with the `name` arm's own
* member type.
*
* Ruling (align): the published TS face states what the contract
* (`ChartDataSeriesSchema`, where `dataKey` and `name` are each independently
* optional beside `type`) and the renderer (every entry goes through
* `normalizeSeries`, objectui#7681) already accept. No other arm changes.
*
* ## Why every pin is ARM-level, not union-level
*
* `series` is a union with no literal discriminant, so TypeScript's excess-
* property check on an object literal only asks whether each key is known to
* SOME arm. A `{ dataKey, type }` literal therefore type-checked against the
* whole union before this card too (`type` is known to the `name` arm), and a
* `{ name, chartType }` literal type-checks against it today (`chartType` is
* known to the `dataKey` arm). A union-level assertion would be green on both
* sides of the change and pin nothing. What was missing is the `dataKey` ARM's
* member: a value typed as that arm could not carry `type`, and an entry
* narrowed to it with `'dataKey' in entry` could not read it.
*
* `tsconfig.test.json` compiles this file, so each statement is enforcement:
* without the member, the `Equal` pin and the narrowed read fail (TS2339) and
* the arm-typed literal fails (TS2353); the `@ts-expect-error` fails the build
* (TS2578) the moment its refusal stops happening.
*/

import { describe, it, expect } from 'vitest';
import type { ChartRendererProps } from './ChartRenderer';

type Equal<A, B> =
(<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
type Expect<T extends true> = T;

type SeriesEntry = NonNullable<ChartRendererProps['schema']['series']>[number];
type DataKeyArm = Extract<SeriesEntry, { dataKey: string }>;
type NameArm = Extract<SeriesEntry, { name: string }>;

/** The ruling's letter: the `dataKey` arm's `type` is the `name` arm's `type`, member for member. */
export type assertionDataKeyArmTypeMatchesNameArm = Expect<Equal<DataKeyArm['type'], NameArm['type']>>;
/** The helper can FAIL — a synthetic control, so a vacuous `Equal` cannot pass this file. */
export type assertionEqualCanFail = Expect<Equal<Equal<NameArm['type'], number | undefined>, false>>;

describe('ChartRendererProps.series — the `dataKey` arm declares `type` (objectui#8086)', () => {
it('accepts `type` on a value typed as the `dataKey` arm', () => {
const entry: DataKeyArm = { dataKey: 'margin', type: 'line' };
expect(entry.type).toBe('line');
});

it('reads `type` off an entry narrowed to the `dataKey` arm', () => {
const familyOf = (e: SeriesEntry): string | undefined => ('dataKey' in e ? e.type : undefined);
expect(familyOf({ dataKey: 'margin', type: 'line' })).toBe('line');
});

it('changes no other arm: the `name` arm still carries no `chartType`', () => {
// @ts-expect-error — `chartType` is the renderer's INTERNAL spelling of `type`; the `name` arm never carried it, and objectui#8086 widened only the `dataKey` arm.
const authored: NameArm = { name: 'margin', chartType: 'line' };
expect(authored.name).toBe('margin');
});
});
12 changes: 7 additions & 5 deletions packages/plugin-charts/src/ChartRenderer.specSeries.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ describe('ChartRenderer — the spec `series` shape', () => {
// `chartType`. An author who wrote the documented `dataKey` binding AND
// the documented `type` override together (both valid independently on
// `ChartDataSeriesSchema`) got neither: predicted/observed 2 bars / 0
// lines before the fix. `type` on a `dataKey` entry is off the
// `ChartRendererProps` TS union (`as any` matches the schema's own
// acceptance, not this internal prop type — see the docblock).
// lines before the fix. The `dataKey` arm of `ChartRendererProps.series`
// declares `type` too (objectui#8086), so this literal is written with no
// cast; the arm-level pin is `ChartRenderer.seriesTypeArm-8086.test.ts`.
const { container } = render(
<ChartRenderer
schema={{
Expand All @@ -130,7 +130,7 @@ describe('ChartRenderer — the spec `series` shape', () => {
xAxisKey: 'month',
series: [{ dataKey: 'revenue' }, { dataKey: 'margin', type: 'line' }],
isAnimationActive: false,
} as any}
}}
/>,
);
expect(await plotted(container)).toEqual({ bars: 1, lines: 1 });
Expand Down Expand Up @@ -160,7 +160,9 @@ describe('ChartRenderer — the spec `series` shape', () => {
// because `normalizeChartSchema` — the one translation point — has always
// consumed it. The axis is written in the canonical `xAxisKey`: the
// Tremor-ish `index` alias that stood here is retired, and its pin lives in
// `ChartRenderer.foreignDialectRetired-8650.test.tsx`.
// `ChartRenderer.foreignDialectRetired-8650.test.tsx`. The `as any` below
// is for `categories`, which `ChartRendererProps.schema` does not declare;
// it is not a `series` escape (objectui#8086 widened only the `series` arm).
const { container } = render(
<ChartRenderer
schema={{
Expand Down
27 changes: 16 additions & 11 deletions packages/plugin-charts/src/ChartRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,18 @@ export interface ChartRendererProps {
* blank. `normalizeChartSchema` translates every entry, of either shape,
* uniformly — see the normalization comment in the component body.
*
* This TS union stays as declared (the `dataKey` arm has no `type`, the
* `name` arm has no `chartType`): at RUNTIME `type` is honoured on a
* `dataKey`-shaped entry too (objectui#7681, both keys are independently
* optional on `ChartDataSeriesSchema`), because JSON metadata never goes
* through this TS type. Widening the arm to match is a separate,
* public-face decision this fix does not make.
* Both arms carry the per-series family override `type`, with the same
* member type (objectui#8086): the renderer honours it on either shape,
* because every entry goes through `normalizeSeries` (objectui#7681), and
* `ChartDataSeriesSchema` declares `dataKey` and `name` independently
* optional beside it — so a `dataKey` entry carrying `type` is what the
* contract and the renderer both accept, and this union says so.
* `chartType` stays on the `dataKey` arm alone: it is the renderer's
* INTERNAL spelling of `type`, and it wins when an entry writes both.
* Pinned at compile time in `ChartRenderer.seriesTypeArm-8086.test.ts`.
*/
series?: Array<
| { dataKey: string; label?: string; variant?: 'current' | 'comparison'; opacity?: number; dashArray?: string; chartType?: 'bar' | 'line' | 'area'; stack?: string; yAxis?: 'left' | 'right'; color?: string }
| { dataKey: string; label?: string; type?: string; variant?: 'current' | 'comparison'; opacity?: number; dashArray?: string; chartType?: 'bar' | 'line' | 'area'; stack?: string; yAxis?: 'left' | 'right'; color?: string }
| { name: string; label?: unknown; type?: string; variant?: 'current' | 'comparison' | 'primary'; opacity?: number; dashArray?: string; stack?: string; yAxis?: 'left' | 'right'; color?: string }
>;
/** Spec `ChartConfig` shape — honored via `normalizeChartSchema`
Expand Down Expand Up @@ -151,10 +154,12 @@ export const ChartRenderer: React.FC<ChartRendererProps> = ({ schema, onChartCli
// together — both valid on `ChartDataSeriesSchema` independently — got
// NEITHER honoured (objectui#7681).
//
// `normalizeSeries` is a no-op on a well-formed internal-shaped entry: it
// round-trips every key the internal arm of `ChartRendererProps.series`
// declares (`dataKey`/`label`/`chartType`/`variant`/`opacity`/`dashArray`/
// `stack`/`yAxis`/`color`) unchanged. So always taking the normalized array
// `normalizeSeries` is a no-op on a well-formed internal-shaped entry apart
// from `type`: it round-trips every other key the internal arm of
// `ChartRendererProps.series` declares (`dataKey`/`label`/`chartType`/
// `variant`/`opacity`/`dashArray`/`stack`/`yAxis`/`color`) unchanged, and
// translates `type` to `chartType` — the translation this routing exists
// for. So always taking the normalized array
// is not a second read site for `type` (AGENTS.md #0.1) — it is routing
// EVERY entry, of either shape, through the ONE normalization layer
// (objectui#2880 S1) instead of special-casing one shape around it, which
Expand Down
9 changes: 5 additions & 4 deletions packages/types/src/objectql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4351,10 +4351,11 @@ export interface ObjectChartSchema extends BaseSchema {
* INTERNAL (relay-composed) — the plotted series, in the renderer's internal
* `{ dataKey }` contract.
*
* The element type is `ChartRendererProps.schema.series`' internal arm
* VERBATIM — that is the read this value ends at, and the ruling on
* objectui#7946 asked for the reads rather than a copy of any producer's
* literal. The spec's AUTHOR-facing `ChartSeriesSchema` is the other arm
* The element type is `ChartRendererProps.schema.series`' internal arm —
* that is the read this value ends at, and the ruling on objectui#7946 asked
* for the reads rather than a copy of any producer's literal — every member
* of it except `type`, which that arm gained under objectui#8086 and this
* copy has not taken up. The spec's AUTHOR-facing `ChartSeriesSchema` is the other arm
* (`{ name }`), and it refuses `dataKey` by name; `normalizeChartSchema` is
* the one translation between them.
*/
Expand Down
Loading