diff --git a/.changeset/6665-data-table-non-array-data-diagnostic.md b/.changeset/6665-data-table-non-array-data-diagnostic.md new file mode 100644 index 0000000000..4bd694e716 --- /dev/null +++ b/.changeset/6665-data-table-non-array-data-diagnostic.md @@ -0,0 +1,45 @@ +--- +'@object-ui/components': patch +--- + +A non-array `data` authored on a `data-table` node is now named at render +instead of dropped in silence (objectui#6665). + +`DataTableRenderer` takes its rows from `data: rawData = EMPTY_ROWS` off the +node and then collapses `Array.isArray(rawData) ? rawData : EMPTY_ROWS`. Any +non-array value an author wrote therefore becomes zero rows with no error and +no warning, and the table draws a correct-looking header over `No results +found` — which reads as a success receipt, the hardest failure shape for a +human or an AI author to self-check. + +The spelling that opened the card is a `${...}` expression string, and it is a +defect rather than a design because the SAME expression is evaluated one key +over. Re-measured on merge-base `5967be095` through the real `SchemaRenderer` +(the table was previously quoted from `skills/objectui/rules/protocol.md` as a +measurement on `f1c27f037` and had not been re-run); all four legs reproduced, +and they are now pinned as tests rather than prose: + +| node | body | +|---|---| +| `{ "data": "${data.customers}" }` | `No results found` | +| `{ "props": { "data": "${data.customers}" } }` | `No results found` | +| `{ "properties": { "data": "${data.customers}" } }` | the two rows | +| `{ "data": [ two literal records ] }` | the two rows | + +The predicate is deliberately WIDER than the reported spelling: `data` authored +and not an array. The `${...}` shape only selects a sharper sentence, because +the swallow at `Array.isArray(...)` is general — a number, an object, a `null` +and a plain string are dropped exactly as silently, and a predicate keyed on +the expression shape would leave each of them to arrive as a fresh card. + +It reuses objectui#6575's channel (`dataTableBindDiagnostic.ts`) as a SECOND +predicate rather than a widened one. The nodes that trip this carry no `bind` +at all, so that diagnostic's silence on them is correct behaviour, not a gap. + +No behaviour change: node-level `data` still does not evaluate expressions. +Making it do so is a behaviour change on a published component and was ruled to +the maintainer, not to this change. Nothing is added to the published surface +either — the new predicate, message builder and prefix constant are +module-internal and are not re-exported from the package entry, matching +objectui#6575's own symbols. The trap stops being silent; it does not stop +being a trap. diff --git a/packages/components/src/__tests__/data-table-node-data-diagnostic.test.tsx b/packages/components/src/__tests__/data-table-node-data-diagnostic.test.tsx new file mode 100644 index 0000000000..6a3aab752e --- /dev/null +++ b/packages/components/src/__tests__/data-table-node-data-diagnostic.test.tsx @@ -0,0 +1,274 @@ +/** + * 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#6665 — a `${...}` expression written into node-level `data` is not + * evaluated, and the table renders a correct-looking header over `No results + * found` with nothing thrown and nothing logged. + * + * ## Why this file RE-RUNS the measurement instead of quoting it + * + * The four-leg table below reached the card as a quotation from + * `skills/objectui/rules/protocol.md`, which states it as a measurement on + * `f1c27f037` — a commit nobody had re-run it against since. A behaviour table + * that lives only in prose ages silently: the day the renderer changes, the doc + * still reads as a measurement. Re-run on merge-base `5967be095` against a tree + * with none of this card's code in it, all four legs reproduced exactly, and + * they are pinned HERE from now on so the next change to any of them is a red + * test rather than a stale paragraph. + * + * ## The contrast IS the argument + * + * The same expression is evaluated under `properties` and not evaluated at node + * level. That side-by-side is what makes this a defect rather than a design + * choice, so all four legs are asserted together in one table-driven test + * rather than split across files where they could drift apart. + * + * ## What this card did NOT change + * + * Behaviour. Making node-level `data` evaluate expressions is a behaviour + * change on a published component, and the triage ruling put it on the + * maintainer floor rather than in this PR. Every RENDER assertion below + * therefore passes identically against the tree before the diagnostic existed; + * only the WARNING assertions can tell the two apart. That is deliberate: the + * trap stops being silent, it does not stop being a trap. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; + +// The REAL renderers, imported at module scope so `data-table` is in the +// registry before the first render (AGENTS.md §测试纪律 — never behind a lazy +// boundary inside a bounded window). The relative path is required: this file +// lives INSIDE `@object-ui/components`, and a bare specifier would be a package +// self-import (`scripts/check-package-self-import.mjs`). +import '../renderers'; +import { + DATA_TABLE_BIND_DIAGNOSTIC_PREFIX, + DATA_TABLE_DATA_DIAGNOSTIC_PREFIX, +} from '../renderers/complex/dataTableBindDiagnostic'; + +/** Identical in every leg, so the only variable is where `data` was written. */ +const COLUMNS = [ + { header: 'Name', accessorKey: 'name' }, + { header: 'Email', accessorKey: 'email' }, +]; + +const RECORDS = [ + { name: 'Ada Lovelace', email: 'ada@example.com' }, + { name: 'Grace Hopper', email: 'grace@example.com' }, +]; + +/** The provider really does hold the path every expression below spells. */ +const SCOPE = { customers: RECORDS }; + +const ROWS_ON_SCREEN = [ + 'Ada Lovelace', + 'ada@example.com', + 'Grace Hopper', + 'grace@example.com', +]; + +/** The single cell the empty state renders — one `tbody tr`, not zero. */ +const EMPTY_STATE = ['No results foundTry adjusting your filters or search query.']; + +/** Every rendered body cell's text, row-major. */ +function bodyCells(): string[] { + return Array.from(document.querySelectorAll('tbody td')).map((td) => (td.textContent ?? '').trim()); +} + +/** + * Console lines this render emitted on a given diagnostic channel. + * + * Filtered by the diagnostic's own prefix rather than by call count: these + * renders go through the REAL `SchemaRenderer` and the real registry, and an + * unrelated warning from some other component (react-i18next emits one here) + * must not be able to satisfy — or break — an assertion about this one. + */ +function warningsOn(prefix: string): string[] { + const spy = console.warn as unknown as { mock?: { calls: unknown[][] } }; + return (spy.mock?.calls ?? []) + .map((args) => String(args[0])) + .filter((line) => line.startsWith(prefix)); +} + +function tree(schema: unknown) { + return ( + + + + ); +} + +function renderNode(schema: unknown) { + return render(tree(schema)); +} + +describe('data-table node-level `data` — the four-leg table, re-measured (#6665)', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + it.each([ + [ + 'node-level `data` holding a `${...}` expression', + { type: 'data-table', data: '${data.customers}', columns: COLUMNS }, + EMPTY_STATE, + ], + [ + 'the same expression under the `props` envelope', + { type: 'data-table', props: { data: '${data.customers}' }, columns: COLUMNS }, + EMPTY_STATE, + ], + [ + 'the same expression under `properties`', + { type: 'data-table', properties: { data: '${data.customers}' }, columns: COLUMNS }, + ROWS_ON_SCREEN, + ], + [ + 'node-level `data` holding the literal array', + { type: 'data-table', data: RECORDS, columns: COLUMNS }, + ROWS_ON_SCREEN, + ], + ] as const)('%s', (_label, node, expected) => { + renderNode(node); + expect(bodyCells()).toEqual(expected); + }); + + it('the header is correct in the failing leg — which is why it reads as success', () => { + // The failure shape the card is about: nothing on screen says anything went + // wrong. Measured, rather than asserted from the card's description. + renderNode({ type: 'data-table', data: '${data.customers}', columns: COLUMNS }); + const headers = Array.from(document.querySelectorAll('thead th')).map((th) => + (th.textContent ?? '').trim(), + ); + expect(headers).toContain('Name'); + expect(headers).toContain('Email'); + expect(screen.getByText('No results found')).toBeInTheDocument(); + expect(document.querySelectorAll('tbody tr')).toHaveLength(1); + }); +}); + +describe('data-table node-level `data` — the diagnostic (#6665)', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('names the expression string that was swallowed', () => { + renderNode({ + type: 'data-table', + id: 'customers-table', + caption: 'Customers', + data: '${data.customers}', + columns: COLUMNS, + }); + + // Behaviour stays pinned above; THIS is the load-bearing half of the card — + // every other assertion in this file passes identically against the tree + // before the diagnostic existed. + const warnings = warningsOn(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("id: 'customers-table'"); + expect(warnings[0]).toContain("`data: '${data.customers}'` was never evaluated"); + expect(warnings[0]).toContain('at node level is read as a literal string'); + expect(warnings[0]).toContain('renders its header over an empty body'); + expect(warnings[0]).toContain('Resolve the rows in the host'); + + // The #6575 channel stays silent: this node carries no `bind`, and the + // ruling is explicit that its silence here is correct, not a gap. + expect(warningsOn(DATA_TABLE_BIND_DIAGNOSTIC_PREFIX)).toEqual([]); + }); + + it.each([ + ['a number', 42, 'the number `42`'], + ['null', null, '`null`'], + ['an object', { rows: RECORDS }, 'an object'], + ['a plain string', 'customers', "the string 'customers'"], + ] as const)('covers the general non-array case: %s', (_label, value, expected) => { + // The ruling's second constraint. `data-table.tsx`'s + // `Array.isArray(rawData) ? rawData : EMPTY_ROWS` swallows every one of + // these exactly as silently as the `${...}` string, so a diagnostic keyed + // on the expression shape alone would leave each to arrive as a fresh card. + renderNode({ type: 'data-table', data: value, columns: COLUMNS }); + + expect(bodyCells()).toEqual(EMPTY_STATE); + const warnings = warningsOn(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(expected); + expect(warnings[0]).toContain('takes its rows only from an array'); + // Only the `${...}` value earns the sharper sentence. + expect(warnings[0]).not.toContain('was never evaluated'); + }); + + it('says it ONCE across re-renders that rebuild the node', () => { + // The rate limit is the effect key, and the key is the MESSAGE rather than + // the raw value on purpose: an authored object is a fresh reference on + // every render that rebuilds the node, so a `rawData`-keyed effect would + // reprint the same line indefinitely. Two renders of an equal-but-new + // object is the cheapest reading that tells those two keys apart. + const { rerender } = render( + tree({ type: 'data-table', data: { rows: RECORDS }, columns: COLUMNS }), + ); + rerender(tree({ type: 'data-table', data: { rows: RECORDS }, columns: COLUMNS })); + + expect(warningsOn(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toHaveLength(1); + }); + + it('stays silent on every leg that puts rows on screen', () => { + // The zeros that keep this diagnostic worth reading. Each is a reading + // rather than a dead code path because the tests above find a line through + // the same helper, on the same channel, one `data` value apart. + for (const node of [ + { type: 'data-table', data: RECORDS, columns: COLUMNS }, + { type: 'data-table', properties: { data: '${data.customers}' }, columns: COLUMNS }, + ]) { + const { unmount } = renderNode(node); + expect(bodyCells()).toEqual(ROWS_ON_SCREEN); + expect(warningsOn(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toEqual([]); + unmount(); + } + }); + + it('stays silent on a table that authored no `data` at all', () => { + // An absent key is not an authoring mistake — a `data-table` awaiting rows + // is ordinary. Warning here would fire on tables that are merely empty, + // which is how a diagnostic teaches authors to ignore it. + renderNode({ type: 'data-table', columns: COLUMNS }); + expect(bodyCells()).toEqual(EMPTY_STATE); + expect(warningsOn(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toEqual([]); + }); + + it('is not fooled by an empty array — that is a real, authored answer', () => { + renderNode({ type: 'data-table', data: [], columns: COLUMNS }); + expect(bodyCells()).toEqual(EMPTY_STATE); + expect(warningsOn(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toEqual([]); + }); + + it('does NOT reach into the `props` envelope — a separate, wider defect', () => { + // An honest boundary, recorded rather than quietly left out. `properties.*` + // is HOISTED onto the node by `SchemaRenderer` (which is why leg 3 above + // renders rows); `props` is not — it is spread as React props, and + // `DataTableRenderer` reads only `schema`. So node-level `data` is + // genuinely ABSENT here and this diagnostic is correctly silent. Diagnosing + // the dropped envelope is a `SchemaRenderer`-level question affecting every + // renderer that reads `schema`, not a data-table one; widening this + // predicate to `props.data` would patch one component against a repo-wide + // problem. + renderNode({ type: 'data-table', props: { data: '${data.customers}' }, columns: COLUMNS }); + expect(bodyCells()).toEqual(EMPTY_STATE); + expect(warningsOn(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toEqual([]); + }); +}); diff --git a/packages/components/src/renderers/complex/__tests__/data-table-bind-diagnostic.test.ts b/packages/components/src/renderers/complex/__tests__/data-table-bind-diagnostic.test.ts index bebdd656af..04e2c43ae1 100644 --- a/packages/components/src/renderers/complex/__tests__/data-table-bind-diagnostic.test.ts +++ b/packages/components/src/renderers/complex/__tests__/data-table-bind-diagnostic.test.ts @@ -24,8 +24,11 @@ import { describe, it, expect } from 'vitest'; import { describeIgnoredBind, + describeNonArrayData, hasAuthoredBind, + hasNonArrayAuthoredData, DATA_TABLE_BIND_DIAGNOSTIC_PREFIX, + DATA_TABLE_DATA_DIAGNOSTIC_PREFIX, } from '../dataTableBindDiagnostic'; const ADDRESS = { blockType: 'data-table', id: 'customers-table', caption: 'Customers' }; @@ -119,3 +122,161 @@ describe('describeIgnoredBind — what the author is told (#6575)', () => { expect(message).not.toContain('()'); }); }); + +/** + * objectui#6665 — the SECOND question this module asks: `data` was authored and + * it is not an array. + * + * Same discipline as the block above: every zero is paired with a positive + * control in the same call shape, so "silent for X" is a reading rather than a + * code path that never ran. + * + * The rendered half — these lines reaching the console through the REAL + * `SchemaRenderer`, and the four-leg render table that says the defect is real + * — is pinned in `src/__tests__/data-table-node-data-diagnostic.test.tsx`. + */ +describe('hasNonArrayAuthoredData — absence and arrays are the only silence (#6665)', () => { + it('is false for an omitted key, and true for the reported spelling', () => { + expect(hasNonArrayAuthoredData(undefined)).toBe(false); + // The positive control in the same shape: the `${...}` string that opened + // the card really is caught. + expect(hasNonArrayAuthoredData('${data.customers}')).toBe(true); + }); + + it('is false for any array — the shape the renderer actually wants', () => { + expect(hasNonArrayAuthoredData([])).toBe(false); + expect(hasNonArrayAuthoredData(ROWS)).toBe(false); + }); + + it('is true for every non-array an author can type, not just `${...}`', () => { + // `data-table.tsx`'s `Array.isArray(rawData) ? rawData : EMPTY_ROWS` drops + // ALL of these to zero rows with the same silence. A predicate keyed on the + // expression shape would leave each of them to arrive as a fresh card. + expect(hasNonArrayAuthoredData(null)).toBe(true); + expect(hasNonArrayAuthoredData(42)).toBe(true); + expect(hasNonArrayAuthoredData(0)).toBe(true); + expect(hasNonArrayAuthoredData('')).toBe(true); + expect(hasNonArrayAuthoredData('customers')).toBe(true); + expect(hasNonArrayAuthoredData({ rows: ROWS })).toBe(true); + expect(hasNonArrayAuthoredData(false)).toBe(true); + }); +}); + +describe('describeNonArrayData — silence, and the control that earns it (#6665)', () => { + it('says nothing when `data` was not authored', () => { + expect(describeNonArrayData(undefined, ADDRESS)).toBeNull(); + // Counter-probe: the SAME call with an authored non-array does speak. + expect(describeNonArrayData('${data.customers}', ADDRESS)).not.toBeNull(); + }); + + it('says nothing about a real array, empty or not', () => { + // An empty table is a legitimate authoring outcome; only a DROPPED value is + // a defect. Warning here would fire on every table that happens to have no + // rows, which is how a diagnostic teaches authors to ignore it. + expect(describeNonArrayData([], ADDRESS)).toBeNull(); + expect(describeNonArrayData(ROWS, ADDRESS)).toBeNull(); + }); +}); + +describe('describeNonArrayData — the expression-shaped string (#6665)', () => { + const message = describeNonArrayData('${data.customers}', ADDRESS) as string; + + it('is on its own channel and names the node', () => { + expect(message.startsWith(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toBe(true); + expect(message).toContain("id: 'customers-table'"); + expect(message).toContain("caption: 'Customers'"); + }); + + it('quotes what was written and names what happened to it', () => { + expect(message).toContain("`data: '${data.customers}'` was never evaluated"); + expect(message).toContain('at node level is read as a literal string'); + expect(message).toContain('renders its header over an empty body'); + }); + + it('gives the way out the guides actually teach', () => { + expect(message).toContain('Resolve the rows in the host'); + expect(message).toContain('(objectui#6665)'); + // Deliberately NOT `properties`. The same expression IS evaluated there, + // and that contrast is what makes this a defect — but whether `properties` + // is an authoring channel for `ui:*` is an open contract question, and a + // console line is the wrong place to settle it. + expect(message).not.toContain('properties'); + }); + + it('needs a CLOSING brace — a lone `${` in prose is not an expression', () => { + const prose = describeNonArrayData('cost is ${ per seat', ADDRESS) as string; + expect(prose).not.toContain('was never evaluated'); + // Control, one closing brace apart. + expect(describeNonArrayData('${x}', ADDRESS)).toContain('was never evaluated'); + }); + + it('bounds a long value instead of dumping it into the console', () => { + const long = describeNonArrayData(`\${data.${'x'.repeat(200)}}`, ADDRESS) as string; + expect(long).toContain('…'); + expect(long.length).toBeLessThan(600); + }); +}); + +describe('describeNonArrayData — the general non-array fallback (#6665)', () => { + // The ruling's second constraint: `data-table.tsx:784` swallows ANY non-array, + // so the fallback covers "authored and not an array" rather than the `${...}` + // shape. Otherwise the next non-array spelling is just a fresh card. + it.each([ + [42, 'the number `42`'], + [null, '`null`'], + [false, 'the boolean `false`'], + [{ rows: 1 }, 'an object'], + ['customers', "the string 'customers'"], + ] as const)('names what was written: %o', (value, expected) => { + const message = describeNonArrayData(value, ADDRESS) as string; + expect(message).not.toBeNull(); + expect(message.startsWith(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toBe(true); + expect(message).toContain(expected); + expect(message).toContain('takes its rows only from an array'); + expect(message).toContain('renders its header over an empty body'); + expect(message).toContain('Resolve the rows in the host'); + }); + + it('does not claim a plain string was an unevaluated expression', () => { + // The sharper wording is only for a value that really carries `${...}`; + // saying it of `"customers"` would send the author hunting for an + // expression they never wrote. + const plain = describeNonArrayData('customers', ADDRESS) as string; + expect(plain).not.toContain('was never evaluated'); + // Control: the same assertion the other way, one `${}` apart. + expect(describeNonArrayData('${customers}', ADDRESS)).toContain('was never evaluated'); + }); + + it('falls back to the node type when the node has no id or caption', () => { + const message = describeNonArrayData(42, {}) as string; + expect(message).toContain(`${DATA_TABLE_DATA_DIAGNOSTIC_PREFIX} data-table —`); + }); +}); + +describe('the two diagnostics ask different questions (#6575 vs #6665)', () => { + // The ruling is explicit that #6575 staying silent on these nodes is CORRECT + // behaviour, not a gap: its predicate is keyed on an authored `bind`, and + // these nodes carry none. That is why a second predicate exists rather than + // the first one being widened — pinned here so nobody "fixes" the silence. + const NODE_DATA = '${data.customers}'; + + it('#6575 is silent on a node with a string `data` and no `bind`', () => { + expect(describeIgnoredBind(undefined, [], ADDRESS)).toBeNull(); + // ...and #6665 is not. Same node, two questions, one answer each. + expect(describeNonArrayData(NODE_DATA, ADDRESS)).not.toBeNull(); + }); + + it('#6665 is silent on a node with a `bind` and a real `data` array', () => { + expect(describeNonArrayData(ROWS, ADDRESS)).toBeNull(); + // ...and #6575 is not. + expect(describeIgnoredBind('customers', ROWS, ADDRESS)).not.toBeNull(); + }); + + it('both speak when both keys are wrong, on distinguishable channels', () => { + const bind = describeIgnoredBind('customers', [], ADDRESS) as string; + const data = describeNonArrayData(NODE_DATA, ADDRESS) as string; + expect(bind.startsWith(DATA_TABLE_BIND_DIAGNOSTIC_PREFIX)).toBe(true); + expect(data.startsWith(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX)).toBe(true); + expect(DATA_TABLE_BIND_DIAGNOSTIC_PREFIX).not.toBe(DATA_TABLE_DATA_DIAGNOSTIC_PREFIX); + }); +}); diff --git a/packages/components/src/renderers/complex/data-table.tsx b/packages/components/src/renderers/complex/data-table.tsx index fa955a6006..3213a8c360 100644 --- a/packages/components/src/renderers/complex/data-table.tsx +++ b/packages/components/src/renderers/complex/data-table.tsx @@ -11,7 +11,7 @@ import React, { useState, useMemo, useRef, useEffect, useLayoutEffect } from 're import { cn } from '../../lib/utils'; import { resolveIcon } from '../action/resolve-icon'; import { useGridFieldAuthoring } from '../../context/gridFieldAuthoring'; -import { describeIgnoredBind } from './dataTableBindDiagnostic'; +import { describeIgnoredBind, describeNonArrayData } from './dataTableBindDiagnostic'; import { ComponentRegistry, compareSortValues, evalRowPredicate, getSortValue } from '@object-ui/core'; import type { DataTableSchema, TableSortItem, TableColumnType } from '@object-ui/types'; import { SchemaRenderer, useRowPredicate, usePredicateScope } from '@object-ui/react'; @@ -781,6 +781,11 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { // must not reach array operations like .filter() / .some(). The non-array // fallback is the shared empty, so a provider-config schema does not re-key // every downstream memo on each render (objectui#4618). + // + // This branch is ALSO the objectui#6665 defect: it swallows an AUTHORED + // non-array as quietly as an absent key — a `${...}` expression string, a + // number, an object, a `null`. The behaviour is deliberate and unchanged + // here; the second effect below is what stops it being silent. const data = Array.isArray(rawData) ? rawData : EMPTY_ROWS; // objectui#6575 — say out loud that an authored `bind` was ignored. @@ -801,6 +806,32 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { if (message) console.warn(message); }, [authoredBind, data, bindDiagnosticBlockType, bindDiagnosticId, caption]); + // objectui#6665 — say out loud that a non-array `data` was dropped. + // + // The SAME channel as the effect above (one module, one `console.warn`, an + // effect key as the rate limit) asking a DIFFERENT question: these nodes + // carry no `bind` at all, so the #6575 predicate is correctly silent on them. + // A second effect rather than a wider key on that one, because the two + // judgements are independent and #6575's key is pinned by its own tests. + // + // The message is computed in render and IS the effect key, rather than the + // effect being keyed on `rawData`. Two reasons, and neither is style: + // - `data` (the collapsed value) cannot be the key — it is already + // `EMPTY_ROWS` for every value this diagnostic fires on, so one bad value + // replaced by another would not re-key and the second would go unsaid. + // - `rawData` cannot be the key either — an authored OBJECT is a fresh + // reference on every render that rebuilds the node, which would print the + // same line again and again. Keying on the message keeps the ceiling this + // module documents: one line per distinct authoring bug. + const nonArrayDataMessage = describeNonArrayData(rawData, { + blockType: bindDiagnosticBlockType, + id: bindDiagnosticId, + caption, + }); + useEffect(() => { + if (nonArrayDataMessage) console.warn(nonArrayDataMessage); + }, [nonArrayDataMessage]); + // The adapter reads the column keys `TableColumn` DECLARES. The `label` // alias is gone (objectui#5351); the `name` alias is HELD, and the hold is // deliberate and documented rather than an oversight. diff --git a/packages/components/src/renderers/complex/dataTableBindDiagnostic.ts b/packages/components/src/renderers/complex/dataTableBindDiagnostic.ts index c14c806ca6..654079235c 100644 --- a/packages/components/src/renderers/complex/dataTableBindDiagnostic.ts +++ b/packages/components/src/renderers/complex/dataTableBindDiagnostic.ts @@ -7,11 +7,25 @@ */ /** - * The diagnostic that says out loud what `data-table` has always done silently - * with an authored `bind` — nothing (objectui#6575, maintainer ruling - * 2026-08-27, option A: 「同意」). + * The diagnostics that say out loud what `data-table` has always done silently + * with a key the AUTHOR wrote. * - * ## The defect this names + * TWO questions, one channel. The file is named for the first because it came + * first (objectui#6575); it was never limited to it. + * + * 1. `bind` was authored — and `data-table` does not read `bind` at all + * (objectui#6575, maintainer ruling 2026-08-27, option A: 「同意」). + * 2. `data` was authored and is NOT an array — so the renderer dropped it to + * zero rows at `data-table.tsx`'s + * `Array.isArray(rawData) ? rawData : EMPTY_ROWS` (objectui#6665). + * + * They are deliberately two predicates asking two questions, not one widened + * predicate. The nodes that trip #2 carry no `bind` AT ALL, so #6575 staying + * silent on them is correct behaviour rather than a gap — the triage ruling on + * objectui#6665 is explicit about that, and widening #6575's question would + * have made its silence look like the defect. See {@link describeNonArrayData}. + * + * ## The first defect this names * * `bind` is the data-scope binding vocabulary: a path string resolved by * `useDataScope()`. `list`, `tree-view` and the `object-*` plugin widgets read @@ -68,14 +82,14 @@ * looking at the rows the renderer actually resolved. */ -/** Prefix for every line this module emits — the handle tests and greps hold. */ +/** Prefix for every `bind` line — the handle tests and greps hold. */ export const DATA_TABLE_BIND_DIAGNOSTIC_PREFIX = '[ObjectUI] DataTable bind:'; /** The key `data-table` really reads its rows from. */ export const DECLARED_ROWS_KEY = 'data'; -/** Where the offending node lives, for the first line of the message. */ -export interface DataTableBindAddress { +/** Where the offending node lives, for the first line of either message. */ +export interface DataTableNodeAddress { /** The schema node's `type` — `data-table`, or an alias that routes here. */ blockType?: unknown; /** The node's `id`, when it has one. */ @@ -88,7 +102,7 @@ function quote(value: unknown): string { return typeof value === 'string' ? `'${value}'` : String(value); } -function describeAddress({ blockType, id, caption }: DataTableBindAddress): string { +function describeAddress({ blockType, id, caption }: DataTableNodeAddress): string { const block = typeof blockType === 'string' && blockType.length > 0 ? blockType : 'data-table'; const parts: string[] = []; if (typeof id === 'string' && id.length > 0) parts.push(`id: '${id}'`); @@ -124,7 +138,7 @@ export function hasAuthoredBind(bind: unknown): boolean { export function describeIgnoredBind( bind: unknown, rows: unknown, - address: DataTableBindAddress, + address: DataTableNodeAddress, ): string | null { if (!hasAuthoredBind(bind)) return null; @@ -142,3 +156,125 @@ export function describeIgnoredBind( + ` Put the rows in \`${DECLARED_ROWS_KEY}\`, or author a component that does read \`bind\` ` + '(`list`, `tree-view`, or an `object-*` widget — they call `useDataScope`). (objectui#6575)'; } + +/* ------------------------------------------------------------------------- * + * objectui#6665 — `data` was authored, and it is not an array. + * ------------------------------------------------------------------------- */ + +/** Prefix for every non-array `data` line — the handle tests and greps hold. */ +export const DATA_TABLE_DATA_DIAGNOSTIC_PREFIX = '[ObjectUI] DataTable data:'; + +/** + * Does this string carry a `${...}` template? + * + * Used ONLY to sharpen the wording. {@link describeNonArrayData} fires on ANY + * non-array `data`; this test decides which sentence it gets, never whether it + * speaks. That is why a local spelling is tolerable where an imported one would + * normally be required: a miss here costs a less specific message, never a + * missed warning, so the failure direction is the safe one. The shape is the + * one `ExpressionEvaluator` itself looks for before taking its template path + * (`@object-ui/core`), tightened with a closing brace so a lone `${` in prose + * does not get to claim it was an expression. + */ +const TEMPLATE_SHAPE = /\$\{[\s\S]*?\}/; + +/** Keep one bad value from turning a console line into a wall of text. */ +function truncate(value: string, max = 80): string { + return value.length <= max ? value : `${value.slice(0, max)}…`; +} + +/** Name the thing the author actually wrote, without dumping it. */ +function describeAuthoredValue(value: unknown): string { + if (value === null) return '`null`'; + if (Array.isArray(value)) return 'an array'; + if (typeof value === 'string') return `the string ${quote(truncate(value))}`; + if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') { + return `the ${typeof value} \`${String(value)}\``; + } + if (typeof value === 'object') return 'an object'; + return `a ${typeof value}`; +} + +/** + * Was `data` authored as something that is not an array? + * + * `undefined` is absence — an omitted key, or the renderer's destructuring + * default. Everything else the author WROTE, and `data-table.tsx` reduces every + * non-array value of it to `EMPTY_ROWS` without a word. + * + * ## Why this is not {@link hasAuthoredBind}'s question + * + * That one asks "was a `bind` authored?" — a key `data-table` does not read at + * all. This asks about `data`, the key it genuinely does read, and asks a + * SHAPE question about it rather than a presence one. The nodes that trip this + * carry no `bind`, so #6575's predicate is correctly silent on them; that + * silence is precisely why a second question had to be asked here instead of + * the first one being widened. + * + * ## Why the predicate is not keyed on the `${...}` shape + * + * The expression string is the spelling that was REPORTED, but it is not the + * defect. `Array.isArray(rawData) ? rawData : EMPTY_ROWS` swallows a number, an + * object, a `null` and a plain string exactly as silently, so a predicate that + * only caught `${...}` would leave the next non-array spelling to arrive as a + * fresh card. The shape only chooses the wording (see {@link TEMPLATE_SHAPE}). + * + * Exported so the renderer and this judgement cannot drift apart: one + * predicate, two readers. + */ +export function hasNonArrayAuthoredData(rawData: unknown): boolean { + return rawData !== undefined && !Array.isArray(rawData); +} + +/** + * The message for a `data-table` node whose `data` is not an array, or `null` + * when there is nothing to say. + * + * ## Why no measured-consequence clause, unlike {@link describeIgnoredBind} + * + * That one has to look at the rows the renderer resolved, because a node can + * carry BOTH a `bind` and a real inline `data` array — "header over an empty + * body" would be false there. Here it cannot be: `data` is the ONLY row source + * `data-table` has, and this function is called exactly when that source is not + * an array, so the body is `EMPTY_ROWS` by construction. The consequence is + * stated because the predicate already established it, not asserted on faith. + * + * ## What it deliberately does NOT say + * + * It does not tell the author to move the expression under `properties`. The + * same expression IS evaluated there — that contrast is what makes this a + * defect rather than a design — but whether `properties` is an authoring + * channel for the `ui:*` / `page:*` namespaces is an open contract question, + * recorded in `skills/objectui/rules/protocol.md` rather than recommended by + * it. A console line is the wrong place to settle that, so the message teaches + * the one route the guides do teach: the host resolves the rows. + * + * It also changes NO behaviour. Making node-level `data` evaluate expressions + * is a behaviour change on a published component; the objectui#6665 triage + * ruling put that arm on the maintainer floor and dispatched only this one. + * The trap stops being silent; it does not stop being a trap. + */ +export function describeNonArrayData( + rawData: unknown, + address: DataTableNodeAddress, +): string | null { + if (!hasNonArrayAuthoredData(rawData)) return null; + + const where = describeAddress(address); + const wayOut = + ' Resolve the rows in the host and put the array on the node as ' + + `\`${DECLARED_ROWS_KEY}\`. (objectui#6665)`; + + if (typeof rawData === 'string' && TEMPLATE_SHAPE.test(rawData)) { + return `${DATA_TABLE_DATA_DIAGNOSTIC_PREFIX} ${where} — ` + + `\`${DECLARED_ROWS_KEY}: ${quote(truncate(rawData))}\` was never evaluated: a ` + + `\`\${...}\` expression written into \`${DECLARED_ROWS_KEY}\` at node level is read as ` + + `a literal string, so \`${DECLARED_ROWS_KEY}\` is a string rather than an array and the ` + + `table renders its header over an empty body.\n${wayOut}`; + } + + return `${DATA_TABLE_DATA_DIAGNOSTIC_PREFIX} ${where} — ` + + `\`${DECLARED_ROWS_KEY}\` was authored as ${describeAuthoredValue(rawData)}, and data-table ` + + 'takes its rows only from an array: every non-array value is dropped to zero rows, so the ' + + `table renders its header over an empty body.\n${wayOut}`; +}