diff --git a/.changeset/grid-column-declared-name-spelling.md b/.changeset/grid-column-declared-name-spelling.md new file mode 100644 index 0000000000..092be93602 --- /dev/null +++ b/.changeset/grid-column-declared-name-spelling.md @@ -0,0 +1,27 @@ +--- +"@object-ui/fields": patch +"@object-ui/plugin-form": patch +--- + +fix(fields): grid columns are keyed by the declared `name`, so spec-compliant grid metadata renders populated cells + +`GridField` declared its own column interface keyed by `field` and read +`c.field` at every site (`key=`, `row[…]`, the blank row, cell writes, the +column chooser, the running-total lookup), while the published +`GridColumnDefinition` in `@object-ui/types` — and the grid documentation, and +the `fields-grid` catalog examples — declare the key as `name`. Metadata +authored against the published type therefore rendered a grid with the correct +row count and every cell empty, plus a React "unique key" warning per column. + +The renderer now reads the declared `name`, and the master-detail derivation +(`deriveColumns` / `hydrateColumns` / `pickAmountField`) produces and consumes +the same key. There is deliberately **no** `col.field ?? col.name` alias: one +spelling at the producer (AGENTS.md #0.1). + +**Breaking for `field`-keyed columns.** Grid / line-item / master-detail +subform columns spelled `{ field: 'amount' }` must be re-spelled +`{ name: 'amount' }`. This affects author-supplied `columns` on the `grid` +field, `record:line_items`, `object-master-detail-form` details and a +relationship field's `inlineColumns`. Auto-derived columns (no explicit +`columns` block) need no change. List-view and `object-grid` columns are a +different contract (`ListColumn`) and keep their own `field` key. diff --git a/apps/console/src/dev/DevLookup.tsx b/apps/console/src/dev/DevLookup.tsx index 0fb73010ad..9733758250 100644 --- a/apps/console/src/dev/DevLookup.tsx +++ b/apps/console/src/dev/DevLookup.tsx @@ -10,9 +10,9 @@ export const DevLookup: React.FC = () => { const [rows, setRows] = React.useState[]>([{}, {}]); const field = { columns: [ - { field: 'account', label: 'Account', type: 'lookup', reference: 'showcase_account', displayField: 'name' }, - { field: 'note', label: 'Note', type: 'text' }, - { field: 'amount', label: 'Amount', type: 'currency' }, + { name: 'account', label: 'Account', type: 'lookup', reference: 'showcase_account', displayField: 'name' }, + { name: 'note', label: 'Note', type: 'text' }, + { name: 'amount', label: 'Amount', type: 'currency' }, ], total_field: 'amount', } as any; diff --git a/apps/console/src/dev/DevMasterDetail.tsx b/apps/console/src/dev/DevMasterDetail.tsx index dccf4f57d0..d0b8c63625 100644 --- a/apps/console/src/dev/DevMasterDetail.tsx +++ b/apps/console/src/dev/DevMasterDetail.tsx @@ -23,9 +23,9 @@ const schema = { amountField: 'amount', totalField: 'total_amount', columns: [ - { field: 'expense_date', label: 'Date', type: 'date' }, + { name: 'expense_date', label: 'Date', type: 'date' }, { - field: 'category', + name: 'category', label: 'Category', type: 'select', options: [ @@ -39,9 +39,9 @@ const schema = { { label: 'Other', value: 'other' }, ], }, - { field: 'description', label: 'Description', type: 'text' }, - { field: 'merchant_vendor', label: 'Merchant', type: 'text' }, - { field: 'amount', label: 'Amount', type: 'currency' }, + { name: 'description', label: 'Description', type: 'text' }, + { name: 'merchant_vendor', label: 'Merchant', type: 'text' }, + { name: 'amount', label: 'Amount', type: 'currency' }, ], }, ], diff --git a/docs/adr/0001-master-detail-subform.md b/docs/adr/0001-master-detail-subform.md index 7d3ea406b8..3840ecf734 100644 --- a/docs/adr/0001-master-detail-subform.md +++ b/docs/adr/0001-master-detail-subform.md @@ -135,11 +135,13 @@ the dormant `GridFieldMetadata` type promised. - Read-only mode renders the same table without inputs (replaces the `"N rows"` stub) — this is the **view** half of the requirement. -Column config (reuses the existing `GridColumnDefinition` shape): +Column config (reuses the existing `GridColumnDefinition` shape — keyed by +`name`, as that type declares; the examples here said `field` until +objectui#3951 aligned the renderer to the declared spelling): ```ts { - field: 'amount', + name: 'amount', label: 'Amount', type: 'currency', // text | number | currency | select | date | lookup options?: [...], // for select @@ -187,10 +189,10 @@ slot) can drop in the children grid bound to the current record: "relationshipField": "expense_claim", "totalField": "total_amount", "columns": [ - { "field": "expense_date", "type": "date" }, - { "field": "category", "type": "lookup", "reference": "expense_category" }, - { "field": "description", "type": "text" }, - { "field": "amount", "type": "currency" } + { "name": "expense_date", "type": "date" }, + { "name": "category", "type": "lookup", "reference": "expense_category" }, + { "name": "description", "type": "text" }, + { "name": "amount", "type": "currency" } ] } } diff --git a/packages/app-shell/src/providers/MetadataProvider.merge.test.ts b/packages/app-shell/src/providers/MetadataProvider.merge.test.ts index fa4c604fd6..08827ffd40 100644 --- a/packages/app-shell/src/providers/MetadataProvider.merge.test.ts +++ b/packages/app-shell/src/providers/MetadataProvider.merge.test.ts @@ -192,7 +192,7 @@ describe('attachInlineSubforms — relationship-level inlineEdit', () => { it('lets an explicit form.subforms entry override the derived one', () => { const withExplicit = objects.map((o) => o.name === 'invoice' - ? { ...o, form: { type: 'simple', subforms: [{ childObject: 'invoice_line', columns: [{ field: 'amount' }] }] } } + ? { ...o, form: { type: 'simple', subforms: [{ childObject: 'invoice_line', columns: [{ name: 'amount' }] }] } } : o, ); const out = attachInlineSubforms(withExplicit); diff --git a/packages/fields/src/__tests__/date-locale-channel.test.tsx b/packages/fields/src/__tests__/date-locale-channel.test.tsx index 534759d781..7fa862542f 100644 --- a/packages/fields/src/__tests__/date-locale-channel.test.tsx +++ b/packages/fields/src/__tests__/date-locale-channel.test.tsx @@ -187,8 +187,8 @@ describe('zh session — every date branch renders Chinese (objectui#4468)', () it('the sub-grid read-only table', () => { const temporalField = { columns: [ - { field: 'merchant', label: 'Merchant', type: 'text' as const }, - { field: 'incurred_on', label: 'Incurred On', type: 'date' as const }, + { name: 'merchant', label: 'Merchant', type: 'text' as const }, + { name: 'incurred_on', label: 'Incurred On', type: 'date' as const }, ], } as any; renderSession( diff --git a/packages/fields/src/complex-widgets.test.tsx b/packages/fields/src/complex-widgets.test.tsx index 40135ecd5f..624601a42f 100644 --- a/packages/fields/src/complex-widgets.test.tsx +++ b/packages/fields/src/complex-widgets.test.tsx @@ -827,8 +827,8 @@ describe('Complex & Relationship Widgets', () => { describe('GridField', () => { const columns = [ - { field: 'name', label: 'Name' }, - { field: 'age', label: 'Age', type: 'number' } + { name: 'name', label: 'Name' }, + { name: 'age', label: 'Age', type: 'number' } ]; const data = [ { name: 'Alice', age: 30 }, diff --git a/packages/fields/src/widgets/GridField.declaredSpelling.test.tsx b/packages/fields/src/widgets/GridField.declaredSpelling.test.tsx new file mode 100644 index 0000000000..85ad794294 --- /dev/null +++ b/packages/fields/src/widgets/GridField.declaredSpelling.test.tsx @@ -0,0 +1,101 @@ +/** + * 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#3951 — grid columns have ONE key spelling, and it is the declared + * one: `GridColumnDefinition.name` (`@object-ui/types`), the same key the grid + * docs page and the three `fields-grid` catalog examples author. + * + * `GridField` used to declare its own local column interface keyed by `field` + * and read `c.field` everywhere — `key={c.field}`, `row[c.field]`, + * `blank[c.field]`, `applyCell(rowIdx, col.field, …)`. Metadata authored + * against the published type therefore rendered a grid with the right row + * COUNT and every cell EMPTY, plus a React "unique key" warning for every + * column (the key was `undefined`). The three demos on `/docs/fields/grid` + * shipped in exactly that state. + * + * The fixtures below are typed as `GridColumnDefinition[]` on purpose: the pin + * is anchored to the DECLARED contract, not to the widget's own idea of it, so + * the two can never silently drift apart again. Per AGENTS.md #0.1 the fix is + * one spelling at the producer — there is deliberately no `c.field ?? c.name` + * alias to make the retired spelling keep working. + * + * Reverse verification: restore the reader to `c.field` and every case here + * goes red — cell inputs read back `''` because `row[undefined]` is + * `undefined`. The other half of the bug, the React missing-key warning, is + * pinned in `GridField.keyWarning.test.tsx` rather than here: React emits that + * warning only ONCE per owner component, so a second render inside this file + * observes nothing and the assertion would pass for an empty reason. It needs + * to own the first render of the file, hence its own file. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import React from 'react'; +import type { GridColumnDefinition, GridFieldMetadata } from '@object-ui/types'; +import { GridField } from './GridField'; + +/** Authored exactly as `GridColumnDefinition` declares — keyed by `name`. */ +const columns: GridColumnDefinition[] = [ + { name: 'product', label: 'Product', type: 'text' }, + { name: 'quantity', label: 'Qty', type: 'number' }, + { name: 'price', label: 'Price', type: 'currency' }, +]; + +/** The `fields-grid/grid-with-data` catalog example's rows, verbatim. */ +const rows = [ + { product: 'Widget A', quantity: 2, price: 29.99 }, + { product: 'Widget B', quantity: 1, price: 49.99 }, +]; + +const field = { type: 'grid', name: 'order_items', columns } as GridFieldMetadata; + +describe('GridField reads the DECLARED column spelling (objectui#3951)', () => { + it('renders every cell populated from metadata authored as GridColumnDefinition', () => { + render( {}} field={field} />); + + // One cell input per column per row, each echoing the row's stored value — + // this is the assertion the `field`-keyed reader could not satisfy. + const products = screen.getAllByLabelText('Product') as HTMLInputElement[]; + const quantities = screen.getAllByLabelText('Qty') as HTMLInputElement[]; + const prices = screen.getAllByLabelText('Price') as HTMLInputElement[]; + + expect(products[0].value).toBe('Widget A'); + expect(quantities[0].value).toBe('2'); + expect(prices[0].value).toBe('29.99'); + expect(products[1].value).toBe('Widget B'); + expect(quantities[1].value).toBe('1'); + expect(prices[1].value).toBe('49.99'); + + // Not one empty cell across the authored rows (the ghost/new row is last + // and is legitimately blank, so only the authored rows are checked). + for (const cell of [...products.slice(0, 2), ...quantities.slice(0, 2), ...prices.slice(0, 2)]) { + expect(cell.value).not.toBe(''); + } + }); + + it('the read-only surface shows the stored values, not blanks', () => { + render( {}} field={field} readonly />); + expect(screen.getByText('Widget A')).toBeTruthy(); + expect(screen.getByText('Widget B')).toBeTruthy(); + }); + + it('a blank row is keyed by the declared column names', () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: /Add line/i })); + expect(onChange).toHaveBeenCalledWith([{ product: null, quantity: null, price: null }]); + }); + + it('an edited cell writes back under the declared column name', () => { + const onChange = vi.fn(); + render(); + fireEvent.change((screen.getAllByLabelText('Qty') as HTMLInputElement[])[0], { target: { value: '7' } }); + expect(onChange).toHaveBeenCalledWith([{ product: 'Widget A', quantity: 7 }]); + }); +}); diff --git a/packages/fields/src/widgets/GridField.keyWarning.test.tsx b/packages/fields/src/widgets/GridField.keyWarning.test.tsx new file mode 100644 index 0000000000..81e1acb01f --- /dev/null +++ b/packages/fields/src/widgets/GridField.keyWarning.test.tsx @@ -0,0 +1,82 @@ +/** + * 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#3951, diagnostics half — a grid authored in the declared spelling + * (`GridColumnDefinition.name`) must render without React's missing-key + * warning. While `GridField` read a divergent `field` key, every header, chip + * and cell was emitted with `key={undefined}`, so a spec-compliant grid logged + * the warning on top of rendering blank cells. + * + * WHY THIS LIVES IN ITS OWN FILE: React emits the missing-key warning only + * ONCE per owner component. A second `GridField` render in the same module + * instance observes nothing at all, so the same assertion sitting after + * another render would pass whether or not the bug is present — green for an + * empty reason. Vitest isolates per FILE (`isolate: true` on the `dom` + * project), so giving the check its own file guarantees it owns the first + * render and the observation is real. Keep it that way: do not add another + * GridField render above it. + * + * The positive control below renders a deliberately key-less list first and + * asserts the spy DOES see a warning, so a green result downstream proves the + * detector is live rather than proving the console spy is broken. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render } from '@testing-library/react'; +import React from 'react'; +import type { GridColumnDefinition, GridFieldMetadata } from '@object-ui/types'; +import { GridField } from './GridField'; + +const columns: GridColumnDefinition[] = [ + { name: 'product', label: 'Product', type: 'text' }, + { name: 'quantity', label: 'Qty', type: 'number' }, + { name: 'price', label: 'Price', type: 'currency' }, +]; + +const field = { type: 'grid', name: 'order_items', columns } as GridFieldMetadata; + +const rows = [ + { product: 'Widget A', quantity: 2, price: 29.99 }, + { product: 'Widget B', quantity: 1, price: 49.99 }, +]; + +/** Collects everything React writes to console for the duration of a render. */ +function captureConsole() { + const messages: string[] = []; + const record = (...args: unknown[]) => { messages.push(args.map(String).join(' ')); }; + const errorSpy = vi.spyOn(console, 'error').mockImplementation(record); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(record); + return { + keyWarnings: () => messages.filter((m) => /unique "?key"?/i.test(m)), + restore: () => { errorSpy.mockRestore(); warnSpy.mockRestore(); }, + }; +} + +/** Deliberately key-less — the positive control for the detector above. */ +function KeylessControl(): React.ReactElement { + return
    {['a', 'b'].map((v) =>
  • {v}
  • )}
; +} + +describe('GridField emits no missing-key warning for declared-spelling columns (objectui#3951)', () => { + it('renders spec-compliant grid metadata with no React "unique key" warning', () => { + const capture = captureConsole(); + + // Control first: prove a missing key IS observable through this spy in + // this environment, so the assertion that follows means something. + render(); + expect(capture.keyWarnings().length).toBeGreaterThan(0); + + const before = capture.keyWarnings().length; + render( {}} field={field} />); + capture.restore(); + + // No NEW key warning attributable to the grid's own render. + expect(capture.keyWarnings().length).toBe(before); + }); +}); diff --git a/packages/fields/src/widgets/GridField.test.tsx b/packages/fields/src/widgets/GridField.test.tsx index 0bf9a6da76..f9f77f2ab4 100644 --- a/packages/fields/src/widgets/GridField.test.tsx +++ b/packages/fields/src/widgets/GridField.test.tsx @@ -5,8 +5,8 @@ import { UploadProvider } from '@object-ui/providers'; import { GridField, LineItemsField, sumColumn, lookupAutofillPatch } from './GridField'; const columns = [ - { field: 'description', label: 'Description', type: 'text' as const }, - { field: 'amount', label: 'Amount', type: 'currency' as const }, + { name: 'description', label: 'Description', type: 'text' as const }, + { name: 'amount', label: 'Amount', type: 'currency' as const }, ]; const field = { columns, total_field: 'amount' } as any; @@ -33,9 +33,9 @@ describe('GridField / LineItemsField — editable line items', () => { describe('column chooser (defaultHidden columns)', () => { const withHidden = { columns: [ - { field: 'title', label: 'Title', type: 'text' as const, required: true }, - { field: 'amount', label: 'Amount', type: 'currency' as const }, - { field: 'notes', label: 'Notes', type: 'text' as const, defaultHidden: true }, + { name: 'title', label: 'Title', type: 'text' as const, required: true }, + { name: 'amount', label: 'Amount', type: 'currency' as const }, + { name: 'notes', label: 'Notes', type: 'text' as const, defaultHidden: true }, ], } as any; @@ -63,8 +63,8 @@ describe('GridField / LineItemsField — editable line items', () => { describe('list mode (displayMode="list" — form-factor for fat children)', () => { const listField = { columns: [ - { field: 'title', label: 'Title', type: 'text' as const, required: true }, - { field: 'status', label: 'Status', type: 'select' as const, options: [{ label: 'To Do', value: 'todo' }] }, + { name: 'title', label: 'Title', type: 'text' as const, required: true }, + { name: 'status', label: 'Status', type: 'select' as const, options: [{ label: 'To Do', value: 'todo' }] }, ], } as any; @@ -126,8 +126,8 @@ describe('GridField / LineItemsField — editable line items', () => { describe('date columns echo the stored value (#3566)', () => { const dateField = { columns: [ - { field: 'description', label: 'Description', type: 'text' as const }, - { field: 'incurred_on', label: 'Incurred On', type: 'date' as const }, + { name: 'description', label: 'Description', type: 'text' as const }, + { name: 'incurred_on', label: 'Incurred On', type: 'date' as const }, ], } as any; @@ -195,10 +195,10 @@ describe('GridField / LineItemsField — editable line items', () => { const temporalField = { columns: [ - { field: 'merchant', label: 'Merchant', type: 'text' as const }, - { field: 'incurred_on', label: 'Incurred On', type: 'date' as const }, - { field: 'incurred_at', label: 'Incurred At', type: 'datetime' as const }, - { field: 'started_at', label: 'Started At', type: 'time' as const }, + { name: 'merchant', label: 'Merchant', type: 'text' as const }, + { name: 'incurred_on', label: 'Incurred On', type: 'date' as const }, + { name: 'incurred_at', label: 'Incurred At', type: 'datetime' as const }, + { name: 'started_at', label: 'Started At', type: 'time' as const }, ], } as any; @@ -363,10 +363,10 @@ describe('GridField / LineItemsField — editable line items', () => { describe('computed columns (amount = qty × unit_price)', () => { const computedField = { columns: [ - { field: 'product', label: 'Product', type: 'text' as const }, - { field: 'quantity', label: 'Qty', type: 'number' as const }, - { field: 'unit_price', label: 'Unit Price', type: 'currency' as const }, - { field: 'amount', label: 'Amount', type: 'currency' as const, computed: true, expr: 'record.quantity * record.unit_price', scale: 2 }, + { name: 'product', label: 'Product', type: 'text' as const }, + { name: 'quantity', label: 'Qty', type: 'number' as const }, + { name: 'unit_price', label: 'Unit Price', type: 'currency' as const }, + { name: 'amount', label: 'Amount', type: 'currency' as const, computed: true, expr: 'record.quantity * record.unit_price', scale: 2 }, ], total_field: 'amount', } as any; @@ -442,11 +442,11 @@ describe('GridField / LineItemsField — editable line items', () => { describe('lookupAutofillPatch (item typeahead auto-fill)', () => { const cols = [ - { field: 'product', type: 'lookup' as const, reference: 'product' }, - { field: 'description', type: 'text' as const }, - { field: 'quantity', type: 'number' as const }, - { field: 'unit_price', type: 'currency' as const }, - { field: 'amount', type: 'currency' as const, computed: true, expr: 'record.quantity * record.unit_price' }, + { name: 'product', type: 'lookup' as const, reference: 'product' }, + { name: 'description', type: 'text' as const }, + { name: 'quantity', type: 'number' as const }, + { name: 'unit_price', type: 'currency' as const }, + { name: 'amount', type: 'currency' as const, computed: true, expr: 'record.quantity * record.unit_price' }, ]; const product = { value: 'p1', label: 'Widget A', name: 'Widget A', description: 'Standard widget', unit_price: 29.99, sku: 'WIDGET-A' }; @@ -476,7 +476,7 @@ describe('GridField / LineItemsField — editable line items', () => { }); it('flags a required, empty cell on a real row (not the ghost row)', () => { - const reqField = { columns: [{ field: 'description', label: 'Description', type: 'text' as const, required: true }] } as any; + const reqField = { columns: [{ name: 'description', label: 'Description', type: 'text' as const, required: true }] } as any; render( {}} field={reqField} />); // The data row's required-empty cell is flagged... expect(screen.getByTestId('line-items-invalid-0-description')).toBeTruthy(); @@ -488,8 +488,8 @@ describe('GridField / LineItemsField — editable line items', () => { describe('file columns (upload in a grid cell — #2360)', () => { const fileField = { columns: [ - { field: 'description', label: 'Description', type: 'text' as const }, - { field: 'receipt', label: 'Receipt', type: 'file' as const }, + { name: 'description', label: 'Description', type: 'text' as const }, + { name: 'receipt', label: 'Receipt', type: 'file' as const }, ], } as any; @@ -546,7 +546,7 @@ describe('GridField / LineItemsField — editable line items', () => { it('passes the column accept list to the native picker', () => { const withAccept = { - columns: [{ field: 'receipt', label: 'Receipt', type: 'file' as const, accept: ['image/*', '.pdf'] }], + columns: [{ name: 'receipt', label: 'Receipt', type: 'file' as const, accept: ['image/*', '.pdf'] }], } as any; render( {}} field={withAccept} />); const input = document.querySelector('input[type="file"]') as HTMLInputElement; @@ -574,9 +574,9 @@ describe('GridField / LineItemsField — editable line items', () => { describe('parent-scoped conditional rules (B2 follow-up — "paid invoice → lock lines")', () => { const lockField = { columns: [ - { field: 'product', label: 'Product', type: 'text' as const }, - { field: 'qty', label: 'Qty', type: 'number' as const, readonlyWhen: "parent.status == 'paid'" }, - { field: 'unit_price', label: 'Unit Price', type: 'currency' as const, readonlyWhen: "parent.status == 'paid'" }, + { name: 'product', label: 'Product', type: 'text' as const }, + { name: 'qty', label: 'Qty', type: 'number' as const, readonlyWhen: "parent.status == 'paid'" }, + { name: 'unit_price', label: 'Unit Price', type: 'currency' as const, readonlyWhen: "parent.status == 'paid'" }, ], } as any; @@ -611,9 +611,9 @@ describe('GridField / LineItemsField — editable line items', () => { it('re-evaluates per row, mixing the parent header with row data', () => { const rowRule = { columns: [ - { field: 'qty', label: 'Qty', type: 'number' as const }, + { name: 'qty', label: 'Qty', type: 'number' as const }, // Locks only when the header is paid AND this row is already invoiced. - { field: 'note', label: 'Note', type: 'text' as const, readonlyWhen: "parent.status == 'paid' && record.invoiced == true" }, + { name: 'note', label: 'Note', type: 'text' as const, readonlyWhen: "parent.status == 'paid' && record.invoiced == true" }, ], } as any; render( diff --git a/packages/fields/src/widgets/GridField.tsx b/packages/fields/src/widgets/GridField.tsx index fb35391ec8..888140c460 100644 --- a/packages/fields/src/widgets/GridField.tsx +++ b/packages/fields/src/widgets/GridField.tsx @@ -32,7 +32,7 @@ import { toDateInputValue, toDateTimeInputValue, fromDateTimeInputValue } from ' * engine behind the master-detail subform (see ADR-0001). * * Column config (a subset of `GridColumnDefinition`): - * { field, label?, type?, options?, width?, required?, prefix?, step? } + * { name, label?, type?, options?, width?, required?, prefix?, step? } * type ∈ 'text' | 'number' | 'currency' | 'date' | 'datetime' | 'time' * | 'select' | 'lookup' | 'file' * @@ -41,7 +41,22 @@ import { toDateInputValue, toDateTimeInputValue, fromDateTimeInputValue } from ' */ export interface GridColumn { - field: string; + /** + * The column's field name — the key it reads and writes on each row object. + * + * Spelled `name`, exactly as the declared `GridColumnDefinition` + * (`@object-ui/types`) and the grid docs page say (objectui#3951). This + * widget used to read a divergent `field` key, so metadata authored against + * the published type rendered every cell empty plus a React key warning. + * There is deliberately no tolerant alias bridging the retired spelling to + * this one: a single spelling, enforced at the producer — AGENTS.md #0.1. + * + * (Wording note: do not restate that rule as an alternation expression over + * the two key names. `column-identity.ratchet.test.ts` (objectui#3104) scans + * these files line by line and cannot tell prose from code, so spelling the + * shape out here registers as a new dual read and fails the gate.) + */ + name: string; label?: string; /** * Cell control + read/write adapter for the column. @@ -246,12 +261,12 @@ export function evalArith(expr: string, row: Row): number | null { * it is unit-testable independent of the picker UI. */ export function lookupAutofillPatch(columns: GridColumn[], col: GridColumn, record: any): Row { - const patch: Row = { [col.field]: record?.value ?? record?.id ?? record?._id }; + const patch: Row = { [col.name]: record?.value ?? record?.id ?? record?._id }; if (col.autofill !== false && record && typeof record === 'object') { for (const other of columns) { - if (other.field === col.field || other.computed || other.type === 'lookup') continue; - const v = record[other.field]; - if (v !== undefined && v !== null && v !== '') patch[other.field] = v; + if (other.name === col.name || other.computed || other.type === 'lookup') continue; + const v = record[other.name]; + if (v !== undefined && v !== null && v !== '') patch[other.name] = v; } } return patch; @@ -263,9 +278,9 @@ export function computeRow(columns: GridColumn[], row: Row): Row { const next = { ...row }; for (const c of computedCols) { const v = evalArith(c.expr!, next); - if (v === null) { next[c.field] = null; continue; } + if (v === null) { next[c.name] = null; continue; } const scale = c.scale ?? (c.type === 'currency' ? 2 : undefined); - next[c.field] = scale != null ? Number(v.toFixed(scale)) : v; + next[c.name] = scale != null ? Number(v.toFixed(scale)) : v; } return next; } @@ -427,7 +442,7 @@ export function GridField({ const [extraShown, setExtraShown] = React.useState>(() => new Set()); const optionalColumns = allColumns.filter((c) => c.defaultHidden && !c.required); const columns: GridColumn[] = allColumns.filter( - (c) => !c.defaultHidden || c.required || extraShown.has(c.field), + (c) => !c.defaultHidden || c.required || extraShown.has(c.name), ); const toggleColumn = useCallback((fieldName: string) => { setExtraShown((prev) => { @@ -465,7 +480,7 @@ export function GridField({ const blankRow = useCallback((): Row => { const blank: Row = {}; - for (const c of columns) blank[c.field] = null; + for (const c of columns) blank[c.name] = null; return blank; }, [columns]); @@ -489,7 +504,7 @@ export function GridField({ ); const applyCell = useCallback( - (rowIdx: number, field: string, value: any) => applyPatch(rowIdx, { [field]: value }), + (rowIdx: number, columnName: string, value: any) => applyPatch(rowIdx, { [columnName]: value }), [applyPatch], ); @@ -509,13 +524,13 @@ export function GridField({ /** Set a cell to an already-typed value (lookup ids, etc.) without coercion. */ const setCellValue = useCallback( - (rowIdx: number, field: string, value: any) => applyCell(rowIdx, field, value), + (rowIdx: number, columnName: string, value: any) => applyCell(rowIdx, columnName, value), [applyCell], ); const setCell = useCallback( (rowIdx: number, col: GridColumn, raw: string) => { - applyCell(rowIdx, col.field, coerce(col.type, raw)); + applyCell(rowIdx, col.name, coerce(col.type, raw)); }, [applyCell], ); @@ -594,7 +609,7 @@ export function GridField({ const total = showTotal ? sumColumn(rows, totalField!) : 0; // Align the running total under the column it sums (not blindly under the // last column). The label sits right-aligned immediately to its left. - const totalColIndex = showTotal ? Math.max(0, columns.findIndex((c) => c.field === totalField)) : -1; + const totalColIndex = showTotal ? Math.max(0, columns.findIndex((c) => c.name === totalField)) : -1; // Column chooser — reveal/hide the optional (default-hidden) columns. Only // rendered when there are optional columns to manage. @@ -621,19 +636,19 @@ export function GridField({
Optional columns
{optionalColumns.map((c) => { - const id = `col-toggle-${c.field}`; + const id = `col-toggle-${c.name}`; return ( ); })} @@ -656,14 +671,14 @@ export function GridField({ )} {columns.map((c) => ( - {c.label || c.field} + {c.label || c.name} ))} @@ -686,12 +701,12 @@ export function GridField({ )} {columns.map((c) => ( - {c.type === 'lookup' && row[c.field] != null && row[c.field] !== '' ? ( + {c.type === 'lookup' && row[c.name] != null && row[c.name] !== '' ? ( {}} readonly field={{ reference: c.reference, display_field: c.displayField, id_field: c.idField } as any} @@ -702,9 +717,9 @@ export function GridField({ // for a date, and for a datetime it would ALSO have been // wrong to render as a bare day (objectui#3569). Now that // the three types are distinct, each formats as itself. - displayText(c, row[c.field], displayLocale) - ) : row[c.field] != null && row[c.field] !== '' ? ( - String(row[c.field]) + displayText(c, row[c.name], displayLocale) + ) : row[c.name] != null && row[c.name] !== '' ? ( + String(row[c.name]) ) : ( '—' )} @@ -749,7 +764,7 @@ export function GridField({ /** Cell content: read-only display (list mode / computed columns) or an * editable borderless control (spreadsheet feel). */ const renderCellInput = (c: GridColumn, colIdx: number, rowIdx: number, row: Row) => { - const val = row?.[c.field]; + const val = row?.[c.name]; // A readonlyWhen-TRUE cell is locked: treat like the form-wide `disabled`. const locked = disabled || cellRules(c, row).readonly; // List (form-factor) mode → read-only at-a-glance display. @@ -772,7 +787,7 @@ export function GridField({ {displayText(c, val, displayLocale)} @@ -782,7 +797,7 @@ export function GridField({ return ( setCellValue(rowIdx, c.field, v)} + onChange={(v: any) => setCellValue(rowIdx, c.name, v)} onSelectRecord={(rec: any) => applyLookupSelection(rowIdx, c, rec)} compact field={{ reference: c.reference, display_field: c.displayField, id_field: c.idField, multiple: c.multiple, options: c.options, placeholder: '—' } as any} @@ -796,11 +811,11 @@ export function GridField({ return ( setCellValue(rowIdx, c.field, v)} + onChange={(v: any) => setCellValue(rowIdx, c.name, v)} multiple={c.multiple} accept={Array.isArray(c.accept) && c.accept.length > 0 ? c.accept.join(',') : undefined} disabled={locked} - aria-label={c.label || c.field} + aria-label={c.label || c.name} data-cell={`${rowIdx}-${colIdx}`} /> ); @@ -808,7 +823,7 @@ export function GridField({ if (c.type === 'select') { return ( `, so a lookup, date, number or * picklist field silently renders as free text. This resolves each such * column's `type` (plus `options` / `reference` / computed `expr`) from the @@ -271,11 +271,11 @@ export function hydrateColumns( if (!cols.length || !fields || typeof fields !== 'object') return cols; return cols.map((col) => { if (col.type) return col; // explicit type — respect the author's choice - const d = (fields as any)[col.field]; + const d = (fields as any)[col.name]; if (!d) return col; // unknown field — leave as-is (grid falls back to text) const type = fieldTypeToColumnType(d?.type); const next: GridColumn = { ...col, type }; - if (next.label == null) next.label = d?.label || col.field; + if (next.label == null) next.label = d?.label || col.name; if (next.required == null) next.required = !!d?.required; const options = optionsFor(d); if (type === 'select' && options && !next.options) next.options = options; @@ -395,12 +395,12 @@ function pickAmountField(columns: GridColumn[]): string | undefined { const numeric = columns.filter((c) => c.type === 'number' || c.type === 'currency'); if (numeric.length === 0) return undefined; const computed = numeric.find((c) => c.computed); - if (computed) return computed.field; - const named = numeric.find((c) => AMOUNT_LIKE_FIELDS.includes(c.field)); - if (named) return named.field; + if (computed) return computed.name; + const named = numeric.find((c) => AMOUNT_LIKE_FIELDS.includes(c.name)); + if (named) return named.name; const lastCurrency = [...numeric].reverse().find((c) => c.type === 'currency'); - if (lastCurrency) return lastCurrency.field; - return numeric[numeric.length - 1].field; + if (lastCurrency) return lastCurrency.name; + return numeric[numeric.length - 1].name; } export interface DerivedDetail { diff --git a/packages/plugin-form/src/index.tsx b/packages/plugin-form/src/index.tsx index 31e5ab3964..49538218ed 100644 --- a/packages/plugin-form/src/index.tsx +++ b/packages/plugin-form/src/index.tsx @@ -287,9 +287,9 @@ import { LineItemsPanel } from './LineItemsPanel'; * must name a field ON the bound child object. Rebinding `object` without * updating it is an authoring error the panel cannot paper over. * - `columns` is NOT a field-name projection here. It is `GridColumn[]` - * (`{ field, type, options, computed, expr, … }`) driving an EDITABLE grid; a + * (`{ name, type, options, computed, expr, … }`) driving an EDITABLE grid; a * saved view's column list would arrive as bare names and render a grid of - * column definitions with no `field`. Wrong shape, not merely a wider answer. + * column definitions with no `name`. Wrong shape, not merely a wider answer. * * So a `view` named on this block now contributes its filter, sort and page size * to the child query (and an unresolvable name still reports instead of silently diff --git a/packages/plugin-form/src/subformHosts.test.tsx b/packages/plugin-form/src/subformHosts.test.tsx index 7bab0a1523..015bd53a98 100644 --- a/packages/plugin-form/src/subformHosts.test.tsx +++ b/packages/plugin-form/src/subformHosts.test.tsx @@ -22,7 +22,7 @@ const ds: any = { batchTransaction: vi.fn(), }; -const subforms = [{ childObject: 'expense_line', relationshipField: 'claim', title: 'Lines', columns: [{ field: 'amount', type: 'number' }] }]; +const subforms = [{ childObject: 'expense_line', relationshipField: 'claim', title: 'Lines', columns: [{ name: 'amount', type: 'number' }] }]; beforeEach(() => vi.clearAllMocks());