From fc06dd0cb94e28e78db09c98daa52cd70349fe91 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 04:47:23 +0000 Subject: [PATCH 1/2] fix(plugin-dashboard): keep schema-shaped props off the KPI card DOM (#4357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MetricWidget and MetricCard ended their prop lists with `...props` and spread it onto the Card. Reached through SchemaRenderer — every dashboard KPI tile is — that spread also received the node's own metadata, and React writes unknown lowercase attributes straight to the DOM, stringifying objects. Every KPI card carried `schema="[object Object]"`. Measured at the SchemaRenderer call site: six props arrive that are not HTML attribute names — schema, events, props, bind, ariaLabel, ariaDescribedBy. They are destructured out in both components; the spread survives for everything that IS one (id, name, role, disabled, aria-*, data-*, className), which is the components' only accessibility passthrough. The list and its measurement live once, in schemaHostProps.ts. The cost was never visible — it was that the defect poisoned the assertion this area attracts. #4032 had to assert on the card heading because the container pin was red for a reason unrelated to labels. That workaround is replaced by the container assertion it was standing in for. --- .changeset/metric-widget-dom-spread-4357.md | 38 ++++ packages/plugin-dashboard/src/MetricCard.tsx | 18 +- .../plugin-dashboard/src/MetricWidget.tsx | 18 +- .../DashboardRenderer.metricI18n.test.tsx | 29 ++- .../__tests__/MetricWidget.domProps.test.tsx | 188 ++++++++++++++++++ .../plugin-dashboard/src/schemaHostProps.ts | 62 ++++++ 6 files changed, 330 insertions(+), 23 deletions(-) create mode 100644 .changeset/metric-widget-dom-spread-4357.md create mode 100644 packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx create mode 100644 packages/plugin-dashboard/src/schemaHostProps.ts diff --git a/.changeset/metric-widget-dom-spread-4357.md b/.changeset/metric-widget-dom-spread-4357.md new file mode 100644 index 000000000..4fa2cbd59 --- /dev/null +++ b/.changeset/metric-widget-dom-spread-4357.md @@ -0,0 +1,38 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +KPI cards no longer write their own schema onto the DOM — `MetricWidget` and +`MetricCard` keep `SchemaRenderer`'s schema-shaped props out of the `...props` +spread (objectui#4357). + +Both components are two things at once: an SDUI block reached through +`SchemaRenderer`, and a plain React component a host may render directly. The +React half wants a `...props` spread on its root so callers can pass `aria-*`, +`data-*`, `id`, `role`. The SDUI half means that spread also received the node's +own metadata — and React writes unknown lowercase attributes straight to the DOM, +stringifying object values. Every KPI card therefore carried +`schema="[object Object]"`, and a widget authored with events, a binding or a +props container carried `events="[object Object]"`, `bind="data.revenue"` and +`props="[object Object]"` beside it. + +Six props were measured arriving at the call site that are not HTML attribute +names — `schema`, `events`, `props`, `bind`, `ariaLabel`, `ariaDescribedBy` (the +last two are the camelCase authored forms of ARIA the renderer already emits in +their dashed spelling). They are destructured out; the spread survives untouched +for everything that IS a DOM attribute: `id`, `name`, `role`, `disabled`, +`aria-*`, `data-*`, `className`. Nothing else about the render moves — no text, +no class, no element. + +The cost of this was never visible; it was that the defect poisoned the +assertion this area attracts. objectui#4163 pins +`not.toContain('[object Object]')` on the dashboard grid, and objectui#4032 +wanted the same pin on the metric path but could not write it: the card carried +the attribute before and after any i18n fix, so the container assertion was red +for a reason unrelated to labels and the tempting repair was to loosen it. That +suite asserted on the card heading instead, with a comment. The workaround is +now removed and the container assertion is back. + +The exported `MetricWidgetProps` / `MetricCardProps` interfaces are unchanged — +the components' accepted props widen only by the optional, ignored +`SchemaHostProps` keys, so no consumer type narrows. diff --git a/packages/plugin-dashboard/src/MetricCard.tsx b/packages/plugin-dashboard/src/MetricCard.tsx index a9878f4d5..0807c9197 100644 --- a/packages/plugin-dashboard/src/MetricCard.tsx +++ b/packages/plugin-dashboard/src/MetricCard.tsx @@ -12,6 +12,7 @@ import { cn } from '@object-ui/components'; import { useObjectTranslation, pickLocalized } from '@object-ui/i18n'; import type { I18nLabel } from '@object-ui/types'; import { ArrowDownIcon, ArrowUpIcon, MinusIcon, AlertCircle, Loader2 } from 'lucide-react'; +import type { SchemaHostProps } from './schemaHostProps'; export interface MetricCardProps { /** @@ -36,7 +37,7 @@ export interface MetricCardProps { * MetricCard - Standalone metric card component for dashboard KPIs * Displays a metric value with optional icon, trend indicator, and description */ -export const MetricCard: React.FC = ({ +export const MetricCard: React.FC = ({ title, value, icon, @@ -46,7 +47,18 @@ export const MetricCard: React.FC = ({ className, loading, error, - ...props + // Schema-shaped props `SchemaRenderer` injects, destructured out so the + // spread below cannot write them to the DOM (objectui#4357). Named and + // measured in `./schemaHostProps`; `schema` alone put a + // `schema="[object Object]"` attribute on every card. The rest spread + // survives — it is the component's genuine DOM/aria passthrough. + schema: _schema, + bind: _bind, + events: _events, + props: _propsBag, + ariaLabel: _ariaLabel, + ariaDescribedBy: _ariaDescribedBy, + ...domProps }) => { // Resolve icon via lazy resolver — each icon ships as its own micro-chunk const IconComponent = icon ? getLazyIcon(icon) : null; @@ -54,7 +66,7 @@ export const MetricCard: React.FC = ({ const { language } = useObjectTranslation(); return ( - + {pickLocalized(title, language)} diff --git a/packages/plugin-dashboard/src/MetricWidget.tsx b/packages/plugin-dashboard/src/MetricWidget.tsx index 794098178..7ce51fcd8 100644 --- a/packages/plugin-dashboard/src/MetricWidget.tsx +++ b/packages/plugin-dashboard/src/MetricWidget.tsx @@ -12,6 +12,7 @@ import { import type { I18nLabel } from '@object-ui/types'; import { ArrowDownIcon, ArrowUpIcon, MinusIcon, AlertCircle, Loader2 } from 'lucide-react'; import { VARIANT_ICON_CLASSES, VARIANT_TEXT_CLASSES, type MetricColorVariant } from './colorVariants'; +import type { SchemaHostProps } from './schemaHostProps'; const TREND_LABEL_DEFAULTS: Record = { 'dashboard.trend.vsLastQuarter': 'vs last quarter', @@ -210,8 +211,19 @@ export const MetricWidget = ({ suffix, onClick, variant = 'card', - ...props -}: MetricWidgetProps) => { + // Schema-shaped props `SchemaRenderer` injects, destructured out so the + // spread below cannot write them to the DOM (objectui#4357). Named and + // measured in `./schemaHostProps`; `schema` alone put a + // `schema="[object Object]"` attribute on every KPI card. The rest spread + // survives — it is the component's genuine DOM/aria passthrough. + schema: _schema, + bind: _bind, + events: _events, + props: _propsBag, + ariaLabel: _ariaLabel, + ariaDescribedBy: _ariaDescribedBy, + ...domProps +}: MetricWidgetProps & SchemaHostProps) => { const iconClasses = VARIANT_ICON_CLASSES[colorVariant] || VARIANT_ICON_CLASSES.default; const { t: tTrend } = useTrendT(); // Two locale channels, deliberately distinct. `useDisplayLocale` is the @@ -298,7 +310,7 @@ export const MetricWidget = ({ onClick(); } } : undefined} - {...props} + {...domProps} > diff --git a/packages/plugin-dashboard/src/__tests__/DashboardRenderer.metricI18n.test.tsx b/packages/plugin-dashboard/src/__tests__/DashboardRenderer.metricI18n.test.tsx index 490213b82..8c94a0703 100644 --- a/packages/plugin-dashboard/src/__tests__/DashboardRenderer.metricI18n.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DashboardRenderer.metricI18n.test.tsx @@ -86,13 +86,6 @@ function dashboard(widget: Record): DashboardComponentSchema { } as unknown as DashboardComponentSchema; } -/** The KPI card's heading row — the slot every assertion below is about. */ -function cardHeading(): HTMLElement { - const el = document.querySelector('.tracking-tight.text-sm.font-medium'); - if (!el) throw new Error('metric card heading not rendered'); - return el as HTMLElement; -} - function renderIn(language: string, schema: DashboardComponentSchema) { return render( @@ -116,7 +109,7 @@ describe('DashboardRenderer — KPI cards translate from the widget convention k }); it('(b) resolves an inline per-locale title map, and never leaks the widget type', () => { - renderIn('zh', dashboard({ + const { container } = renderIn('zh', dashboard({ id: 'unkeyed', type: 'metric', title: { en: 'Total Revenue', 'zh-CN': '总收入' }, @@ -128,17 +121,19 @@ describe('DashboardRenderer — KPI cards translate from the widget convention k // so `|| widgetType` rendered the literal string "metric". expect(screen.queryByText('metric')).toBeNull(); // And the sibling surface's failure mode, which this path never had. - // Asserted on the HEADING, not on `container.innerHTML`: the card also - // carries an unrelated `schema="[object Object]"` DOM attribute, present on - // this path before and after this change and on plain-string titles too — - // `SchemaRenderer` hands the widget schema down and `MetricWidget` spreads - // `...props` onto the `Card`. Filed separately rather than widened into - // this pin, which would have made the assertion pass for the wrong reason. - expect(cardHeading().innerHTML).not.toContain('[object Object]'); + // Asserted on the CONTAINER, which is where this pin belongs and where + // #4163 puts the sibling one. It used to be scoped to the card heading: + // the card carried an unrelated `schema="[object Object]"` attribute on + // every render — `SchemaRenderer` hands the widget schema down and + // `MetricWidget` spread `...props` onto the `Card` — so the container + // assertion was red for a reason that had nothing to do with labels, and + // the tempting repair was to delete it. objectui#4357 removed the leak at + // its source (`src/schemaHostProps.ts`), so the assertion is writable now. + expect(container.innerHTML).not.toContain('[object Object]'); }); it('(b2) resolves the inline map for `en` too — the authored en text, not the raw map', () => { - renderIn('en', dashboard({ + const { container } = renderIn('en', dashboard({ id: 'unkeyed', type: 'metric', title: { en: 'Total Revenue', 'zh-CN': '总收入' }, @@ -146,7 +141,7 @@ describe('DashboardRenderer — KPI cards translate from the widget convention k })); expect(screen.getByText('Total Revenue')).toBeTruthy(); - expect(cardHeading().innerHTML).not.toContain('[object Object]'); + expect(container.innerHTML).not.toContain('[object Object]'); }); it('(b3) prefers the bundle over the inline map when both exist', () => { diff --git a/packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx b/packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx new file mode 100644 index 000000000..46414d461 --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx @@ -0,0 +1,188 @@ +/** + * 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#4357 — `MetricWidget` / `MetricCard` ended their prop lists with + * `...props` and spread the whole thing onto the Shadcn `Card`. Reached through + * `SchemaRenderer` (every KPI tile is), the renderer hands a component its + * widget schema plus the schema's own keys, so SDUI metadata landed on the DOM: + * React passes unknown lowercase attributes straight through and stringifies + * object values. + * + * MEASURED at the `SchemaRenderer` call site (`packages/react/src/SchemaRenderer.tsx`, + * the `React.createElement(Component, …)` block) — the props a widget receives + * that are NOT DOM attributes, and the attribute each one emitted before the fix: + * + * | prop | emitted as | what it is | + * |------------------|-------------------------------|-------------------------------------| + * | `schema` | `schema="[object Object]"` | the node, injected on EVERY render | + * | `events` | `events="[object Object]"` | SDUI action metadata | + * | `props` | `props="[object Object]"` | the props container (spread already)| + * | `bind` | `bind="data.revenue"` | SDUI data-binding path | + * | `ariaLabel` | `arialabel="…"` | camelCase twin of `aria-label` | + * | `ariaDescribedBy`| `ariadescribedby="…"` | camelCase twin of `aria-describedby`| + * + * The line the fix draws is "is this an HTML attribute name": the six above are + * not (the renderer already emits the two dashed `aria-*` forms itself), so they + * are destructured out. Everything that IS one keeps flowing through the spread — + * `id`, `name`, `role`, `disabled`, `aria-*`, `data-*`, `className` — because the + * spread is genuine DOM/aria passthrough and stays. + * + * WHY THE ASSERTION IS `container.innerHTML` AND NOT THE HEADING. This defect's + * cost was never a visible one; it was that it POISONED the assertion this area + * attracts. #4163 pinned `not.toContain('[object Object]')` on + * `DashboardGridLayout`, and #4032 wanted the same pin on the metric path but + * could not write it — the card carried `schema="[object Object]"` before and + * after any i18n fix, so the pin would have been red for a reason unrelated to + * labels, and the tempting repair (loosen it) throws the real pin away. #4032 + * asserted on the card heading instead, with a comment pointing here. That + * workaround is now removed: the container assertion below is the one that could + * not be written, and `DashboardRenderer.metricI18n.test.tsx` asserts on the + * container again. + * + * DIRECTIONS, written before the run: (a)–(d) RED before the fix — (a)/(c)/(d) + * by `MetricWidget`, (b) by `MetricCard`; (e)/(f) GREEN on both sides, they are + * the acceptance boundary (nothing else about the render may move). + */ + +import * as React from 'react'; +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { I18nProvider } from '@object-ui/i18n'; +import type { DashboardComponentSchema } from '@object-ui/types'; +import { SchemaRenderer } from '@object-ui/react'; +// Both widgets are resolved from the registry, populated as a side effect of +// the package barrel. Imported at MODULE scope (never in a hook) per AGENTS.md's +// flaky-test rule: the cost lands in the import phase, unbounded by any timeout. +import { DashboardRenderer } from '../index'; + +afterEach(cleanup); + +/** Every SDUI key at once, so one render measures the whole surface. */ +const SCHEMA_METADATA = { + id: 'revenue', + name: 'revenue_kpi', + bind: 'data.revenue', + events: { onClick: [{ action: 'navigate', params: { url: '/deals' } }] }, + ariaLabel: 'Revenue KPI', + ariaDescribedBy: 'desc-1', + role: 'group', +} as const; + +/** Attribute names that must never appear — none of them is an HTML attribute. */ +const LEAKED_ATTRIBUTES = ['schema', 'events', 'props', 'bind', 'arialabel', 'ariadescribedby']; + +function renderSchema(schema: Record) { + return render( + + + , + ); +} + +function card(container: HTMLElement): HTMLElement { + const el = container.firstElementChild; + if (!el) throw new Error('nothing rendered'); + return el as HTMLElement; +} + +function attributeNames(el: HTMLElement): string[] { + return Array.from(el.attributes).map((a) => a.name); +} + +describe('MetricWidget / MetricCard — schema-shaped props stay off the DOM (#4357)', () => { + it('(a) MetricWidget: the metric container carries no stringified schema', () => { + const { container } = renderSchema({ + type: 'metric', + label: 'Total Revenue', + value: 1930000, + ...SCHEMA_METADATA, + props: { colorVariant: 'success' }, + }); + + // THE pin — the assertion this defect used to block, on the container. + expect(container.innerHTML).not.toContain('[object Object]'); + expect(attributeNames(card(container))).not.toContain('schema'); + }); + + it('(b) MetricCard: same, through its own registry type', () => { + const { container } = renderSchema({ + type: 'metric-card', + title: 'Total Revenue', + value: 1930000, + ...SCHEMA_METADATA, + }); + + expect(container.innerHTML).not.toContain('[object Object]'); + expect(attributeNames(card(container))).not.toContain('schema'); + }); + + it('(c) neither component emits any of the six measured non-DOM props', () => { + for (const schema of [ + { type: 'metric', label: 'Total Revenue', value: 1930000, ...SCHEMA_METADATA, props: { colorVariant: 'success' } }, + { type: 'metric-card', title: 'Total Revenue', value: 1930000, ...SCHEMA_METADATA, props: {} }, + ]) { + const { container } = renderSchema(schema); + const names = attributeNames(card(container)); + for (const leaked of LEAKED_ATTRIBUTES) { + expect(names, `${schema.type} emitted ${leaked}=`).not.toContain(leaked); + } + cleanup(); + } + }); + + it('(d) the dashboard KPI path — the container assertion #4032 could not write', () => { + const schema = { + type: 'dashboard', + name: 'sales', + widgets: [{ id: 'revenue', type: 'metric', title: 'Total Revenue', options: { value: 1930000 } }], + } as unknown as DashboardComponentSchema; + + const { container } = render( + + + , + ); + + expect(container.innerHTML).not.toContain('[object Object]'); + }); + + it('(e) genuine DOM / aria passthrough survives — the spread is not removed', () => { + const { container } = renderSchema({ + type: 'metric', + label: 'Total Revenue', + value: 1930000, + ...SCHEMA_METADATA, + className: 'kpi-tile', + }); + + const el = card(container); + // Authored DOM identity and ARIA, plus the renderer's own data attributes. + expect(el.getAttribute('id')).toBe('revenue'); + expect(el.getAttribute('name')).toBe('revenue_kpi'); + expect(el.getAttribute('role')).toBe('group'); + expect(el.getAttribute('aria-label')).toBe('Revenue KPI'); + expect(el.getAttribute('aria-describedby')).toBe('desc-1'); + expect(el.getAttribute('data-obj-id')).toBe('revenue'); + expect(el.getAttribute('data-obj-type')).toBe('metric'); + expect(el.className).toContain('kpi-tile'); + }); + + it('(f) rendered output is otherwise byte-identical — label and formatted value', () => { + const { container } = renderSchema({ + type: 'metric', + label: 'Total Revenue', + value: 1930000, + ...SCHEMA_METADATA, + }); + + expect(container.textContent).toContain('Total Revenue'); + // The KPI formatter's own contract: thousands separators, no decimals. + expect(container.textContent).toContain('1,930,000'); + }); +}); diff --git a/packages/plugin-dashboard/src/schemaHostProps.ts b/packages/plugin-dashboard/src/schemaHostProps.ts new file mode 100644 index 000000000..0ceffa0d2 --- /dev/null +++ b/packages/plugin-dashboard/src/schemaHostProps.ts @@ -0,0 +1,62 @@ +/** + * 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. + */ + +/** + * The props `SchemaRenderer` hands a widget that are NOT DOM attributes + * (objectui#4357). + * + * A dashboard widget is two things at once: an SDUI block reached through + * `SchemaRenderer`, and a plain React component a host may render directly. + * The React half wants a `...props` spread onto its root element so callers can + * pass `aria-*`, `data-*`, `id`, `role`, `className`. The SDUI half means that + * spread also receives the node's own metadata — and React writes unknown + * lowercase attributes straight to the DOM, stringifying object values. That is + * how every KPI card ended up with `schema="[object Object]"`. + * + * MEASURED at the call site (`packages/react/src/SchemaRenderer.tsx`, the + * `React.createElement(Component, …)` block), one render carrying every SDUI key + * at once. What a widget receives, and the attribute each one emitted: + * + * | prop | emitted as | what it is | + * |-------------------|------------------------------|--------------------------------------| + * | `schema` | `schema="[object Object]"` | the node itself, injected EVERY render | + * | `events` | `events="[object Object]"` | SDUI action metadata (`onClick: […]`) | + * | `props` | `props="[object Object]"` | the props container — the renderer already spreads its CONTENTS separately, so the container is pure metadata | + * | `bind` | `bind="data.revenue"` | SDUI data-binding path | + * | `ariaLabel` | `arialabel="…"` | camelCase authored form; the renderer already emits the resolved `aria-label` | + * | `ariaDescribedBy` | `ariadescribedby="…"` | ditto for `aria-describedby` | + * + * The line this type draws is **"is the key an HTML attribute name"**. None of + * the six is (the two dashed `aria-*` forms the renderer emits itself are, and + * they keep flowing). Everything that IS one stays in the spread and reaches the + * DOM exactly as before: `id`, `name`, `role`, `disabled`, `aria-*`, `data-*`, + * `className`. Removing the spread instead would have been the wrong fix — it is + * the component's only accessibility passthrough. + * + * The renderer strips its own schema metadata (`type` / `children` / `visible` / + * `dataSource` / …) before spreading, so those never arrive; this type covers + * only what survives that strip. Declared here rather than in each component + * because two copies of one key list is how a list becomes two disagreeing + * lists — the same reason `colorVariants` was extracted. The pin that keeps the + * declaration honest for BOTH components is + * `__tests__/MetricWidget.domProps.test.tsx`. + */ +export interface SchemaHostProps { + /** The widget's own schema node, injected by `SchemaRenderer` on every render. */ + schema?: unknown; + /** SDUI data-binding path (`'user.address.city'`). */ + bind?: unknown; + /** SDUI event handlers (`{ onClick: [ActionDef, …] }`) — data, not DOM. */ + events?: unknown; + /** The schema's props container; its contents are spread separately. */ + props?: unknown; + /** Authored camelCase ARIA; the renderer emits the resolved `aria-label`. */ + ariaLabel?: unknown; + /** Authored camelCase ARIA; the renderer emits `aria-describedby`. */ + ariaDescribedBy?: unknown; +} From d68a1cd6e12d2f7a054882b9d43d8ef9d9880cac Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 05:20:07 +0000 Subject: [PATCH 2/2] fix(plugin-dashboard): drop the injected dataSource adapter too (#4357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass measured only what a SCHEMA can carry, and every fixture in this package renders a dashboard without a data source — so `dataSource` read `undefined` and wrote nothing, in every test. Measured again with an adapter attached, the KPI card carried `datasource="[object Object]"`: the adapter is not a schema key at all (SchemaRenderer strips the schema's own `dataSource` binding by name, objectstack#5576) but the object DashboardRenderer hands its SchemaRenderer call, arriving through the renderer's trailing props. That is the only one of the seven that leaked on a production dashboard rather than an authored edge case, and the pin could not see it. Case (g) now renders the dashboard WITH an adapter, so the shape only deployments had is a test. --- .changeset/metric-widget-dom-spread-4357.md | 19 +++++-- packages/plugin-dashboard/src/MetricCard.tsx | 1 + .../plugin-dashboard/src/MetricWidget.tsx | 1 + .../__tests__/MetricWidget.domProps.test.tsx | 54 ++++++++++++++++--- .../plugin-dashboard/src/schemaHostProps.ts | 24 +++++++-- 5 files changed, 85 insertions(+), 14 deletions(-) diff --git a/.changeset/metric-widget-dom-spread-4357.md b/.changeset/metric-widget-dom-spread-4357.md index 4fa2cbd59..a56632a8f 100644 --- a/.changeset/metric-widget-dom-spread-4357.md +++ b/.changeset/metric-widget-dom-spread-4357.md @@ -16,13 +16,22 @@ stringifying object values. Every KPI card therefore carried props container carried `events="[object Object]"`, `bind="data.revenue"` and `props="[object Object]"` beside it. -Six props were measured arriving at the call site that are not HTML attribute +Seven props were measured arriving at the call site that are not HTML attribute names — `schema`, `events`, `props`, `bind`, `ariaLabel`, `ariaDescribedBy` (the last two are the camelCase authored forms of ARIA the renderer already emits in -their dashed spelling). They are destructured out; the spread survives untouched -for everything that IS a DOM attribute: `id`, `name`, `role`, `disabled`, -`aria-*`, `data-*`, `className`. Nothing else about the render moves — no text, -no class, no element. +their dashed spelling) and `dataSource`. They are destructured out; the spread +survives untouched for everything that IS a DOM attribute: `id`, `name`, `role`, +`disabled`, `aria-*`, `data-*`, `className`. Nothing else about the render moves +— no text, no class, no element. + +`dataSource` is the one that only a live dashboard shows. It is not a schema key +(the renderer strips the schema's own `dataSource` binding by name); it is the +injected adapter `DashboardRenderer` hands its `SchemaRenderer` call, which +arrives through the renderer's trailing props. Every fixture in this package +renders without an adapter, so it read `undefined` and wrote nothing — while +every deployment that actually loads data put `datasource="[object Object]"` on +the card. The pin renders a dashboard with an adapter so the case that only +production had is now a test. The cost of this was never visible; it was that the defect poisoned the assertion this area attracts. objectui#4163 pins diff --git a/packages/plugin-dashboard/src/MetricCard.tsx b/packages/plugin-dashboard/src/MetricCard.tsx index 0807c9197..b2e36c64c 100644 --- a/packages/plugin-dashboard/src/MetricCard.tsx +++ b/packages/plugin-dashboard/src/MetricCard.tsx @@ -58,6 +58,7 @@ export const MetricCard: React.FC = ({ props: _propsBag, ariaLabel: _ariaLabel, ariaDescribedBy: _ariaDescribedBy, + dataSource: _dataSource, ...domProps }) => { // Resolve icon via lazy resolver — each icon ships as its own micro-chunk diff --git a/packages/plugin-dashboard/src/MetricWidget.tsx b/packages/plugin-dashboard/src/MetricWidget.tsx index 7ce51fcd8..79716c153 100644 --- a/packages/plugin-dashboard/src/MetricWidget.tsx +++ b/packages/plugin-dashboard/src/MetricWidget.tsx @@ -222,6 +222,7 @@ export const MetricWidget = ({ props: _propsBag, ariaLabel: _ariaLabel, ariaDescribedBy: _ariaDescribedBy, + dataSource: _dataSource, ...domProps }: MetricWidgetProps & SchemaHostProps) => { const iconClasses = VARIANT_ICON_CLASSES[colorVariant] || VARIANT_ICON_CLASSES.default; diff --git a/packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx b/packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx index 46414d461..3cb8e675e 100644 --- a/packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/MetricWidget.domProps.test.tsx @@ -26,13 +26,24 @@ * | `bind` | `bind="data.revenue"` | SDUI data-binding path | * | `ariaLabel` | `arialabel="…"` | camelCase twin of `aria-label` | * | `ariaDescribedBy`| `ariadescribedby="…"` | camelCase twin of `aria-describedby`| + * | `dataSource` | `datasource="[object Object]"`| the injected data-source ADAPTER | * - * The line the fix draws is "is this an HTML attribute name": the six above are + * The line the fix draws is "is this an HTML attribute name": the seven above are * not (the renderer already emits the two dashed `aria-*` forms itself), so they * are destructured out. Everything that IS one keeps flowing through the spread — * `id`, `name`, `role`, `disabled`, `aria-*`, `data-*`, `className` — because the * spread is genuine DOM/aria passthrough and stays. * + * `dataSource` is why case (g) exists and why it renders the dashboard WITH an + * adapter. It is the only one of the seven that never appears in a schema — + * `SchemaRenderer` strips the schema's own `dataSource` binding by name + * (objectstack#5576) and this is the ADAPTER `DashboardRenderer` hands its + * `SchemaRenderer` call, arriving through the renderer's trailing props. Every + * fixture in this package renders without one, so it reads `undefined` and + * writes nothing; every live dashboard has one, so the attribute was there in + * production and in no test. A pin that only ever renders dataless dashboards + * cannot see it. + * * WHY THE ASSERTION IS `container.innerHTML` AND NOT THE HEADING. This defect's * cost was never a visible one; it was that it POISONED the assertion this area * attracts. #4163 pinned `not.toContain('[object Object]')` on @@ -45,9 +56,10 @@ * not be written, and `DashboardRenderer.metricI18n.test.tsx` asserts on the * container again. * - * DIRECTIONS, written before the run: (a)–(d) RED before the fix — (a)/(c)/(d) - * by `MetricWidget`, (b) by `MetricCard`; (e)/(f) GREEN on both sides, they are - * the acceptance boundary (nothing else about the render may move). + * DIRECTIONS, written before the run: (a)–(d) and (g) RED before the fix — + * (a)/(c)/(d)/(g) by `MetricWidget`, (b) by `MetricCard`; (e)/(f) GREEN on both + * sides, they are the acceptance boundary (nothing else about the render may + * move). */ import * as React from 'react'; @@ -75,7 +87,13 @@ const SCHEMA_METADATA = { } as const; /** Attribute names that must never appear — none of them is an HTML attribute. */ -const LEAKED_ATTRIBUTES = ['schema', 'events', 'props', 'bind', 'arialabel', 'ariadescribedby']; +const LEAKED_ATTRIBUTES = ['schema', 'events', 'props', 'bind', 'arialabel', 'ariadescribedby', 'datasource']; + +/** + * A data-source adapter, shaped like the one a live dashboard injects. Only its + * identity matters here: it must reach the widget and not the DOM. + */ +const ADAPTER = { name: 'fake', find: async () => ({ items: [] }), findOne: async () => null }; function renderSchema(schema: Record) { return render( @@ -122,7 +140,7 @@ describe('MetricWidget / MetricCard — schema-shaped props stay off the DOM (#4 expect(attributeNames(card(container))).not.toContain('schema'); }); - it('(c) neither component emits any of the six measured non-DOM props', () => { + it('(c) neither component emits any of the seven measured non-DOM props', () => { for (const schema of [ { type: 'metric', label: 'Total Revenue', value: 1930000, ...SCHEMA_METADATA, props: { colorVariant: 'success' } }, { type: 'metric-card', title: 'Total Revenue', value: 1930000, ...SCHEMA_METADATA, props: {} }, @@ -152,6 +170,30 @@ describe('MetricWidget / MetricCard — schema-shaped props stay off the DOM (#4 expect(container.innerHTML).not.toContain('[object Object]'); }); + it('(g) the dashboard KPI path WITH a data source — the production shape', () => { + const schema = { + type: 'dashboard', + name: 'sales', + widgets: [{ id: 'revenue', type: 'metric', title: 'Total Revenue', options: { value: 1930000 } }], + } as unknown as DashboardComponentSchema; + + const { container } = render( + + + , + ); + + const tile = container.querySelector('[data-obj-type="metric"]'); + if (!tile) throw new Error('metric tile not rendered'); + + // Case (d) renders the same dashboard with no adapter and cannot see this: + // `dataSource` is `undefined` there, so React writes nothing. + expect(container.innerHTML).not.toContain('[object Object]'); + expect(attributeNames(tile as HTMLElement)).not.toContain('datasource'); + // The KPI still renders its number — the adapter is dropped, not the widget. + expect(container.textContent).toContain('1,930,000'); + }); + it('(e) genuine DOM / aria passthrough survives — the spread is not removed', () => { const { container } = renderSchema({ type: 'metric', diff --git a/packages/plugin-dashboard/src/schemaHostProps.ts b/packages/plugin-dashboard/src/schemaHostProps.ts index 0ceffa0d2..0327e1c38 100644 --- a/packages/plugin-dashboard/src/schemaHostProps.ts +++ b/packages/plugin-dashboard/src/schemaHostProps.ts @@ -30,17 +30,29 @@ * | `bind` | `bind="data.revenue"` | SDUI data-binding path | * | `ariaLabel` | `arialabel="…"` | camelCase authored form; the renderer already emits the resolved `aria-label` | * | `ariaDescribedBy` | `ariadescribedby="…"` | ditto for `aria-describedby` | + * | `dataSource` | `datasource="[object Object]"` | the injected data-source ADAPTER | + * + * `dataSource` is the one a schema-only measurement misses, and the only one + * that leaks on a *production* dashboard rather than an authored edge case. + * It does not come from the node: `SchemaRenderer` strips the schema's own + * `dataSource` BINDING by name (objectstack#5576) — this is the adapter object + * `DashboardRenderer` hands its `SchemaRenderer` call, which arrives through the + * renderer's trailing `...props` and lands on whatever the widget spreads onto. + * A dashboard rendered without a data source (every test fixture) leaves it + * `undefined` and nothing shows; a dashboard rendered with one — every live + * deployment — put `datasource="[object Object]"` on the card. * * The line this type draws is **"is the key an HTML attribute name"**. None of - * the six is (the two dashed `aria-*` forms the renderer emits itself are, and + * the seven is (the two dashed `aria-*` forms the renderer emits itself are, and * they keep flowing). Everything that IS one stays in the spread and reaches the * DOM exactly as before: `id`, `name`, `role`, `disabled`, `aria-*`, `data-*`, * `className`. Removing the spread instead would have been the wrong fix — it is * the component's only accessibility passthrough. * * The renderer strips its own schema metadata (`type` / `children` / `visible` / - * `dataSource` / …) before spreading, so those never arrive; this type covers - * only what survives that strip. Declared here rather than in each component + * the schema's `dataSource` binding / …) before spreading, so those never + * arrive; this type covers only what survives that strip — including the + * adapter, which arrives by a different door. Declared here rather than in each component * because two copies of one key list is how a list becomes two disagreeing * lists — the same reason `colorVariants` was extracted. The pin that keeps the * declaration honest for BOTH components is @@ -59,4 +71,10 @@ export interface SchemaHostProps { ariaLabel?: unknown; /** Authored camelCase ARIA; the renderer emits `aria-describedby`. */ ariaDescribedBy?: unknown; + /** + * The injected data-source adapter, forwarded by `DashboardRenderer` through + * `SchemaRenderer`'s trailing props. Neither metric component reads it — the + * async, object-aware KPI is `ObjectMetricWidget`. + */ + dataSource?: unknown; }