|
1 | 1 | // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
2 | 2 |
|
3 | 3 | import { isIncoherentAggregate } from '@objectstack/spec/data'; |
4 | | -import { ChartTypeSchema } from '@objectstack/spec/ui'; |
5 | 4 |
|
6 | 5 | import { walkFilterFieldKeys } from './filter-walk.js'; |
7 | 6 | import { |
@@ -77,8 +76,13 @@ import { |
77 | 76 | * |
78 | 77 | * Advisory rules — severity `warning`, build stays green: |
79 | 78 | * |
80 | | - * - `chart-config-missing` — a chart-type widget (bar/line/pie/…) has no |
81 | | - * `chartConfig`, so the renderer cannot tell which measure to plot. |
| 79 | + * - `chart-config-missing` — a `combo` widget has no `chartConfig`, so no |
| 80 | + * series carries a mark and the combination chart draws as one uniform |
| 81 | + * family. Narrowed to `combo` because `chartConfig` carries NO binding: |
| 82 | + * `DatasetWidget` derives the axis and one series per measure from the |
| 83 | + * widget's own `dimensions` / `values`, and actively REFUSES an authored |
| 84 | + * `ChartAxis.field` / `ChartSeries.name`. See "What the renderer actually |
| 85 | + * derives" below for the pinned contract this now mirrors. |
82 | 86 | * - `table-count-only` (#1719) — a `table`/`pivot` widget whose selected |
83 | 87 | * measures are ALL `aggregate: 'count'` and which declares no |
84 | 88 | * `dimensions` asks the analytics service for a single summary row. That |
@@ -191,6 +195,65 @@ import { |
191 | 195 | * through the flat `SYSTEM_FIELDS` union, which is what lets a reference to |
192 | 196 | * `owner_id` on an `ownership: 'none'` object stay a real finding. |
193 | 197 | * |
| 198 | + * ### What the renderer actually derives (#14436) |
| 199 | + * |
| 200 | + * `chart-config-missing` used to fire on EVERY chart family that was not a |
| 201 | + * single-value or tabular type, on the stated grounds that without |
| 202 | + * `chartConfig` "the renderer cannot determine which measure to plot, so the |
| 203 | + * series renders empty". That consequence was false, and it fired on this |
| 204 | + * platform's OWN shipped metadata: the `system_overview` dashboard draws a pie |
| 205 | + * and a bar, each with one dimension and one measure and no `chartConfig`, and |
| 206 | + * both render correctly. A warning-tier rule that mis-fires on first-party |
| 207 | + * metadata is the ADR-0072 D1 cost the whole family exists to avoid — it |
| 208 | + * teaches every reader, human and agent, that this family is noise. |
| 209 | + * |
| 210 | + * The renderer's contract, read at the `@object-ui` revision this repo PINS |
| 211 | + * (`.objectui-sha`), not at objectui's `origin/main` — the Console ships the |
| 212 | + * pin, so the exemption is written against the behaviour the pin has: |
| 213 | + * |
| 214 | + * - `DatasetWidget` calls `buildChartSeries(rows, dimensions, values, …)`, |
| 215 | + * which returns `{ data, xAxisKey, series }` — the x-axis key is |
| 216 | + * `dimensions[0]` and there is exactly one series per entry of `values`. |
| 217 | + * `chartConfig` is not an argument to it. |
| 218 | + * - The authored `chartConfig` reaches the chart only through |
| 219 | + * `mergeAuthoredPresentation` (per-series/axis PRESENTATION merged ONTO |
| 220 | + * those derived bindings) and `chartConfigPresentation` (chrome: titles, |
| 221 | + * height, colours, data labels, annotations, legend). |
| 222 | + * - The BINDING half of an authored `chartConfig` is refused outright. The |
| 223 | + * renderer pins this by name in |
| 224 | + * `packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx`: |
| 225 | + * *"ignores an authored axis `field` and keeps the derived axis binding"*, |
| 226 | + * *"ignores an authored series and keeps one derived series per measure"*, |
| 227 | + * *"ignores `chartConfig.type` — the widget type owns the chart family"*, |
| 228 | + * and *"emits none of the presentation keys when no chartConfig is |
| 229 | + * declared"*. |
| 230 | + * |
| 231 | + * So for every chart family except one, a missing `chartConfig` costs the |
| 232 | + * widget nothing at all, and ADDING one could not have repaired a widget whose |
| 233 | + * selection is empty either — `chart-field-unknown` above refuses a |
| 234 | + * `yAxis[].field` that names anything the widget did not select, so |
| 235 | + * `chartConfig` can never supply a measure the `values` array is missing. |
| 236 | + * |
| 237 | + * **The one surviving arm is `combo`**, and it is a real loss rather than an |
| 238 | + * inferred one. A combination chart's whole identity is a per-series MARK, and |
| 239 | + * that mark is authored as `chartConfig.series[].type` — presentation, so it |
| 240 | + * does merge forward. `mergeAuthoredSeries` states the default it falls back |
| 241 | + * to: *"a derived series with no authored entry keeps the family default"*, |
| 242 | + * and `DatasetWidget.comboPresentation.test.tsx` records what that cost when |
| 243 | + * the merge was missing — *"Without it the line measure drew as the second |
| 244 | + * bar."* A `combo` with no `chartConfig` therefore draws every measure with |
| 245 | + * one mark: the author asked for a combination and got a plain one. Warning |
| 246 | + * rather than error, because the numbers are right and the chart renders — it |
| 247 | + * is the shape that is wrong. |
| 248 | + * |
| 249 | + * ⚠️ Two genuinely un-renderable shapes are NOT this rule's, and are |
| 250 | + * deliberately left unreported here rather than folded in under an id that |
| 251 | + * would then misname its own condition: a chart-type widget selecting NO |
| 252 | + * measures (the pin renders an explicit "Pick measures (values)" placeholder) |
| 253 | + * and one selecting no dimensions (the pin's `isMetric` covers |
| 254 | + * `dimensions.length === 0`, so it renders as a KPI number instead of the |
| 255 | + * family asked for). Neither is caused by, nor repairable with, `chartConfig`. |
| 256 | + * |
194 | 257 | * Warnings can be deliberately suppressed per widget via |
195 | 258 | * `suppressWarnings: ['<rule-id>']`; errors cannot — they describe a |
196 | 259 | * binding the analytics service cannot satisfy. |
@@ -274,28 +337,26 @@ function asStrings(v: unknown): string[] { |
274 | 337 | } |
275 | 338 |
|
276 | 339 | /** |
277 | | - * Chart families that plot a single value or every column, and so need no |
278 | | - * `chartConfig` measure mapping: single-value types plot their lone value, |
279 | | - * tabular types render each column as-is. |
280 | | - */ |
281 | | -const MEASURE_EXEMPT_CHART_TYPES = new Set([ |
282 | | - 'gauge', 'solid-gauge', 'metric', 'kpi', 'bullet', |
283 | | - 'table', 'pivot', |
284 | | -]); |
285 | | - |
286 | | -/** |
287 | | - * Chart families whose renderer needs a `chartConfig` measure mapping — the |
288 | | - * taxonomy minus the exemptions above. |
| 340 | + * Chart families whose rendered SHAPE depends on `chartConfig` — today exactly |
| 341 | + * one, `combo`, whose per-series mark is authored as `chartConfig.series[].type` |
| 342 | + * and has no other channel. |
| 343 | + * |
| 344 | + * ⛔ This is NOT "the families that need a measure mapping". No family needs one: |
| 345 | + * `DatasetWidget` derives the axis key and one series per measure from the |
| 346 | + * widget's `dimensions` / `values`, and refuses an authored `ChartAxis.field` / |
| 347 | + * `ChartSeries.name` outright — the rule docblock's "What the renderer actually |
| 348 | + * derives" carries the pinned test names that establish it (#14436). The |
| 349 | + * previous spelling of this set was the taxonomy minus a hand-written exemption |
| 350 | + * list, which reported every bar, line and pie that never wrote a `chartConfig` |
| 351 | + * — including this platform's own `system_overview` tiles. |
289 | 352 | * |
290 | | - * Derived from `ChartTypeSchema` rather than restated. As a hand-written list it |
291 | | - * had no way to know when the taxonomy grew, and the omission is silent in |
292 | | - * exactly the wrong direction: an unlisted family is treated as "not a chart", |
293 | | - * so a widget missing its measure mapping passes validation instead of being |
294 | | - * reported. objectui#2945. |
| 353 | + * Membership is checked against `ChartTypeSchema` in this rule's tests rather |
| 354 | + * than derived from it: a family belongs here because of what its RENDERER does |
| 355 | + * with `chartConfig`, which a taxonomy cannot know. objectui#2945's lesson — |
| 356 | + * that a hand-written list cannot notice the taxonomy growing — survives as |
| 357 | + * that check: a member that stops being a declared chart type reds. |
295 | 358 | */ |
296 | | -const CHART_TYPES = new Set<string>( |
297 | | - ChartTypeSchema.options.filter(t => !MEASURE_EXEMPT_CHART_TYPES.has(t)), |
298 | | -); |
| 359 | +export const MARK_MIXING_CHART_TYPES = new Set<string>(['combo']); |
299 | 360 |
|
300 | 361 | function list(names: Iterable<string>): string { |
301 | 362 | const arr = [...names]; |
@@ -863,7 +924,11 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { |
863 | 924 | const chartConfig = (w.chartConfig && typeof w.chartConfig === 'object') |
864 | 925 | ? (w.chartConfig as AnyRec) |
865 | 926 | : undefined; |
866 | | - const isChartType = typeof w.type === 'string' && CHART_TYPES.has(w.type); |
| 927 | + // [#14436] Not "is this a chart" — the renderer derives every chart's |
| 928 | + // binding from `dimensions`/`values`. This asks the only question a |
| 929 | + // MISSING `chartConfig` can still answer badly: does this family carry |
| 930 | + // its shape in `chartConfig`? |
| 931 | + const isMarkMixing = typeof w.type === 'string' && MARK_MIXING_CHART_TYPES.has(w.type); |
867 | 932 |
|
868 | 933 | if (chartConfig) { |
869 | 934 | // The query result carries the widget's selected dimensions and |
@@ -916,17 +981,23 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { |
916 | 981 | const name = series[k]?.name; |
917 | 982 | if (typeof name === 'string') measureField(`series[${k}].name`, name); |
918 | 983 | } |
919 | | - } else if (isChartType) { |
| 984 | + } else if (isMarkMixing) { |
920 | 985 | push({ |
921 | 986 | severity: 'warning', |
922 | 987 | rule: CHART_CONFIG_MISSING, |
923 | 988 | message: |
924 | | - `chart-type widget ('${w.type}') has no chartConfig — the renderer ` + |
925 | | - `cannot determine which measure to plot, so the series renders empty.`, |
| 989 | + `'${w.type}' widget has no chartConfig — a combination chart takes its ` + |
| 990 | + `per-series mark from \`chartConfig.series[].type\`, and with none declared ` + |
| 991 | + `every measure (${list(values)}) draws with the same default mark, so the ` + |
| 992 | + `chart is not a combination at all. The data and the axis are unaffected: ` + |
| 993 | + `the renderer derives those from this widget's dimensions and values.`, |
926 | 994 | hint: |
927 | | - `Add chartConfig with xAxis.field set to a dimension (${list(dims)}) and ` + |
928 | | - `yAxis[].field set to a measure name (${list(values)}). If the default ` + |
929 | | - `rendering is intentional, suppress with: suppressWarnings: ['${CHART_CONFIG_MISSING}']`, |
| 995 | + `Give each measure its mark — chartConfig: { series: [{ name: '<measure>', ` + |
| 996 | + `type: 'bar' | 'line' | 'area' }] } — naming measures this widget selects ` + |
| 997 | + `(${list(values)}). \`series[].name\` selects WHICH derived series the mark ` + |
| 998 | + `lands on; it cannot add, remove or re-point one. If one uniform mark is ` + |
| 999 | + `intentional, prefer that family's own widget type, or suppress with: ` + |
| 1000 | + `suppressWarnings: ['${CHART_CONFIG_MISSING}']`, |
930 | 1001 | }); |
931 | 1002 | } |
932 | 1003 |
|
|
0 commit comments