Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .changeset/grid-column-declared-name-spelling.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 3 additions & 3 deletions apps/console/src/dev/DevLookup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ export const DevLookup: React.FC = () => {
const [rows, setRows] = React.useState<Record<string, any>[]>([{}, {}]);
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;
Expand Down
10 changes: 5 additions & 5 deletions apps/console/src/dev/DevMasterDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand All @@ -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' },
],
},
],
Expand Down
14 changes: 8 additions & 6 deletions docs/adr/0001-master-detail-subform.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" }
]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/fields/src/__tests__/date-locale-channel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions packages/fields/src/complex-widgets.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
101 changes: 101 additions & 0 deletions packages/fields/src/widgets/GridField.declaredSpelling.test.tsx
Original file line number Diff line number Diff line change
@@ -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(<GridField value={rows} onChange={() => {}} 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(<GridField value={rows} onChange={() => {}} 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(<GridField value={[]} onChange={onChange} field={field} />);
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(<GridField value={[{ product: 'Widget A' }]} onChange={onChange} field={field} />);
fireEvent.change((screen.getAllByLabelText('Qty') as HTMLInputElement[])[0], { target: { value: '7' } });
expect(onChange).toHaveBeenCalledWith([{ product: 'Widget A', quantity: 7 }]);
});
});
82 changes: 82 additions & 0 deletions packages/fields/src/widgets/GridField.keyWarning.test.tsx
Original file line number Diff line number Diff line change
@@ -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 <ul>{['a', 'b'].map((v) => <li>{v}</li>)}</ul>;
}

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(<KeylessControl />);
expect(capture.keyWarnings().length).toBeGreaterThan(0);

const before = capture.keyWarnings().length;
render(<GridField value={rows} onChange={() => {}} field={field} />);
capture.restore();

// No NEW key warning attributable to the grid's own render.
expect(capture.keyWarnings().length).toBe(before);
});
});
Loading
Loading