diff --git a/.changeset/11520-object-chart-spectype-routing.md b/.changeset/11520-object-chart-spectype-routing.md new file mode 100644 index 0000000000..474856efd2 --- /dev/null +++ b/.changeset/11520-object-chart-spectype-routing.md @@ -0,0 +1,18 @@ +--- +'@object-ui/plugin-charts': minor +--- + +fix(plugin-charts): an `object-chart` whose `specType` is a single-value or tabular spec family draws its routed form, not a silent bar chart; the renderer has no default family (objectui#11520) + +**What an accepted document draws changes.** `specType` is the react tier's chart family: the react-page wrapper parks `` there, because `type` is the node's discriminator. Both faces declare it as the spec's whole `ChartTypeSchema`, so an `object-chart` with `specType` set to `gauge`, `solid-gauge`, `metric`, `kpi`, `bullet`, `table` or `pivot` parsed on both faces, and drew a **bar chart**, with no note. The same families on `chartType` already drew the forms below. + +- **`gauge`, `solid-gauge`, `metric`, `kpi`, `bullet` on `specType`** now draw the number card: one row's number, the form the same family draws on `chartType`. +- **`table`, `pivot` on `specType`** now draw the tabular notice, which names the data-table and pivot components. +- **An off-spec `specType`** (both faces refuse it) draws the unknown-type notice, which names the value. +- **A chart that names no family at all** (both faces refuse it: `object-chart` needs `chartType` or `specType`, and `chart` needs `chartType`) draws a notice reading "This chart names no chart type — nothing was drawn." It used to draw a bar chart. A caller that means a bar names `bar`. + +Every family this block draws as a chart draws exactly what it drew before, on both keys. + +How: `normalizeChartSchema` used to keep only the families this package draws as a chart, and `AdvancedChartImpl` drew a missing family as `'bar'`. The normalizer now hands the named family on as named, so `specType` reaches the same family dispatch `chartType` reaches, and the `'bar'` default is removed (with its four copies in the refusal guards). + +⚠️ **Type.** `NormalizedChartSchema['chartType']`, exported from this package, widens from the thirteen drawn families to the declared chart families (`DeclaredChartFamily` in the emitted declarations; not a new entry export): `object-chart`'s `chartType` union together with its `specType` (the spec's `ChartType`), both read off `ObjectChartSchema` in `@object-ui/types` by reference. It is now the family the schema names, drawn or not, and a misspelled family still fails to compile. Shipped as `minor`, not `major` (objectui never declares `major`); a consumer that reads it exhaustively needs a branch for each single-value and tabular family, which this package draws no chart of. diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index 6965130bfd..2c45ba4c59 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -47,7 +47,7 @@ import { ChartContainerConfig } from './ChartContainerImpl'; import { mapScatterClick, mapTreemapClick, mapSankeyClick } from './chartDrillEvents'; -import { formatterFor, domainFor, ticksFor, RENDERABLE, SINGLE_VALUE_CHART_TYPES, TABULAR_CHART_TYPES, effectiveChartFamily, comboBaseFamily, placeYAxes, type NormalizedAxis, type NormalizedSeries, type ValueAxisSlot, type YAxisPlacement } from './normalizeChartSchema'; +import { formatterFor, domainFor, ticksFor, RENDERABLE, SINGLE_VALUE_CHART_TYPES, TABULAR_CHART_TYPES, effectiveChartFamily, comboBaseFamily, placeYAxes, type DeclaredChartFamily, type NormalizedAxis, type NormalizedSeries, type ValueAxisSlot, type YAxisPlacement } from './normalizeChartSchema'; import { buildCategoryRank, chartRowBucketId, isRealCalendarDate, toDateInputValue, toDisplayDate, type ChartSegmentClickEvent } from '@object-ui/core'; import { useDisplayLocale, useSafeTranslate } from '@object-ui/i18n'; @@ -238,11 +238,22 @@ const seriesLabelForKey = ( export interface AdvancedChartImplProps { /** - * Chart family. `combo` is renderer-local and rarely needs to be passed: - * series declaring different families derive it (`effectiveChartFamily`), - * which is how `@objectstack/spec` expresses a combo chart. + * Chart family, as the schema names it. `combo` is renderer-local and rarely + * needs to be passed: series declaring different families derive it + * (`effectiveChartFamily`), which is how `@objectstack/spec` expresses a + * combo chart. + * + * Typed as the declared chart families ({@link DeclaredChartFamily}: the + * spec's `ChartType` and the `object-chart` node's own union, by reference), + * so a misspelled family does not compile. The family dispatch below is the + * one place a family becomes a form: a chart for `RENDERABLE`, the number + * card for the single-value families, the tabular notice for the tabular + * ones, and a notice for anything else, which is how a value from unvalidated + * JSON outside the type is answered at runtime. ⛔ There is no default family + * (objectui#11520): an absent family is that notice too, never a bar. A + * caller that means a bar passes `'bar'`. */ - chartType?: 'bar' | 'column' | 'horizontal-bar' | 'line' | 'area' | 'pie' | 'donut' | 'radar' | 'scatter' | 'funnel' | 'combo' | 'treemap' | 'sankey'; + chartType?: DeclaredChartFamily; data?: Array>; config?: ChartContainerConfig; xAxisKey?: string; @@ -1426,7 +1437,10 @@ function unplottedPointsNote( * This component is lazy-loaded to avoid including Recharts in the initial bundle */ function AdvancedChartImplInner({ - chartType: rawChartType = 'bar', + // ⛔ No `= 'bar'` default (objectui#11520): it drew a bar for every family + // that arrived unset, which is how a `specType: 'gauge'` chart became a bar + // chart with no note. An absent family is the notice in the dispatch below. + chartType: rawChartType, data: rawData = [], config = {}, xAxisKey = 'name', @@ -1676,22 +1690,6 @@ function AdvancedChartImplInner({ window.addEventListener('resize', checkMobile); return () => window.removeEventListener('resize', checkMobile); }, []); - const ChartComponent = { - bar: BarChart, - 'horizontal-bar': BarChart, - line: LineChart, - area: AreaChart, - pie: PieChart, - donut: PieChart, - radar: RadarChart, - scatter: ScatterChart, - funnel: FunnelChart as any, - // combo/treemap/sankey return from their own branches above; mapped here - // only so the index type stays exhaustive. - combo: ComposedChart, - treemap: BarChart, - sankey: BarChart, - }[chartType] || BarChart; // Format ISO date strings into compact "MMM D" / "MMM YYYY" labels for X-axis ticks. // Falls back to the raw value when not parseable as a date. @@ -2038,12 +2036,17 @@ function AdvancedChartImplInner({ // #2942 — the non-series spec families used to fall through the component // map's `|| BarChart` into a bar shell whose series marks all returned // null: grid, axes, tooltip and legend rendered with NO data marks, - // indistinguishable from an empty dataset. Reachable because ChartRenderer - // resolves `schema.chartType ?? spec.chartType` without going through - // `normalizeChartSchema`'s RENDERABLE gate. Single-value families render + // indistinguishable from an empty dataset. Single-value families render // the measure as a number (the spec's own framing for them); tabular ones // say which component owns the rendering; unknown values are named instead // of guessed at. + // + // objectui#11520 — this is the ONE family → form dispatch, and every channel + // reaches it with the family as named: `chartType` straight from + // `ChartRenderer`, and `specType` (the react tier's family) through + // `normalizeChartSchema`, which no longer drops the families this block + // draws no chart of. A chart that arrives with no family at all is the + // unknown-type notice below, not a bar. if (chartType && SINGLE_VALUE_CHART_TYPES.has(chartType)) { const dataKey = series[0]?.dataKey || 'value'; const raw = data[0]?.[dataKey]; @@ -2071,18 +2074,43 @@ function AdvancedChartImplInner({ ); } - if (chartType && !RENDERABLE.has(chartType)) { + if (!chartType || !RENDERABLE.has(chartType)) { return (
- Chart type “{chartType}” is not a spec chart type — nothing was drawn. + {chartType ? ( + <>Chart type “{chartType}” is not a spec chart type — nothing was drawn. + ) : ( + <>This chart names no chart type — nothing was drawn. + )}
); } + // The recharts root the cartesian tail at the end draws in. It is read + // only past the dispatch above, which returns for every family outside + // `RENDERABLE` and for none (objectui#11520), so `chartType` is a drawn + // family here. combo/treemap/sankey return from their own branches below; + // they are mapped only so every drawn family has a row. + const cartesianRoots = { + bar: BarChart, + 'horizontal-bar': BarChart, + line: LineChart, + area: AreaChart, + pie: PieChart, + donut: PieChart, + radar: RadarChart, + scatter: ScatterChart, + funnel: FunnelChart as any, + combo: ComposedChart, + treemap: BarChart, + sankey: BarChart, + }; + const ChartComponent = cartesianRoots[chartType as keyof typeof cartesianRoots] || BarChart; + // Pie and Donut charts if (chartType === 'pie' || chartType === 'donut') { const innerRadius = chartType === 'donut' ? '52%' : 0; @@ -2916,11 +2944,24 @@ function AdvancedChartImplInner({ * it (see `bucketNullCategories` in `@object-ui/core`), which is what keeps this * predicate meaning what it says. */ +/** + * The family the refusal guards below judge, `column` read as the bar it draws. + * + * ⛔ No `'bar'` default (objectui#11520). Each guard used to restate the + * component's own `= 'bar'`, so a chart that named no family was judged as a + * bar here before it was drawn as one. It now names no family to them either: + * none of them refuses it, and the component draws the unknown-type notice. + */ +function guardFamily(props: AdvancedChartImplProps): DeclaredChartFamily | undefined { + return props.chartType === 'column' ? 'bar' : props.chartType; +} + function hasNoCategoryKey(props: AdvancedChartImplProps): boolean { - const chartType = props.chartType === 'column' ? 'bar' : (props.chartType ?? 'bar'); + const chartType = guardFamily(props); const rows = Array.isArray(props.data) ? props.data : []; const key = props.xAxisKey ?? 'name'; return ( + chartType !== undefined && CATEGORY_AXIS_CHART_TYPES.has(chartType) && rows.length > 0 && !rows.some((row) => row != null && typeof row === 'object' && key in row) @@ -3019,9 +3060,9 @@ const SERIES_ONLY_CHART_TYPES: ReadonlySet = new Set([ type NoPlottableSeries = 'empty' | 'undeclared'; function hasNoPlottableSeries(props: AdvancedChartImplProps): NoPlottableSeries | null { - const chartType = props.chartType === 'column' ? 'bar' : (props.chartType ?? 'bar'); + const chartType = guardFamily(props); const rows = Array.isArray(props.data) ? props.data : []; - if (!SERIES_ONLY_CHART_TYPES.has(chartType) || rows.length === 0) return null; + if (chartType === undefined || !SERIES_ONLY_CHART_TYPES.has(chartType) || rows.length === 0) return null; if (props.series === undefined) return 'undeclared'; if (Array.isArray(props.series) && props.series.length === 0) return 'empty'; return null; @@ -3079,10 +3120,10 @@ function hasNoPlottableSeries(props: AdvancedChartImplProps): NoPlottableSeries * counts. */ function hasNoNumericSeriesValue(props: AdvancedChartImplProps): string[] | null { - const chartType = props.chartType === 'column' ? 'bar' : (props.chartType ?? 'bar'); + const chartType = guardFamily(props); const rows = Array.isArray(props.data) ? props.data : []; const series = Array.isArray(props.series) ? props.series : []; - if (!SERIES_ONLY_CHART_TYPES.has(chartType) || rows.length === 0 || series.length === 0) return null; + if (chartType === undefined || !SERIES_ONLY_CHART_TYPES.has(chartType) || rows.length === 0 || series.length === 0) return null; const keys = Array.from(new Set(series.map((s) => String(s.dataKey)))); const resolved = keys.every((key) => rows.some( @@ -3145,10 +3186,10 @@ function hasNoNumericSeriesValue(props: AdvancedChartImplProps): string[] | null * at all". Scatter's absent key is already refused by `no-plottable-points`. */ function hasNoCarriedSeriesKey(props: AdvancedChartImplProps): string[] | null { - const chartType = props.chartType === 'column' ? 'bar' : (props.chartType ?? 'bar'); + const chartType = guardFamily(props); const rows = Array.isArray(props.data) ? props.data : []; const series = Array.isArray(props.series) ? props.series : []; - if (!SERIES_ONLY_CHART_TYPES.has(chartType) || rows.length === 0 || series.length === 0) return null; + if (chartType === undefined || !SERIES_ONLY_CHART_TYPES.has(chartType) || rows.length === 0 || series.length === 0) return null; const keys: unknown[] = Array.from(new Set(series.map((s) => s.dataKey))); const absent = keys.every( (key) => diff --git a/packages/plugin-charts/src/__tests__/object-chart-spectype-routing-11520.test.tsx b/packages/plugin-charts/src/__tests__/object-chart-spectype-routing-11520.test.tsx new file mode 100644 index 0000000000..ff6aed3259 --- /dev/null +++ b/packages/plugin-charts/src/__tests__/object-chart-spectype-routing-11520.test.tsx @@ -0,0 +1,330 @@ +/** + * 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#11520: an `object-chart` whose family rides on `specType` draws the + * form its family routes to, and never a silent bar. + * + * ## The defect this pins + * + * `specType` is the react tier's chart family: the react-page wrapper parks the + * author's `` there, because `type` is the node's + * discriminator. Both faces declare it as the spec's whole `ChartTypeSchema`. + * `normalizeChartSchema` read it, then dropped any family outside the drawn set, + * so `ChartRenderer` handed `AdvancedChartImpl` no family at all, and + * `AdvancedChartImpl` defaulted to `'bar'`. A `gauge`, `kpi`, `metric`, + * `bullet`, `solid-gauge`, `table` or `pivot` document that both faces accept + * drew a bar chart, with no note. The same families on `chartType` already drew + * a number card or the tabular notice. + * + * ## What this pins + * + * Triage's ruling on objectui#11520: the `specType` channel routes a family + * this block draws no chart of through the same dispatch the `chartType` + * channel reaches, and the `'bar'` default is gone. So every cell below is + * held to its ROUTED FORM, read off the dispatch's own sets in + * `normalizeChartSchema.ts` (no family list is restated here): + * + * - `SINGLE_VALUE_CHART_TYPES` → the number card (`advanced-chart-single-value`); + * - `TABULAR_CHART_TYPES` → the tabular notice (`advanced-chart-tabular-notice`); + * - `RENDERABLE` → the family's OWN marks, so a family silently drawn as a bar + * fails; + * - anything else (the off-spec `sunburst`) → the unknown-type notice, naming + * the value. + * + * The matrix is every family of the installed spec plus `sunburst`, in five + * node shapes, each carrying the family on `specType`: + * + * 1. the flat node with inline rows (`data`, `xAxisKey`, `series[{ dataKey }]`); + * 2. the authored `properties` bag on the object path (`objectName`, + * `aggregate`, the spec's `xAxis` and `series[{ name }]`): the document the + * card measured, which both faces accept; + * 3. the dashboard producer's flat node (`objectName`, `aggregate`, `xAxisKey`, + * `series[{ dataKey }]`); + * 4. the react-page wrapper's own output, `{ ...props, specType, type }`, from + * the react tier's `` props; + * 5. a dataset-bound node (`dataset`, one dimension, one value). + * + * Controls: `bar` (and `column`, `horizontal-bar`) still draw bars, so the mark + * counters can see a chart. `sunburst` draws no chart and names itself. A node + * that names no family at all shows the notice instead of the bar it used to + * get. The two channels are held to one answer: per family, `specType` draws + * the same form `chartType` draws. `object-chart-declared-families-11513.test.tsx` + * holds the `chartType` channel's refusals. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; + +// Recharts' ResponsiveContainer measures via ResizeObserver, which reports 0x0 +// under the headless DOM, so nothing paints. Fix its size. +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +import { ChartTypeSchema } from '@objectstack/spec/ui'; +import { chartCategoryKey, chartMeasureKey } from '@object-ui/core'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import { enumOptions } from '@object-ui/test-support'; +import { StrictAnyComponentSchema, safeValidateSchema } from '@object-ui/types/zod'; +import { RENDERABLE, SINGLE_VALUE_CHART_TYPES, TABULAR_CHART_TYPES } from '../normalizeChartSchema'; +// The package entry, for its REGISTRATION side effects. Relative: a package +// must not import itself (`pnpm check:self-import`). +import '../index'; +// `ChartRenderer` renders its implementation behind +// `React.lazy(() => import('./AdvancedChartImpl'))`; importing it here pays the +// recharts graph in the import phase, which no test timeout applies to +// (AGENTS.md §测试纪律). +import '../AdvancedChartImpl'; + +afterEach(cleanup); + +// `ObjectChart` loads the grouped field's (or the dataset's) metadata over +// `fetch` for its labels. Answer it here, for the whole file, so no read +// reaches a socket. +vi.stubGlobal('fetch', async () => new Response('{}', { status: 404 })); + +/** The off-spec family: both faces refuse it, and it draws no chart. */ +const OFF_SPEC = 'sunburst'; +const SPEC_FAMILIES: string[] = enumOptions(ChartTypeSchema); +const FAMILIES: string[] = [...SPEC_FAMILIES, OFF_SPEC]; + +const AGGREGATE = { field: 'amount', function: 'sum', groupBy: 'stage' } as const; +const MEASURE = chartMeasureKey(AGGREGATE, 'value'); +const CATEGORY = chartCategoryKey(AGGREGATE, 'name'); + +/** Scatter plots its category as a position, so its categories are numeric. */ +const categoriesFor = (family: string) => (family === 'scatter' ? [1, 2, 3] : ['won', 'open', 'lost']); +/** The rows the object path's `aggregate` answers, and the inline rows. */ +const rowsFor = (family: string) => categoriesFor(family).map((stage, i) => ({ [CATEGORY]: stage, [MEASURE]: i + 1 })); +/** The rows the dataset path's `queryDataset` answers, keyed by dimension and value name. */ +const datasetRowsFor = (family: string) => categoriesFor(family).map((stage, i) => ({ stage, amount: i + 1 })); + +const sourceFor = (family: string) => ({ + find: async () => [], + findOne: async () => null, + aggregate: async () => rowsFor(family), + queryDataset: async () => ({ rows: datasetRowsFor(family), fields: [], object: 'probe_deal' }), + count: async () => 0, + getObject: async () => null, +}); + +/** The react tier's `` props, as an author writes them on the block. */ +const reactTierProps = (family: string) => ({ + type: family, + objectName: 'probe_deal', + aggregate: AGGREGATE, + xAxis: { field: CATEGORY }, + series: [{ name: MEASURE }], +}); + +/** + * The react-page wrapper's node, built the way `react-page.tsx`'s `Wrapper` + * builds it: the author's `type` is parked as `specType`, and the block's own + * discriminator wins the `type` slot. + */ +const reactPageNode = (tag: string, props: Record) => { + const specType = typeof props.type === 'string' && props.type !== tag ? props.type : undefined; + return { ...props, ...(specType ? { specType } : {}), type: tag }; +}; + +/** The five node shapes, each carrying the family on `specType`. */ +const SPELLINGS: Record Record> = { + 'flat node, inline rows': (family) => ({ + type: 'object-chart', + specType: family, + data: rowsFor(family), + xAxisKey: CATEGORY, + series: [{ dataKey: MEASURE }], + }), + 'properties bag, object path': (family) => ({ + type: 'object-chart', + properties: { + specType: family, + objectName: 'probe_deal', + aggregate: AGGREGATE, + xAxis: { field: CATEGORY }, + series: [{ name: MEASURE }], + }, + }), + "dashboard producer's node": (family) => ({ + type: 'object-chart', + specType: family, + objectName: 'probe_deal', + aggregate: AGGREGATE, + xAxisKey: CATEGORY, + series: [{ dataKey: MEASURE }], + }), + "react-page wrapper's node": (family) => reactPageNode('object-chart', reactTierProps(family)), + 'dataset node': (family) => ({ + type: 'object-chart', + specType: family, + dataset: 'probe_deal_dataset', + dimensions: ['stage'], + values: ['amount'], + }), +}; + +interface Drawn { + marks: Record; + valueAxes: number; + xTicks: string[]; + yTicks: string[]; + notice: string | null; + noticeText: string; + refusal: string | null; + surface: boolean; +} + +async function renderNode(node: Record, family: string): Promise { + const { container: c } = render( + + + , + ); + await waitFor( + () => { + const painted = + c.querySelector('.recharts-surface') ?? + c.querySelector('[data-chart-error]') ?? + c.querySelector('[data-testid^="advanced-chart-"]'); + expect(painted, `${family}: the chart never rendered past its loading state`).not.toBeNull(); + }, + { timeout: 10_000 }, + ); + const ticks = (axis: 'x' | 'y') => + [...c.querySelectorAll(`.recharts-${axis}Axis-tick-labels .recharts-cartesian-axis-tick-value`)].map((e) => e.textContent ?? ''); + const count = (selector: string) => c.querySelectorAll(selector).length; + const notice = c.querySelector('[data-testid^="advanced-chart-"]'); + return { + marks: { + bar: count('.recharts-bar'), + line: count('.recharts-line'), + area: count('.recharts-area'), + pie: count('.recharts-pie'), + funnel: count('.recharts-trapezoids'), + scatter: count('.recharts-scatter'), + treemap: count('.recharts-treemap-depth-1'), + sankey: count('.recharts-sankey-nodes') + count('.recharts-sankey-links'), + radar: count('.recharts-radar'), + }, + valueAxes: count('.recharts-yAxis'), + xTicks: ticks('x'), + yTicks: ticks('y'), + notice: notice?.getAttribute('data-testid') ?? null, + noticeText: notice?.textContent ?? '', + refusal: c.querySelector('[data-chart-error]')?.getAttribute('data-chart-error') ?? null, + surface: !!c.querySelector('.recharts-surface'), + }; +} + +const drawsAChart = (d: Drawn) => d.surface && d.notice === null && d.refusal === null; +const markKinds = (d: Drawn) => Object.entries(d.marks).filter(([, n]) => n > 0).map(([name]) => name).join(','); + +/** What each drawn family's own marks look like, read off what recharts put in the DOM. */ +const OWN_MARKS: Record boolean> = { + bar: (d) => markKinds(d) === 'bar' && d.xTicks.includes('won') && d.valueAxes === 1, + column: (d) => markKinds(d) === 'bar' && d.xTicks.includes('won') && d.valueAxes === 1, + 'horizontal-bar': (d) => markKinds(d) === 'bar' && d.yTicks.includes('won') && !d.xTicks.includes('won'), + line: (d) => markKinds(d) === 'line', + area: (d) => markKinds(d) === 'area', + pie: (d) => markKinds(d) === 'pie', + donut: (d) => markKinds(d) === 'pie', + funnel: (d) => markKinds(d) === 'funnel', + scatter: (d) => markKinds(d) === 'scatter', + treemap: (d) => markKinds(d) === 'treemap', + sankey: (d) => markKinds(d) === 'sankey', + radar: (d) => markKinds(d) === 'radar', + // One measure on the combo arm: its positional first mark, on the arm's two value axes. + combo: (d) => markKinds(d) === 'bar' && d.valueAxes === 2, +}; + +/** The form a family routes to, by the dispatch's own sets. */ +const routedForm = (family: string): string => { + if (SINGLE_VALUE_CHART_TYPES.has(family)) return 'advanced-chart-single-value'; + if (TABULAR_CHART_TYPES.has(family)) return 'advanced-chart-tabular-notice'; + if (RENDERABLE.has(family)) return `chart:${family}`; + return 'advanced-chart-unknown-type'; +}; + +/** The form a render actually took: a notice's test id, or the chart its marks say it drew. */ +const drawnForm = (family: string, d: Drawn): string => { + if (d.notice) return d.notice; + if (d.refusal) return `refusal:${d.refusal}`; + if (!drawsAChart(d)) return 'nothing'; + // A drawn family must draw ITS OWN marks; anything else names the marks it drew instead. + return OWN_MARKS[family]?.(d) ? `chart:${family}` : `marks:${markKinds(d) || 'none'}`; +}; + +describe('an object-chart draws its specType family\'s routed form, never a silent bar (objectui#11520)', () => { + it('reads the spec\'s families, and each one has a routed form', () => { + expect(SPEC_FAMILIES.length, 'could not read ChartTypeSchema.options from the spec').toBeGreaterThan(0); + // Every drawn family has its own mark predicate, so none can pass as "some chart". + expect(Object.keys(OWN_MARKS).sort()).toEqual([...RENDERABLE].filter((f) => SPEC_FAMILIES.includes(f)).sort()); + // Every spec family lands on a branch of the dispatch, and the off-spec control does not. + for (const family of SPEC_FAMILIES) expect(routedForm(family), family).not.toBe('advanced-chart-unknown-type'); + expect(routedForm(OFF_SPEC)).toBe('advanced-chart-unknown-type'); + }); + + it('the card\'s document parses on both faces for every spec family on specType, and the off-spec control does not', () => { + for (const family of FAMILIES) { + const doc = SPELLINGS['properties bag, object path'](family); + for (const parse of [safeValidateSchema, (d: unknown) => StrictAnyComponentSchema.safeParse(d)]) { + expect(parse(doc).success, `${family}: parses exactly when it is a spec family`).toBe(SPEC_FAMILIES.includes(family)); + } + } + }); + + describe.each(Object.keys(SPELLINGS).map((s) => [s] as const))('%s', (spelling) => { + it.each(FAMILIES.map((f) => [f] as const))('`%s` on specType draws its routed form', async (family) => { + const drawn = await renderNode(SPELLINGS[spelling](family), family); + expect(drawnForm(family, drawn), `${family} (${spelling}): ${JSON.stringify(drawn)}`).toBe(routedForm(family)); + if (routedForm(family) === 'advanced-chart-unknown-type') expect(drawn.noticeText).toContain(family); + }); + }); + + it.each(FAMILIES.map((f) => [f] as const))('`%s`: specType draws the same form chartType draws', async (family) => { + const viaSpecType = await renderNode(SPELLINGS['properties bag, object path'](family), family); + cleanup(); + const bag = SPELLINGS['properties bag, object path'](family).properties as Record; + const { specType: _specType, ...rest } = bag; + const viaChartType = await renderNode({ type: 'object-chart', properties: { ...rest, chartType: family } }, family); + expect(drawnForm(family, viaSpecType)).toBe(drawnForm(family, viaChartType)); + }); + + it('a node that names no family shows the notice, not a bar', async () => { + // Both faces refuse it ("names no chart family"); the render door now says so too. + const familyless = SPELLINGS['properties bag, object path']('bar'); + const { specType: _specType, ...rest } = familyless.properties as Record; + const node = { type: 'object-chart', properties: rest }; + expect(safeValidateSchema(node).success).toBe(false); + expect(StrictAnyComponentSchema.safeParse(node).success).toBe(false); + const drawn = await renderNode(node, 'bar'); + expect(drawn.notice, JSON.stringify(drawn)).toBe('advanced-chart-unknown-type'); + expect(drawn.noticeText).toContain('names no chart type'); + expect(drawn.marks.bar).toBe(0); + }); + + it('the bare `chart` node that names no family shows the same notice; naming `bar` still draws a bar', async () => { + // `ChartRenderer` directly, without `ObjectChart`: the registration that used to + // reach `AdvancedChartImpl`'s `'bar'` default with nothing in between. + const inline = { type: 'chart', data: rowsFor('bar'), xAxisKey: CATEGORY, series: [{ dataKey: MEASURE }] }; + const familyless = await renderNode(inline, 'bar'); + expect(familyless.notice, JSON.stringify(familyless)).toBe('advanced-chart-unknown-type'); + expect(familyless.marks.bar).toBe(0); + cleanup(); + const bar = await renderNode({ ...inline, chartType: 'bar' }, 'bar'); + expect(drawnForm('bar', bar), JSON.stringify(bar)).toBe('chart:bar'); + }); +}); diff --git a/packages/plugin-charts/src/normalizeChartSchema.test.ts b/packages/plugin-charts/src/normalizeChartSchema.test.ts index 8b27f6b9c3..bcd1f94aea 100644 --- a/packages/plugin-charts/src/normalizeChartSchema.test.ts +++ b/packages/plugin-charts/src/normalizeChartSchema.test.ts @@ -117,10 +117,16 @@ describe('normalizeChartSchema — the `type` collision', () => { expect(normalizeChartSchema({ type: 'object-chart', specType: 'donut' }).chartType).toBe('donut'); }); - it('leaves a family this renderer cannot draw unset', () => { - // `metric`/`kpi` are single-value families rendered by other components; - // mapping them onto a bar chart would draw the wrong picture silently. - expect(normalizeChartSchema({ specType: 'metric' }).chartType).toBeUndefined(); + it('hands a family this renderer draws no chart of to the dispatch as named (objectui#11520)', () => { + // This used to answer `undefined` for `metric`, and `AdvancedChartImpl` + // drew `undefined` as its `'bar'` default: a single-value chart drew a bar + // with no note. The family now reaches the dispatch, which draws the number + // card for it, the tabular notice for `table`, and names an off-spec value. + expect(normalizeChartSchema({ type: 'object-chart', specType: 'metric' }).chartType).toBe('metric'); + expect(normalizeChartSchema({ type: 'object-chart', specType: 'pivot' }).chartType).toBe('pivot'); + expect(normalizeChartSchema({ type: 'object-chart', specType: 'sunburst' }).chartType).toBe('sunburst'); + // `chartType` still wins when a node writes both. + expect(normalizeChartSchema({ type: 'object-chart', chartType: 'line', specType: 'gauge' }).chartType).toBe('line'); }); }); diff --git a/packages/plugin-charts/src/normalizeChartSchema.ts b/packages/plugin-charts/src/normalizeChartSchema.ts index 4e6be5159e..78935ea479 100644 --- a/packages/plugin-charts/src/normalizeChartSchema.ts +++ b/packages/plugin-charts/src/normalizeChartSchema.ts @@ -7,6 +7,7 @@ */ import { pickLocalized } from '@object-ui/i18n'; +import type { ObjectChartSchema } from '@object-ui/types'; /** * The ONE place the spec's author-facing chart shape is translated into the @@ -74,6 +75,24 @@ export type ChartFamily = | 'treemap' | 'sankey' | 'combo'; +/** + * Every chart family a schema can name on this renderer's family channels, as + * the faces DECLARE them (objectui#11520): the union of `object-chart`'s two + * family keys, read off `ObjectChartSchema` by reference and ⛔ never restated. + * + * - `specType` is the spec's whole `ChartType` (the react tier's family; a bare + * `chart` node's `chartType` declares the same domain). + * - `chartType` is the node's own union, the families this block draws + * (objectui#11513). It sits inside the spec's today, and is named anyway so + * the type says which two channels it covers. + * + * Wider than {@link ChartFamily} on purpose: it includes the single-value and + * tabular families, which `AdvancedChartImpl`'s dispatch routes to the number + * card and the tabular notice. Narrower than `string` on purpose: a misspelled + * family is a compile error, not a runtime notice. + */ +export type DeclaredChartFamily = NonNullable; + export const RENDERABLE = new Set([ 'bar', 'column', 'horizontal-bar', 'line', 'area', @@ -246,7 +265,20 @@ export interface NormalizedSeries { } export interface NormalizedChartSchema { - chartType?: ChartFamily; + /** + * The chart family the schema names, as named: `chartType`, else `specType`, + * else a bare `type` that is a spec family, else a registered keyword's family + * (objectui#11520). It is NOT narrowed to the families this renderer draws: + * `AdvancedChartImpl`'s family dispatch decides the form (a chart, the number + * card, the tabular notice, or a notice naming the value), so a family this + * module dropped could only ever reach that dispatch as no family at all. + * + * Typed as the vocabulary the faces DECLARE ({@link DeclaredChartFamily}), + * not as `string`, so a misspelled family is a compile error for every + * producer. A value outside it can still arrive in unvalidated JSON; see the + * JSON-boundary cast in {@link normalizeChartSchema}. + */ + chartType?: DeclaredChartFamily; xAxisKey?: string; series?: NormalizedSeries[]; /** X-axis presentation config (its `field` is hoisted to `xAxisKey`). */ @@ -617,10 +649,26 @@ export function normalizeChartSchema( str(schema.specType) ?? (rawType && CHART_TYPES.has(rawType) ? rawType : undefined) ?? familyFromComponentType(rawType); - // A family this renderer does not draw (`metric`, `table`, …) is left unset - // rather than mapped onto a bar chart — the caller's own default is a more - // honest answer than silently drawing the wrong picture. - if (chartType && RENDERABLE.has(chartType)) out.chartType = chartType as ChartFamily; + // The named family goes to `AdvancedChartImpl`'s family dispatch AS NAMED, + // drawn or not (objectui#11520). That dispatch is the one place a family + // becomes a form: a chart for `RENDERABLE`, the number card for + // `SINGLE_VALUE_CHART_TYPES`, the tabular notice for `TABULAR_CHART_TYPES`, + // and a notice naming anything else. This used to keep only `RENDERABLE` + // families, on the theory that the caller's own default was the more honest + // answer; the caller's default was `'bar'`, so a `specType: 'gauge'` chart + // (the react tier's ``) drew a bar with no note, + // while the same family on `chartType`, which `ChartRenderer` hands over + // without this module, drew the number card. Both channels now reach the + // same branch with the same value. + // + // ⚠️ The JSON-boundary cast (the one place the family is asserted rather than + // proven). `schema` is unvalidated JSON, so the string read above is a + // `DeclaredChartFamily` only when a face validated the document. The cast + // states the declared vocabulary for every typed consumer downstream; a value + // outside it (an off-spec `specType`, a stale stored node) still reaches the + // dispatch as named, and the dispatch's unknown-type notice is the runtime + // guard for it. + if (chartType) out.chartType = chartType as DeclaredChartFamily; // ── axes ──────────────────────────────────────────────────────────────── // Spec `xAxis` is an object; the report surface narrows it to a bare string. @@ -758,10 +806,11 @@ export function comboBaseFamily(chartType: string | undefined): SeriesFamily | u * same family keeps its own family, so nothing that renders correctly today * changes; and an explicit `combo` is returned untouched. * - * Pass the chart's EFFECTIVE family (defaults already applied) — the answer - * depends on what an un-annotated series would otherwise have drawn. + * Pass the chart's EFFECTIVE family — the answer depends on what an + * un-annotated series would otherwise have drawn. A chart that names no family + * gets `undefined` back: there is no default family to widen (objectui#11520). */ -export function effectiveChartFamily( +export function effectiveChartFamily( chartType: T, series: readonly Pick[] | undefined, ): T | 'combo' {