From 798482b3366faae10eb4556f31b64ab53188821f Mon Sep 17 00:00:00 2001 From: os-support-ai Date: Wed, 19 Aug 2026 16:54:23 +0000 Subject: [PATCH] =?UTF-8?q?fix(plugin-grid,react):=20one=20column=20spelli?= =?UTF-8?q?ng=20=E2=80=94=20retire=20the=20undeclared=20accessorKey/header?= =?UTF-8?q?=20round=20trip=20(#5068)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectGridSchema.columns` is declared `string[] | ListColumn[]`, and `ListColumnSchema` is a strict object that refuses `accessorKey` / `header` by name. `ObjectGrid` accepted them anyway through a branch that sniffed `columns[0]`, and `bridgeListView` produced them by down-translating columns that arrived canonical — a round trip through a spelling the contract rejects. Both halves land together, the shape objectui#3951 used (PR4909 migrated its consumer in `packages/fields` and its producer in `packages/plugin-form` in one PR): deleting the consumer half alone silently blanks every bridged grid, and the whole plugin-grid suite stays green while it happens. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- ...grid-column-declared-spelling-only-5068.md | 40 ++ ...bridge-list-view-canonical-columns-5068.md | 33 ++ packages/plugin-grid/src/ObjectGrid.tsx | 403 +++++++++--------- .../columnDeclaredSpellingOnly.test.tsx | 141 ++++++ .../specBridgeColumnSpelling.test.tsx | 132 ++++++ .../spec-bridge/__tests__/SpecBridge.test.ts | 34 +- .../src/spec-bridge/bridges/list-view.ts | 40 +- 7 files changed, 604 insertions(+), 219 deletions(-) create mode 100644 .changeset/grid-column-declared-spelling-only-5068.md create mode 100644 .changeset/spec-bridge-list-view-canonical-columns-5068.md create mode 100644 packages/plugin-grid/src/__tests__/columnDeclaredSpellingOnly.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/specBridgeColumnSpelling.test.tsx diff --git a/.changeset/grid-column-declared-spelling-only-5068.md b/.changeset/grid-column-declared-spelling-only-5068.md new file mode 100644 index 0000000000..13182dfcab --- /dev/null +++ b/.changeset/grid-column-declared-spelling-only-5068.md @@ -0,0 +1,40 @@ +--- +"@object-ui/plugin-grid": minor +--- + +fix(plugin-grid): `ObjectGrid` reads the declared column spelling, and only it + +`ObjectGridSchema.columns` is declared `string[] | ListColumn[]`, and +`ListColumnSchema` in `@objectstack/spec/ui` is a **strict** object: `field` is +required, and `accessorKey` / `header` are refused **by name** — +`unrecognized_keys`, with a prescriptive message. The renderer accepted that +refused spelling anyway, through a branch that sniffed `columns[0]` for an +`accessorKey` and synthesized a `ListColumn` from it. One key, two spellings: +one the schema admits, one only the runtime did. + +That branch retires (inheriting the disposition of objectui#3951 together with +its reason — unify at the producer, no consumer-side tolerance alias, AGENTS.md +#0.1). It is also why the fictional `{ header, accessorKey }` column interface +in the plugin README (objectui#5013) read as credible: it rendered, so nothing +signalled that the contract refuses it. + +**Affected input.** A column authored `{ accessorKey, header }` no longer +resolves; it is dropped, and a grid whose columns are all mis-spelled renders as +the row-number column alone. Write columns the declared way — `{ field, label }` +— which is what the spec has always accepted and what the docs have always said +(`content/docs/plugins/plugin-grid.mdx`: "The field this column reads. There is +no `accessorKey`."). No authored usage of the retired spelling exists in this +repo's examples, docs, apps or fixtures; every in-repo occurrence of the name +belongs to the `table` / `data-table` component, which legitimately owns it. + +The `columns[0]` sniff goes with the branch. Column identity is a per-column +property, and one filter now judges it: a mis-spelled column is dropped alone, +where the sniff let the first entry decide the fate of the whole array — a +declared column standing behind an undeclared one was lost with it, and the +reverse order threw a `TypeError` mid-render. + +`accessorKey` keeps its job on the way **out**: it is the data-table adapter's +column key, which `@object-ui/core` deliberately holds outside the metadata +identity fold (`TABLE_ADAPTER_COLUMN_KEY`) and which `ObjectGrid` still writes +when it hands columns to the adapter. Metadata vocabulary in, adapter vocabulary +out, one translation at one boundary. diff --git a/.changeset/spec-bridge-list-view-canonical-columns-5068.md b/.changeset/spec-bridge-list-view-canonical-columns-5068.md new file mode 100644 index 0000000000..4f50bf362d --- /dev/null +++ b/.changeset/spec-bridge-list-view-canonical-columns-5068.md @@ -0,0 +1,33 @@ +--- +"@object-ui/react": minor +--- + +fix(react): `bridgeListView` emits the column spelling the spec declares + +`mapColumn` took a spec-canonical `ListColumn` — whose columns are **already** +spelled `field` / `label` — and down-translated every one of them to +`{ accessorKey, header }` before emitting the `object-grid` node, which +`ObjectGrid` then translated back. A round trip through a spelling +`ListColumnSchema` refuses by name, on a value that arrived canonical. The +bridge now forwards the declared shape, and the tolerance branch on the other +side retires in the same release (see `@object-ui/plugin-grid`). + +**Output shape.** `bridgeListView` / `SpecBridge.transformListView` emit +`columns: [{ field, label?, … }]`. Code reading `node.columns[i].accessorKey` +off a bridged node reads `field` instead; `header` becomes `label`. The bare +string shorthand `columns: ['name']` now maps to `{ field: 'name' }`. + +**No label is invented any more.** `header: col.label ?? col.field` turned "the +author declared no label" into "the author declared the machine name", and that +synthesized value pre-empted `ObjectGrid`'s own header chain — the column's +label, then the **object field's** label, then the prettified machine name — +whose middle step exists so a localized field label wins on a non-English app. +A bridged view therefore rendered raw machine names where a directly authored +`object-grid` rendered the field's real label. A bare `{ field }` column now +reaches that chain intact. + +Speaking the declared spelling also routes bridged views through the renderer's +full ListColumn path rather than its type-inference-only one: object-schema +field enrichment, `hidden` filtering, primary-field auto-linking, and per-column +`link` / `action` handling now apply to a bridged `ListView` exactly as they do +to an authored grid. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 406cef2afb..14804aa4fb 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -1448,224 +1448,207 @@ export const ObjectGrid: React.FC = ({ const cols = normalizeColumns(schemaColumns); if (cols) { - // Check if columns are already in data-table format (have 'accessorKey') - // vs ListColumn format (have 'field') + // ObjectStack's DECLARED column spelling is the only one read + // (objectui#5068). `ObjectGridSchema.columns` is `string[] | ListColumn[]`, + // and `ListColumnSchema` in `@objectstack/spec/ui` is a STRICT object: + // `field` is required, `accessorKey` / `header` are refused BY NAME with + // `unrecognized_keys`. A branch here used to accept that refused spelling + // anyway — it sniffed `columns[0]` for an `accessorKey` and synthesized a + // `ListColumn` from it — so one key had two spellings, one the schema + // admits and one only the runtime did. That second de-facto contract is + // what AGENTS.md #0.1 forbids, and objectui#3951 already settled the + // shape of the fix for this defect family: unify at the PRODUCER, no + // consumer-side tolerance alias. The one in-repo producer that spoke it, + // `bridgeListView` in `@object-ui/react`, was migrated in the same PR. + // + // The `columns[0]` sniff goes with it. Column identity is a per-column + // property, and the filter below is the one place that judges it: a + // mis-spelled column is now dropped alone, where the sniff let the FIRST + // entry decide the fate of the whole array (a declared column standing + // behind an undeclared one was lost with it; the reverse order threw). + // + // `accessorKey` keeps its job on the way OUT — it is the data-table + // adapter's key, which `@object-ui/core` deliberately holds outside the + // metadata identity fold (`TABLE_ADAPTER_COLUMN_KEY`) and which this + // component still writes below. Metadata vocabulary in, adapter + // vocabulary out, one translation. if (cols.length > 0 && typeof cols[0] === 'object' && cols[0] !== null) { - const firstCol = cols[0] as any; - - // Already in data-table format - apply type inference for columns without custom cell renderers - if ('accessorKey' in firstCol) { - return (cols as any[]).map((col) => { - if (col.cell) return col; // already has custom renderer - - const syntheticCol: ListColumn = { field: col.accessorKey, label: col.header, type: col.type }; - const inferredType = inferColumnType(syntheticCol); - if (!inferredType) return col; - - const CellRenderer = getCellRenderer(inferredType); - const fieldMeta: Record = { name: col.accessorKey, type: inferredType }; - - if (inferredType === 'select') { - const uniqueValues = Array.from(new Set(data.map(row => row[col.accessorKey]).filter(Boolean))); - fieldMeta.options = uniqueValues.map((v: any) => ({ value: v, label: humanizeLabel(String(v)) })); + return (cols as ListColumn[]) + .filter((col) => col?.field && typeof col.field === 'string' && !col.hidden) + .map((col, colIndex) => { + // Fall back to the SCHEMA FIELD's label before prettifying the machine + // name — otherwise a column declared as bare { field } shows an English + // name-derived header (e.g. "Request title") even when the field has a + // localized label (e.g. "申请标题") on a non-English app. + const rawHeader = resolveColumnLabel(col.label) + || resolveColumnLabel(objectSchema?.fields?.[col.field]?.label) + || col.field.charAt(0).toUpperCase() + col.field.slice(1).replace(/_/g, ' '); + const header = schema.objectName ? resolveFieldLabel(schema.objectName, col.field, rawHeader) : rawHeader; + + // Build custom cell renderer based on column configuration + let cellRenderer: ((value: any, row: any) => React.ReactNode) | undefined; + + // Type-based cell renderer: explicit col type > objectDef type > heuristic inference. + // Format hints (e.g. `text` + `format: 'phone'`) promote to the + // richer renderer (PhoneCellRenderer) via resolveCellRendererType. + const objectDefField = objectSchema?.fields?.[col.field]; + const baseInferredType = col.type || objectDefField?.type || inferColumnType({ field: col.field }) || null; + const formatHint = (col as any).format ?? objectDefField?.format; + const inferredType = baseInferredType + ? resolveCellRendererType({ type: baseInferredType, format: formatHint }) + : null; + const CellRenderer = inferredType ? getCellRenderer(inferredType) : null; + + // Build field metadata for cell renderers with objectDef enrichment + const fieldMeta: Record = { name: col.field, type: inferredType || 'text' }; + // Merge objectDef field properties (options with colors, currency, precision, etc.) + if (objectDefField) { + if (objectDefField.label) fieldMeta.label = objectDefField.label; + if (objectDefField.currency) fieldMeta.currency = objectDefField.currency; + if (objectDefField.precision !== undefined) fieldMeta.precision = objectDefField.precision; + if ((objectDefField as any).scale !== undefined) (fieldMeta as any).scale = (objectDefField as any).scale; + if (objectDefField.format) fieldMeta.format = objectDefField.format; + if (objectDefField.options) fieldMeta.options = translateOptions(schema.objectName, col.field, objectDefField.options); } - // Pass through metadata-defined appearance only — never override - // the field's display style from the renderer. This keeps list - // cells visually consistent with detail / form rendering. - if ((col as any).appearance != null) { - fieldMeta.appearance = (col as any).appearance; + // Preserve relational metadata (reference_to, display_field, …) so + // lookup cells resolve ids to names and the inline picker can query. + applyRelationalMeta(fieldMeta, objectDefField as any); + // Auto-generate options from data for inferred select without existing options + if (inferredType === 'select' && !fieldMeta.options) { + const uniqueValues = Array.from(new Set(data.map(row => row[col.field]).filter(Boolean))); + fieldMeta.options = uniqueValues.map(v => ({ value: v, label: humanizeLabel(String(v)) })); + } + if ((col as any).options) { + fieldMeta.options = translateOptions(schema.objectName, col.field, (col as any).options); + } + // Honor metadata-defined appearance only (col.appearance or + // objectDef field.appearance). When unset, the cell renders + // its default badge style — same as detail / form views. + const explicitAppearance = (col as any).appearance ?? objectDefField?.appearance; + if (explicitAppearance != null) { + fieldMeta.appearance = explicitAppearance; + } + + // Auto-link primary field (first column) to record detail (Airtable-style) + const isPrimaryField = colIndex === 0 && !col.link && !col.action; + const isLinked = col.link || isPrimaryField; + + if ((col.link && col.action) || (isPrimaryField && col.action)) { + // Both link and action: link takes priority for navigation, action executes on secondary interaction + cellRenderer = (value: any, row: any) => { + const displayContent = CellRenderer + ? + : (value != null && value !== '' ? String(value) : —); + return ( + navigation.handleClick(row)} + objectName={schema.objectName} + recordId={rowRecordId(row)} + > + {displayContent} + + ); + }; + } else if (isLinked) { + // Link column: clicking navigates to the record detail + cellRenderer = (value: any, row: any) => { + const displayContent = CellRenderer + ? + : (value != null && value !== '' ? String(value) : —); + return ( + navigation.handleClick(row)} + objectName={schema.objectName} + recordId={rowRecordId(row)} + > + {displayContent} + + ); + }; + } else if (col.action) { + // Action column: render as action button + cellRenderer = (value: any, row: any) => { + return ( + + ); + }; + } else if (CellRenderer) { + // Type-only cell renderer (no link/action) + cellRenderer = (value: any) => ( + + ); + } else { + // Default renderer with empty value handling + cellRenderer = (value: any) => ( + value != null && value !== '' + ? {String(value)} + : + ); + } + + // Wrap with prefix compound cell renderer (Airtable-style: [Badge] Text in same cell) + const prefixConfig = (col as any).prefix; + if (prefixConfig?.field) { + const baseCellRenderer = cellRenderer; + const PrefixRenderer = prefixConfig.type === 'badge' ? getCellRenderer('select') : null; + cellRenderer = (value: any, row: any) => { + const prefixValue = row[prefixConfig.field]; + const prefixEl = prefixValue != null && prefixValue !== '' + ? PrefixRenderer + ? + : {String(prefixValue)} + : null; + return ( + + {prefixEl} + {baseCellRenderer(value, row)} + + ); + }; } + // Auto-infer alignment from field type if not explicitly set + const numericTypes = ['number', 'currency', 'percent']; + const effectiveType = inferredType || col.type; + const inferredAlign = col.align || (effectiveType && numericTypes.includes(effectiveType) ? 'right' as const : undefined); + + // Determine if column should be hidden on mobile + const isEssential = colIndex === 0 || (col as any).essential === true; + return { - ...col, - // Forward the resolved type so the inline editor (data-table) can - // pick a type-aware control (date picker, number, ...). - type: col.type ?? inferredType, + header, + accessorKey: col.field, + // Forward the resolved (base) field type so the inline editor can + // pick a type-aware control. Use baseInferredType (date/number/...) + // rather than the renderer type so e.g. `date` stays `date`. + ...(baseInferredType && { type: baseInferredType }), ...(schema.showColumnTypeIcons && { headerIcon: getTypeIcon(inferredType) }), - cell: (value: any) => , + ...(!isEssential && { className: 'hidden sm:table-cell' }), + ...(col.width && { width: col.width }), + ...(inferredAlign && { align: inferredAlign }), + sortable: col.sortable !== false, + ...(col.resizable !== undefined && { resizable: col.resizable }), + ...(col.wrap !== undefined && { wrap: col.wrap }), + ...(cellRenderer && { cell: cellRenderer }), + ...(col.pinned && { pinned: col.pinned }), }; }); - } - - // ListColumn format - convert to data-table format with full feature support - if ('field' in firstCol) { - return (cols as ListColumn[]) - .filter((col) => col?.field && typeof col.field === 'string' && !col.hidden) - .map((col, colIndex) => { - // Fall back to the SCHEMA FIELD's label before prettifying the machine - // name — otherwise a column declared as bare { field } shows an English - // name-derived header (e.g. "Request title") even when the field has a - // localized label (e.g. "申请标题") on a non-English app. - const rawHeader = resolveColumnLabel(col.label) - || resolveColumnLabel(objectSchema?.fields?.[col.field]?.label) - || col.field.charAt(0).toUpperCase() + col.field.slice(1).replace(/_/g, ' '); - const header = schema.objectName ? resolveFieldLabel(schema.objectName, col.field, rawHeader) : rawHeader; - - // Build custom cell renderer based on column configuration - let cellRenderer: ((value: any, row: any) => React.ReactNode) | undefined; - - // Type-based cell renderer: explicit col type > objectDef type > heuristic inference. - // Format hints (e.g. `text` + `format: 'phone'`) promote to the - // richer renderer (PhoneCellRenderer) via resolveCellRendererType. - const objectDefField = objectSchema?.fields?.[col.field]; - const baseInferredType = col.type || objectDefField?.type || inferColumnType({ field: col.field }) || null; - const formatHint = (col as any).format ?? objectDefField?.format; - const inferredType = baseInferredType - ? resolveCellRendererType({ type: baseInferredType, format: formatHint }) - : null; - const CellRenderer = inferredType ? getCellRenderer(inferredType) : null; - - // Build field metadata for cell renderers with objectDef enrichment - const fieldMeta: Record = { name: col.field, type: inferredType || 'text' }; - // Merge objectDef field properties (options with colors, currency, precision, etc.) - if (objectDefField) { - if (objectDefField.label) fieldMeta.label = objectDefField.label; - if (objectDefField.currency) fieldMeta.currency = objectDefField.currency; - if (objectDefField.precision !== undefined) fieldMeta.precision = objectDefField.precision; - if ((objectDefField as any).scale !== undefined) (fieldMeta as any).scale = (objectDefField as any).scale; - if (objectDefField.format) fieldMeta.format = objectDefField.format; - if (objectDefField.options) fieldMeta.options = translateOptions(schema.objectName, col.field, objectDefField.options); - } - // Preserve relational metadata (reference_to, display_field, …) so - // lookup cells resolve ids to names and the inline picker can query. - applyRelationalMeta(fieldMeta, objectDefField as any); - // Auto-generate options from data for inferred select without existing options - if (inferredType === 'select' && !fieldMeta.options) { - const uniqueValues = Array.from(new Set(data.map(row => row[col.field]).filter(Boolean))); - fieldMeta.options = uniqueValues.map(v => ({ value: v, label: humanizeLabel(String(v)) })); - } - if ((col as any).options) { - fieldMeta.options = translateOptions(schema.objectName, col.field, (col as any).options); - } - // Honor metadata-defined appearance only (col.appearance or - // objectDef field.appearance). When unset, the cell renders - // its default badge style — same as detail / form views. - const explicitAppearance = (col as any).appearance ?? objectDefField?.appearance; - if (explicitAppearance != null) { - fieldMeta.appearance = explicitAppearance; - } - - // Auto-link primary field (first column) to record detail (Airtable-style) - const isPrimaryField = colIndex === 0 && !col.link && !col.action; - const isLinked = col.link || isPrimaryField; - - if ((col.link && col.action) || (isPrimaryField && col.action)) { - // Both link and action: link takes priority for navigation, action executes on secondary interaction - cellRenderer = (value: any, row: any) => { - const displayContent = CellRenderer - ? - : (value != null && value !== '' ? String(value) : —); - return ( - navigation.handleClick(row)} - objectName={schema.objectName} - recordId={rowRecordId(row)} - > - {displayContent} - - ); - }; - } else if (isLinked) { - // Link column: clicking navigates to the record detail - cellRenderer = (value: any, row: any) => { - const displayContent = CellRenderer - ? - : (value != null && value !== '' ? String(value) : —); - return ( - navigation.handleClick(row)} - objectName={schema.objectName} - recordId={rowRecordId(row)} - > - {displayContent} - - ); - }; - } else if (col.action) { - // Action column: render as action button - cellRenderer = (value: any, row: any) => { - return ( - - ); - }; - } else if (CellRenderer) { - // Type-only cell renderer (no link/action) - cellRenderer = (value: any) => ( - - ); - } else { - // Default renderer with empty value handling - cellRenderer = (value: any) => ( - value != null && value !== '' - ? {String(value)} - : - ); - } - - // Wrap with prefix compound cell renderer (Airtable-style: [Badge] Text in same cell) - const prefixConfig = (col as any).prefix; - if (prefixConfig?.field) { - const baseCellRenderer = cellRenderer; - const PrefixRenderer = prefixConfig.type === 'badge' ? getCellRenderer('select') : null; - cellRenderer = (value: any, row: any) => { - const prefixValue = row[prefixConfig.field]; - const prefixEl = prefixValue != null && prefixValue !== '' - ? PrefixRenderer - ? - : {String(prefixValue)} - : null; - return ( - - {prefixEl} - {baseCellRenderer(value, row)} - - ); - }; - } - - // Auto-infer alignment from field type if not explicitly set - const numericTypes = ['number', 'currency', 'percent']; - const effectiveType = inferredType || col.type; - const inferredAlign = col.align || (effectiveType && numericTypes.includes(effectiveType) ? 'right' as const : undefined); - - // Determine if column should be hidden on mobile - const isEssential = colIndex === 0 || (col as any).essential === true; - - return { - header, - accessorKey: col.field, - // Forward the resolved (base) field type so the inline editor can - // pick a type-aware control. Use baseInferredType (date/number/...) - // rather than the renderer type so e.g. `date` stays `date`. - ...(baseInferredType && { type: baseInferredType }), - ...(schema.showColumnTypeIcons && { headerIcon: getTypeIcon(inferredType) }), - ...(!isEssential && { className: 'hidden sm:table-cell' }), - ...(col.width && { width: col.width }), - ...(inferredAlign && { align: inferredAlign }), - sortable: col.sortable !== false, - ...(col.resizable !== undefined && { resizable: col.resizable }), - ...(col.wrap !== undefined && { wrap: col.wrap }), - ...(cellRenderer && { cell: cellRenderer }), - ...(col.pinned && { pinned: col.pinned }), - }; - }); - } } // String array format - enrich with objectDef field metadata for type-aware rendering diff --git a/packages/plugin-grid/src/__tests__/columnDeclaredSpellingOnly.test.tsx b/packages/plugin-grid/src/__tests__/columnDeclaredSpellingOnly.test.tsx new file mode 100644 index 0000000000..ca8d7b9a2d --- /dev/null +++ b/packages/plugin-grid/src/__tests__/columnDeclaredSpellingOnly.test.tsx @@ -0,0 +1,141 @@ +/** + * 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. + */ + +/** + * `ObjectGrid` reads ONE column spelling — the declared one (objectui#5068). + * + * `ObjectGridSchema.columns` is declared `string[] | ListColumn[]`, and + * `ListColumnSchema` in `@objectstack/spec/ui` is a STRICT object: `field` is + * required and `accessorKey` / `header` are refused BY NAME + * (`unrecognized_keys`, with a prescriptive message). The renderer used to + * accept that refused spelling anyway, via a branch that sniffed `columns[0]` + * for an `accessorKey` and synthesized a `ListColumn` from it — so the same + * key had two spellings, one the schema admits and one only the runtime did. + * That is the second de-facto contract AGENTS.md #0.1 forbids, and it is the + * reason the fictional `{ header, accessorKey }` shape in the plugin README + * (objectui#5013) looked credible: it rendered. + * + * The disposition is inherited from objectui#3951 — unify at the producer, no + * consumer-side tolerance alias — so this file pins the consumer half: + * + * declared `{ field, label }` → renders (unchanged) + * undeclared `{ accessorKey, header }` → does not resolve + * + * `accessorKey` remains the vocabulary of the data-table ADAPTER on the way + * OUT (`@object-ui/components`' TanStack column key, `columnIdentity.ts`'s + * `TABLE_ADAPTER_COLUMN_KEY`). This card is about the way IN. + * + * LEGIBILITY (pinned below, deliberately, rather than left as folklore): an + * undeclared column does not throw, does not render an empty header, and + * produces no console line — it is simply DROPPED, so a grid whose columns are + * all mis-spelled renders as the row-number column alone. Whether that silence + * deserves a dev-time diagnostic is its own decision (objectui#5068 Q2) and is + * deliberately NOT implemented here. + */ +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider } from '@object-ui/react'; + +registerAllFields(); + +const ROWS = [ + { id: '1', name: 'Ada', amount: 100 }, + { id: '2', name: 'Grace', amount: 200 }, +]; + +/** Render a bare `object-grid` node over inline data — no host, no dataSource. */ +function renderGrid(columns: unknown[]) { + return render( + + + , + ); +} + +/** Every rendered header, in order. `#` is the built-in row-number column. */ +function headers(): string[] { + return screen.getAllByRole('columnheader').map((h) => (h.textContent ?? '').trim()); +} + +describe('ObjectGrid columns — the declared spelling is the only one read (#5068)', () => { + it('renders a column authored the declared way (field / label)', () => { + renderGrid([ + { field: 'name', label: 'Name' }, + { field: 'amount', label: 'Amount' }, + ]); + + expect(headers()).toEqual(['#', 'Name', 'Amount']); + expect(screen.getByText('Ada')).toBeInTheDocument(); + expect(screen.getByText('Grace')).toBeInTheDocument(); + }); + + it('does not resolve a column authored the undeclared way (accessorKey / header)', () => { + // The change. Before this card the tolerance branch synthesized + // `{ field: col.accessorKey, label: col.header }` and these rendered + // identically to the declared spelling above — which is precisely why + // nothing signalled that the spec refuses them. + renderGrid([ + { accessorKey: 'name', header: 'Name' }, + { accessorKey: 'amount', header: 'Amount' }, + ]); + + expect(headers()).toEqual(['#']); + expect(screen.queryByText('Ada')).not.toBeInTheDocument(); + expect(screen.queryByText('Grace')).not.toBeInTheDocument(); + }); + + it('drops the undeclared column and keeps the declared one, whichever comes first', () => { + // Per-column, not a first-element sniff. The retired branch dispatched on + // `columns[0]` alone, so on the old code the FIRST entry decided the fate + // of the whole array: a declared column standing behind an undeclared one + // was lost with it. Both orders are pinned so that cannot come back. + renderGrid([ + { accessorKey: 'amount', header: 'Amount' }, + { field: 'name', label: 'Name' }, + ]); + + expect(headers()).toEqual(['#', 'Name']); + expect(screen.getByText('Ada')).toBeInTheDocument(); + expect(screen.queryByText('100')).not.toBeInTheDocument(); + }); + + it('drops a trailing undeclared column without disturbing the declared one', () => { + renderGrid([ + { field: 'name', label: 'Name' }, + { accessorKey: 'amount', header: 'Amount' }, + ]); + + expect(headers()).toEqual(['#', 'Name']); + expect(screen.getByText('Grace')).toBeInTheDocument(); + }); + + it('leaves the grid standing — and silent — when every column is undeclared', () => { + // The legibility pin. Not an error, not an empty header cell, not a + // console line: the columns are simply gone. Recorded as behaviour so the + // follow-up diagnostics decision (Q2) has something to measure against. + const { container } = renderGrid([{ accessorKey: 'name', header: 'Name' }]); + + expect(headers()).toEqual(['#']); + expect(container.querySelector('table')).toBeInTheDocument(); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('still renders a plain string[] column list', () => { + // Untouched by this card — the other declared spelling — pinned because + // the retired branch sat between `normalizeColumns` and this arm. + renderGrid(['name', 'amount']); + + expect(headers()).toEqual(['#', 'Name', 'Amount']); + expect(screen.getByText('Ada')).toBeInTheDocument(); + }); +}); diff --git a/packages/plugin-grid/src/__tests__/specBridgeColumnSpelling.test.tsx b/packages/plugin-grid/src/__tests__/specBridgeColumnSpelling.test.tsx new file mode 100644 index 0000000000..b545149ddf --- /dev/null +++ b/packages/plugin-grid/src/__tests__/specBridgeColumnSpelling.test.tsx @@ -0,0 +1,132 @@ +/** + * 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. + */ + +/** + * SpecBridge to ObjectGrid — a bridged view keeps its columns (objectui#5068). + * + * This is the half that could not have caught itself. `bridgeListView` takes a + * spec-canonical `ListView` whose columns are ALREADY spelled `field`/`label`, + * and used to down-translate every one of them to `accessorKey`/`header` + * before emitting the `object-grid` node — which `ObjectGrid` then translated + * back through the tolerance branch this card retires. Producer and consumer + * were the two halves of one round trip, and deleting the consumer half alone + * blanked every bridged grid in TOTAL SILENCE: measured on this card, + * `headers ["#","Name"]` collapsed to `["#"]` with no error, while all 715 + * tests in this package stayed green — `specBridgeExportFormats.test.tsx` + * renders bridge output through `ObjectGrid` and watched its grid lose every + * column, because it asserts the export menu and never the columns. + * + * So the round trip is gone on BOTH sides in one PR (the shape objectui#3951 + * used: PR4909 migrated its consumer in `packages/fields` and its producer + * `deriveMasterDetail` in `packages/plugin-form` together), and this file pins + * the seam that had no pin. It lives in `plugin-grid` for the same reason the + * export-formats test does: `@object-ui/plugin-grid` depends on + * `@object-ui/react`, so it can see both `SpecBridge` and `ObjectGrid`; react + * cannot import the grid without inverting the dependency graph. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider, SpecBridge } from '@object-ui/react'; + +registerAllFields(); + +const ROWS = [ + { id: '1', name: 'Ada', email: 'ada@example.com' }, + { id: '2', name: 'Grace', email: 'grace@example.com' }, +]; + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: ROWS, total: ROWS.length, hasMore: false, pageSize: 50 })), + getObjectSchema: vi.fn(async (name: string) => ({ + name, + fields: { + id: { type: 'text' }, + name: { type: 'text', label: 'Full name' }, + email: { type: 'text', label: 'Email address' }, + }, + })), + } as any; +} + +/** Author a spec ListView, bridge it, render the node the bridge produced. */ +function bridgeAndRender(columns: unknown[]) { + const node = new SpecBridge().transformListView({ + name: 'contacts_all', + label: 'All Contacts', + columns, + }); + + render( + + + , + ); + + return node as any; +} + +function headers(): string[] { + return screen.getAllByRole('columnheader').map((h) => (h.textContent ?? '').trim()); +} + +describe('SpecBridge to ObjectGrid — bridged columns survive the seam (#5068)', () => { + it('renders every column of a bridged view', async () => { + bridgeAndRender([ + { field: 'name', label: 'Name' }, + { field: 'email', label: 'Email' }, + ]); + + expect(await screen.findByText('Ada')).toBeInTheDocument(); + expect(headers()).toEqual(['#', 'Name', 'Email']); + expect(screen.getByText('grace@example.com')).toBeInTheDocument(); + }); + + it('emits the declared spelling — the node carries no adapter key', () => { + // The producer pin. `accessorKey` is the data-table adapter's key, applied + // by `ObjectGrid` on the way OUT; a producer of `object-grid` metadata must + // not speak it on the way IN. Asserted on the node itself so a future + // regression fails HERE, loudly, instead of one layer down as blank cells. + const node = bridgeAndRender([{ field: 'name', label: 'Name' }]); + + expect(node.columns).toEqual([{ field: 'name', label: 'Name' }]); + expect(Object.keys(node.columns[0])).not.toContain('accessorKey'); + expect(Object.keys(node.columns[0])).not.toContain('header'); + }); + + it('lets the object schema label a bridged column the view left bare', async () => { + // A consequence of speaking the declared spelling, not an addition: a bare + // `{ field }` column reaches ObjectGrid's ListColumn arm, whose header + // chain is `col.label` → the OBJECT FIELD's label → the prettified machine + // name. The down-translation used to pre-empt that chain by writing + // `header: col.label ?? col.field`, so a bridged view always rendered the + // raw machine name where a directly authored object-grid rendered the + // field's real (and localizable) label. + bridgeAndRender([{ field: 'email' }]); + + expect(await screen.findByText('ada@example.com')).toBeInTheDocument(); + expect(headers()).toEqual(['#', 'Email address']); + }); + + it('carries a bare string column through as a declared field column', async () => { + // The spec's shorthand: `columns: ['name']`. Down-translated it became + // `{ accessorKey: 'name', header: 'name' }` — a synthesized label nobody + // authored. It is now the canonical `{ field: 'name' }`, and the header + // comes from the object schema like any other bare column. + const node = bridgeAndRender(['name']); + + expect(node.columns).toEqual([{ field: 'name' }]); + expect(await screen.findByText('Ada')).toBeInTheDocument(); + expect(headers()).toEqual(['#', 'Full name']); + }); +}); diff --git a/packages/react/src/spec-bridge/__tests__/SpecBridge.test.ts b/packages/react/src/spec-bridge/__tests__/SpecBridge.test.ts index 67cd477d7c..c3fa33354e 100644 --- a/packages/react/src/spec-bridge/__tests__/SpecBridge.test.ts +++ b/packages/react/src/spec-bridge/__tests__/SpecBridge.test.ts @@ -104,22 +104,46 @@ describe('SpecBridge', () => { expect(node.id).toBe('accounts_list'); expect(node.label).toBe('All Accounts'); expect(node.columns).toHaveLength(2); - expect(node.columns[0].accessorKey).toBe('name'); - expect(node.columns[0].header).toBe('Account Name'); + // The DECLARED spelling, and only it (objectui#5068). `accessorKey` / + // `header` is the data-table adapter's vocabulary, which `ObjectGrid` + // applies on the way OUT; a producer of `object-grid` metadata emits the + // spec's `ListColumn` — the shape it was handed in the first place. + expect(node.columns[0].field).toBe('name'); + expect(node.columns[0].label).toBe('Account Name'); + expect(node.columns[0].accessorKey).toBeUndefined(); + expect(node.columns[0].header).toBeUndefined(); expect(node.columns[0].width).toBe(200); expect(node.columns[0].sortable).toBe(true); - expect(node.columns[1].accessorKey).toBe('industry'); + expect(node.columns[1].field).toBe('industry'); expect(node.data).toEqual({ provider: 'object', object: 'Account' }); expect(node.selection).toEqual({ mode: 'multiple' }); expect(node.pagination).toEqual({ pageSize: 25 }); }); - it('uses field name as header fallback', () => { + it('leaves a bare column bare — no label is invented (#5068)', () => { + // This used to assert `header === 'email'`: the down-translation wrote + // `header: col.label ?? col.field`, so "the author declared no label" + // arrived downstream as "the author declared the machine name". That + // synthesized value pre-empted `ObjectGrid`'s own header chain + // (`col.label` → the object FIELD's label → the prettified name), which + // exists precisely for a column authored as a bare `{ field }` — see the + // localized-label comment at `ObjectGrid.tsx`'s ListColumn arm. The + // bridge now forwards what the view declared and nothing else; the + // rendered consequence is pinned in `@object-ui/plugin-grid`'s + // `specBridgeColumnSpelling.test.tsx`. const node = bridgeListView( { columns: [{ field: 'email' }] }, {}, ); - expect(node.columns[0].header).toBe('email'); + expect(node.columns[0]).toEqual({ field: 'email' }); + }); + + it('maps the spec shorthand string column to a declared field column (#5068)', () => { + const node = bridgeListView( + { columns: ['email'] as any }, + {}, + ); + expect(node.columns[0]).toEqual({ field: 'email' }); }); it('maps column properties correctly', () => { diff --git a/packages/react/src/spec-bridge/bridges/list-view.ts b/packages/react/src/spec-bridge/bridges/list-view.ts index 21533b9b36..bebab62115 100644 --- a/packages/react/src/spec-bridge/bridges/list-view.ts +++ b/packages/react/src/spec-bridge/bridges/list-view.ts @@ -20,17 +20,49 @@ import type { ListView, ListColumn, RowHeight } from '@objectstack/spec/ui'; */ type ListViewSpec = Partial; +/** + * A spec `ListColumn` in, the same column out — in the spelling the spec + * declares (objectui#5068). + * + * This used to down-translate every column to the data-table adapter's + * `{ accessorKey, header }` before handing it to an `object-grid` node, and + * `ObjectGrid` translated it straight back through a tolerance branch that + * sniffed `columns[0]`. A round trip whose input was ALREADY canonical: the + * bridge's own parameter is `@objectstack/spec/ui`'s `ListColumn`, where + * `field` is required and `accessorKey` / `header` are refused by name + * (`ListColumnSchema` is a strict object). So the producer emitted a spelling + * the contract rejects, and the renderer grew a second de-facto contract to + * read it back — exactly the shape AGENTS.md #0.1 forbids, and the disposition + * objectui#3951 already settled: unify at the producer, no consumer-side alias. + * + * `accessorKey` is not retired as a concept — it is the TanStack adapter key + * `@object-ui/core` deliberately keeps OUT of the metadata identity fold + * (`column-identity.ts`'s `TABLE_ADAPTER_COLUMN_KEY`). `ObjectGrid` still + * applies it on the way OUT, at the one boundary that owns it. Metadata + * vocabulary comes in; adapter vocabulary goes out; one translation, one place. + * + * Nothing is invented on the way through, which is the second half of the fix. + * `header: col.label ?? col.field` used to turn "no label declared" into "the + * machine name was declared as the label", and that synthesized value then + * pre-empted `ObjectGrid`'s own header chain — `col.label` → the OBJECT + * FIELD's label → the prettified name — whose whole purpose is a column + * authored as a bare `{ field }`, and whose middle step exists so a localized + * field label wins over the name-derived English one on a non-English app. A + * bridged view therefore rendered raw machine names where a directly authored + * `object-grid` rendered the field's real, localizable label. It now forwards + * the declaration and lets the renderer resolve what was left unsaid. + */ function mapColumn(col: ListColumn | string): Record { - // Spec-legacy shorthand: a bare field name stands for a default column. + // Spec shorthand: a bare field name stands for a default column. if (typeof col === 'string') { - return { accessorKey: col, header: col }; + return { field: col }; } const mapped: Record = { - accessorKey: col.field, - header: col.label ?? col.field, + field: col.field, }; + if (col.label != null) mapped.label = col.label; if (col.width != null) mapped.width = col.width; if (col.align) mapped.align = col.align; if (col.hidden != null) mapped.hidden = col.hidden;