From 289749402d03f43536bd50276ee54ee5c6040e20 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 01:02:46 +0000 Subject: [PATCH 1/2] fix(plugin-grid): type ObjectGrid's column emit against the TableColumn[] slot it fills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `generateColumns()` had no return annotation and every call site cast to `any`, so nothing checked what this producer wrote into `DataTableSchema.columns: TableColumn[]`. Annotating it would have changed nothing, and that measurement is the substance of this change. On this program `generateColumns(): TableColumn[]` raises ZERO diagnostics — the emit literals reach the annotation through `.map()`, which strips the freshness excess-property checking depends on, so even an undeclared key written out longhand is accepted. objectui#6373 found spreads exempt at its seam; here freshness is gone entirely. Underneath that sat the reason no annotation could bite: `objectSchema` is `useState`, and an `any` spread into an object literal collapses the WHOLE literal to `any`. Naming the four inference locals stops it at that boundary. The emit type then carries ADR-0049 `?: never` tombstones DERIVED from `keyof ListColumn` — never hand-listed — so a future spec key is refused by default and has to be adjudicated to escape. Tombstones bite by assignability, which never depended on freshness. Casts dropped at all four call sites (the card's census named three) plus a fifth inside the producer, `const generatedColumns: any[]`, which had left one emit path unchecked even after the return was annotated. Verdicts: `headerIcon` and `pinned` are HELD — both have measured live readers (`data-table`'s header cell; this file's own pin-reorder pass) — and are now declared at the seam instead of arriving anonymously. `wrap` is HELD too and deliberately NOT retired: nothing reads it, but objectui#5453 owns that key and is blocked on objectui#5415. `options` is RETIRED: no reader on either side of the seam, and every value it carried still reaches its consumer through the field metadata the cell closure captures and the object schema the inline editor reads. `type` stays objectui#5853's; `name` is not emitted here. No rendering change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .changeset/6004-grid-column-emit-boundary.md | 31 +++ packages/plugin-grid/src/ObjectGrid.tsx | 231 +++++++++++++++--- .../__tests__/columnEmitBoundary-6004.test.ts | 186 ++++++++++++++ 3 files changed, 412 insertions(+), 36 deletions(-) create mode 100644 .changeset/6004-grid-column-emit-boundary.md create mode 100644 packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts diff --git a/.changeset/6004-grid-column-emit-boundary.md b/.changeset/6004-grid-column-emit-boundary.md new file mode 100644 index 000000000..c714459cc --- /dev/null +++ b/.changeset/6004-grid-column-emit-boundary.md @@ -0,0 +1,31 @@ +--- +'@object-ui/plugin-grid': minor +--- + +fix(plugin-grid): type ObjectGrid's column emit against the `TableColumn[]` slot it fills + +`generateColumns()` had no return annotation and all four of its call sites cast +to `any`, so nothing checked what this producer wrote into +`DataTableSchema.columns: TableColumn[]`. + +Annotating it is not enough, and that is the substance of the change. Measured on +this program: `generateColumns(): TableColumn[]` raises **zero** diagnostics — the +emit literals reach the annotation through `.map()`, which strips the freshness +that excess-property checking depends on, so even an undeclared key written out +longhand is accepted. Underneath that sits the reason the annotation could not +bite at all: `objectSchema` is `useState`, and an `any` spread into an object +literal collapses the **entire** literal to `any`. + +So the fix has three parts: name the four inference locals so `any` stops at the +boundary, carry ADR-0049 `?: never` tombstones **derived** from +`keyof ListColumn` (never hand-listed, so a future spec key is refused by +default), and drop the `any` at every call site — including a fourth the card's +census missed and a fifth (`const generatedColumns: any[]`) inside the producer. + +Key verdicts: `headerIcon`, `pinned` and `wrap` are HELD and now declared at the +seam; `options` is RETIRED — nothing on either side of the seam read it, and +every value it carried still reaches its consumer through the field metadata the +cell closure captures and the object schema the inline editor reads. `type` stays +objectui#5853's and `name` is not emitted here at all. + +No rendering change. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 3b562478a..f7c063048 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -22,7 +22,7 @@ */ import React, { useEffect, useState, useCallback, useMemo } from 'react'; -import type { ObjectGridSchema, DataSource, ListColumn, ViewData, TableSortItem, DataTableSchema, ListViewExportFormat } from '@object-ui/types'; +import type { ObjectGridSchema, DataSource, ListColumn, TableColumn, ViewData, TableSortItem, DataTableSchema, ListViewExportFormat } from '@object-ui/types'; import { isSystemManagedField, normalizeTableColumnType } from '@object-ui/types'; import type { I18nLabel } from '@objectstack/spec/ui'; import { SchemaRenderer, useDataScope, useNavigationOverlay, useAction, useSafeFieldLabel, usePredicateScope, useRelatedRecordActions } from '@object-ui/react'; @@ -481,6 +481,136 @@ function normalizeColumns( return columns as string[]; } +/** + * ⭐ WHAT THIS GRID'S COLUMN PRODUCER IS ALLOWED TO EMIT (objectui#6004). + * + * `generateColumns()` below builds the column array this component hands to + * `data-table`, whose slot is `DataTableSchema.columns: TableColumn[]`. It had + * no return annotation, so its four emit paths were inferred structurally and + * nothing ever compared them to the slot's declaration — and all four call + * sites re-widened to `any` on the way out, so even an annotation would have + * been discarded one hop later. + * + * ## ⭐ Why this is not just `: TableColumn[]` + * + * objectui#6373 measured, at the sibling producer in `plugin-dashboard`, that a + * bare `TableColumn` annotation raises no error on a `{ ...spread }` emit, + * because TypeScript's excess-property check is a FRESHNESS check on the + * properties an object literal WRITES OUT and spread properties are exempt. + * + * Measured again HERE before choosing this shape, because this seam is worse + * than that one and the difference matters: + * + * 1. `generateColumns = useCallback((): TableColumn[] => {`, everything else + * unchanged → `tsc --noEmit` exit 0, ZERO diagnostics. + * 2. Same, PLUS an undeclared key `wrapControl: true` WRITTEN OUT in the path-A + * column literal (not arriving through a spread) → exit 0, ZERO diagnostics. + * + * Run 2 is the one that fixes the mechanism. At objectui#6373's seam the object + * literal sits in `enrich()`'s return position, so freshness still caught keys + * written out longhand and only the spread escaped. Here every literal is the + * return value of a `.map()` callback, so it is inferred into the callback's + * return type FIRST and reaches the annotation as a non-fresh `X[]`. Freshness + * is gone entirely: written-out keys escape too. A bare annotation at this seam + * is not a weak instrument, it is an inert one — a green gate and no guard. + * + * The `?: never` members are what make the annotation able to fail. They are + * ADR-0049 retirement tombstones — this repo's convention for a key that is + * REFUSED rather than merely absent (`StaticTableColumn` in `@object-ui/types`) + * — and they bite by ASSIGNABILITY, which survives `.map()` because it never + * depended on freshness. Re-adding `...(col.summary && { summary: col.summary })` + * to a column literal is a compile error naming `summary`. + * + * ## The rule (objectui#6373's, applied to this producer's key set) + * + * A producer may write into a `TableColumn[]` slot only keys the CONSUMER of + * that slot reads, and the read set is MEASURED from the consumer's source, + * never assumed. A key the consumer reads and `TableColumn` declares is + * written; a key the consumer reads that `TableColumn` does not declare is HELD + * where a ruling already holds it; a key nothing reads is RETIRED — never + * declared, because declaring a key nothing reads is the same + * `declared != enforced` defect facing the other way. Before retiring, prove + * the value has a second road to its consumer. + * + * Consumers measured for THIS producer — two of them, because the array is read + * twice before it reaches the slot: + * + * - `data-table.tsx`, comments stripped, every `col.` read: `accessorKey`, + * `width`, `align`, `header`, `className`, `cellClassName`, `sortable`, + * `resizable`, `editable`, `type`, `cell`, `headerIcon`, `fitContent`, `name`. + * - THIS FILE's own downstream passes, which read the array before handing it + * on: `pinned` (the left/right reorder + the frozen-column verdict), + * `accessorKey`, `header`, `width`, `type`, `fitContent`. + * + * Verdicts, each with the read-count behind it: + * + * - `headerIcon` — HELD. Live: `data-table.tsx` renders it into the header + * cell (2 reads). Whether `TableColumn` should DECLARE it is objectui#6424's + * call, not this card's; declared here at the seam meanwhile, so the hold is + * visible instead of anonymous. + * - `pinned` — HELD. Live, and consumed BEFORE the slot: the reorder pass + * below reads it (5 reads in this file) and re-expresses it as the sticky + * `className` that `data-table` actually reads. `data-table` never reads + * `pinned` itself, and does not need to. + * - `wrap` — HELD, and deliberately NOT retired here. Nothing anywhere reads + * it, so this card's rule would retire it — but objectui#5453 already owns + * that key and is `pm:blocked` on objectui#5415, whose outcome decides + * implement-vs-remove. Retiring it here would settle a blocked card from + * the outside. It is declared, inert, and stays exactly as it was. + * - `options` — RETIRED (see the enrichment pass below). + * - `type` — not adjudicated here; objectui#5853 owns its VALUE set and its + * fold still stands. It is the one member whose vocabulary differs between + * the two types below. + * - `name` — not emitted by this producer at all, so objectui#5120's alias + * needs no hold here. Tombstoned only in the sense that nothing writes it. + * + * `essential` is absent from both types on purpose: objectui#6004's suggested + * key list named it, but it is READ off the authored column and turned into a + * `className` — it is never emitted, and it is not a `ListColumn` member either. + */ + +/** + * The tombstoned keys — DERIVED from the authored input type, never hand-listed, + * so a future `ListColumn` member is refused by default and has to be + * adjudicated to escape. + * + * `ListColumn` is the right derivation source because it is where this + * producer's drift comes from: every key the emit could wrongly grow is a key + * the author wrote on the input and someone forwarded. `wrap` and `pinned` are + * the two that already escaped, both adjudicated HELD above. + */ +export type RetiredListColumnKey = Exclude; + +/** The undeclared-but-live keys this producer holds. See the docblock above. */ +export interface GridColumnHolds { + /** HELD, objectui#6424 — `data-table` renders it; `TableColumn` does not declare it. */ + headerIcon?: React.ReactNode; + /** HELD — consumed by this file's own reorder pass before the array reaches the slot. */ + pinned?: 'left' | 'right'; + /** HELD, objectui#5453 (blocked on objectui#5415) — inert, and not this card's to retire. */ + wrap?: boolean; +} + +/** + * What `generateColumns()` returns: everything final EXCEPT `type`, which is + * still the producer's raw inference vocabulary (`@objectstack/spec`'s + * `FieldType`, 49 values) rather than the 7-literal union `TableColumn` + * declares. objectui#5853 folds it downstream, in a pass that is deliberately + * separate from the enrichment map — so the pre-fold shape needs a name, and + * this is it. + */ +export type GridColumnDraft = + Omit + & { type?: string } + & GridColumnHolds + & { [K in RetiredListColumnKey]?: never }; + +/** Post-fold: what actually reaches `DataTableSchema.columns: TableColumn[]`. */ +export type GridColumn = + TableColumn + & GridColumnHolds + & { [K in RetiredListColumnKey]?: never }; + /** The row heights this grid styles — the five `RowHeight` values the spec admits. */ type RowHeightMode = 'compact' | 'short' | 'medium' | 'tall' | 'extra_tall'; @@ -1522,7 +1652,7 @@ export const ObjectGrid: React.FC = ({ if (message) console.warn(message); }, [schema.columns, columnDiagnosticBlockType, schema.objectName, columnDiagnosticLabel]); - const generateColumns = useCallback(() => { + const generateColumns = useCallback((): GridColumnDraft[] => { // Map field type to column header icon (Airtable-style) const getTypeIcon = (fieldType: string | null): React.ReactNode => { if (!fieldType) return ; @@ -1674,9 +1804,16 @@ export const ObjectGrid: React.FC = ({ // 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; + // ⭐ ANNOTATED, and the annotation is load-bearing (objectui#6004). + // `objectSchema` is `useState`, so `objectDefField?.type` is + // `any` — and an `any` SPREAD into an object literal collapses the + // WHOLE literal to `any`, which silently un-checks every other key + // in it. Measured: without this annotation the emit below infers + // `any[]`, and `GridColumnDraft` cannot bite on any member. Naming + // the producer vocabulary here stops `any` at this one boundary. + const baseInferredType: string | null = col.type || objectDefField?.type || inferColumnType({ field: col.field }) || null; const formatHint = (col as any).format ?? objectDefField?.format; - const inferredType = baseInferredType + const inferredType: string | null = baseInferredType ? resolveCellRendererType({ type: baseInferredType, format: formatHint }) : null; const CellRenderer = inferredType ? getCellRenderer(inferredType) : null; @@ -1843,7 +1980,11 @@ export const ObjectGrid: React.FC = ({ const header = schema.objectName ? resolveFieldLabel(schema.objectName, fieldName, rawHeader) : rawHeader; // Resolve type: objectDef type > heuristic inference (consistent with ListColumn path) - const resolvedType = fieldDef?.type || inferColumnType({ field: fieldName }) || null; + // Annotated for the same reason as path A's `baseInferredType` + // above: `fieldDef` is `any`, and an `any` reaching the `...(resolvedType + // && { type: resolvedType })` spread below collapses the emit literal + // to `any` (objectui#6004). + const resolvedType: string | null = fieldDef?.type || inferColumnType({ field: fieldName }) || null; const CellRenderer = resolvedType ? getCellRenderer(resolvedType) : null; // Build field metadata with objectDef enrichment @@ -1924,7 +2065,8 @@ export const ObjectGrid: React.FC = ({ const fieldsToShow = schemaFields || Object.keys(inlineData[0]); return fieldsToShow.map((fieldName) => { const fieldDef = objectSchema?.fields?.[fieldName]; - const resolvedType = fieldDef?.type || inferColumnType({ field: fieldName }) || null; + // Annotated for the same reason as paths A and B (objectui#6004). + const resolvedType: string | null = fieldDef?.type || inferColumnType({ field: fieldName }) || null; const CellRenderer = resolvedType ? getCellRenderer(resolvedType) : null; const header = fieldDef?.label || fieldName.charAt(0).toUpperCase() + fieldName.slice(1).replace(/_/g, ' '); @@ -1969,7 +2111,7 @@ export const ObjectGrid: React.FC = ({ if (!objectSchema) return []; - const generatedColumns: any[] = []; + const generatedColumns: GridColumnDraft[] = []; // Default columns priority (when schema doesn't specify columns): // 1. The object's `highlightFields` semantic role (ADR-0085). // 2. Otherwise, all schema fields with system-managed fields pushed to the end. @@ -2020,6 +2162,9 @@ export const ObjectGrid: React.FC = ({ if (perms?.isLoaded && schema.objectName && !perms.checkField(schema.objectName, fieldName, 'read')) return; + // Annotated for the same reason as paths A-C (objectui#6004): `field` is + // `any`, so this value has to be named before it reaches a spread below. + const fieldType: string | undefined = field.type; const CellRenderer = getCellRenderer(field.type); const numericTypes = ['number', 'currency', 'percent']; const translatedField = field.options @@ -2030,8 +2175,8 @@ export const ObjectGrid: React.FC = ({ header: schema.objectName ? resolveFieldLabel(schema.objectName, fieldName, field.label || fieldName) : field.label || fieldName, accessorKey: fieldName, // Forward the field type for the type-aware inline editor. - ...(field.type && { type: field.type }), - ...(numericTypes.includes(field.type) && { align: 'right' }), + ...(fieldType && { type: fieldType }), + ...(numericTypes.includes(field.type) && { align: 'right' as const }), cell: (value: any) => , sortable: field.sortable !== false, }); @@ -2105,8 +2250,8 @@ export const ObjectGrid: React.FC = ({ && exportConfig?.streaming !== false; if (serverEligible) { - const cols = generateColumns().filter((c: any) => c.accessorKey !== '_actions'); - const fields = cols.map((c: any) => c.accessorKey).filter(Boolean); + const cols = generateColumns().filter((c) => c.accessorKey !== '_actions'); + const fields = cols.map((c) => c.accessorKey).filter(Boolean); // Same lowered value the fetch above sends, which is what keeps the // downloaded file agreeing with the screen: both read `schemaFilter` @@ -2173,9 +2318,9 @@ export const ObjectGrid: React.FC = ({ }; if (format === 'csv') { - const cols = generateColumns().filter((c: any) => c.accessorKey !== '_actions'); - const fields = cols.map((c: any) => c.accessorKey); - const headers = cols.map((c: any) => c.header); + const cols = generateColumns().filter((c) => c.accessorKey !== '_actions'); + const fields = cols.map((c) => c.accessorKey); + const headers = cols.map((c) => c.header); const rows: string[] = []; if (includeHeaders) { rows.push(headers.join(',')); @@ -2224,19 +2369,31 @@ export const ObjectGrid: React.FC = ({ ); } - const columns = generateColumns().map((col: any) => { - // Enrich each column with its field type + select options so the - // data-table's type-aware inline editor can pick the matching control - // (dropdown for select, checkbox for boolean) the form uses, instead of a - // plain text box. Additive: never overrides a type/options a path already set. - if (!col || col.accessorKey === '_actions') return col; + const columns: GridColumn[] = generateColumns().map((col): GridColumnDraft => { + // Enrich each column with its field type so the data-table's type-aware + // inline editor can pick the matching control (dropdown for select, + // checkbox for boolean) the form uses, instead of a plain text box. + // Additive: never overrides a type a path already set. + // + // ⭐ THE `options` KEY RETIRED HERE (objectui#6004). This pass also used to + // write `next.options = translateOptions(…)`, and nothing read it. Measured + // read sets, comments stripped: `data-table.tsx` reads no column-level + // `options` at all — its select/boolean editors are not hand-rolled there, + // they come from the host through `renderCellEditor`, and THIS component's + // `renderCellEditor` (below) rebuilds the field from + // `objectSchema.fields[ctx.column.accessorKey]` rather than from the column. + // So the write had no reader on either side of the seam. + // + // Retiring it is behaviour-preserving because the value still has its own + // road to every consumer that wants it — that check is part of the rule, + // not an aside: cell renderers read translated options off the `fieldMeta` + // built inside `generateColumns()`, and the inline editor reads them off + // the object schema. Neither ever consulted `col.options`. + if (col.accessorKey === '_actions') return col; const fieldDef = (objectSchema as any)?.fields?.[col.accessorKey]; if (!fieldDef) return col; - const next: any = { ...col }; + const next: GridColumnDraft = { ...col }; if (next.type == null && fieldDef.type) next.type = fieldDef.type; - if (next.options == null && fieldDef.options) { - next.options = translateOptions(schema.objectName, col.accessorKey, fieldDef.options); - } // Read-only / computed / binary fields are not value-editable in place — // mark the column so the data-table never opens an editor (otherwise it // falls back to a plain text box for e.g. a Formula or File cell). Only @@ -2264,15 +2421,17 @@ export const ObjectGrid: React.FC = ({ // // An out-of-union type drops the `type` KEY — never the column. See // `normalizeTableColumnType` for why absence beats folding onto `'text'`. - .map((col: any) => { - if (!col || col.type == null) return col; - const normalized = normalizeTableColumnType(col.type); - if (normalized === col.type) return col; - if (normalized === undefined) { - const { type: _undeclared, ...rest } = col; - return rest; - } - return { ...col, type: normalized }; + // + // This pass is also where the draft becomes the real thing: `type` is the + // one member whose vocabulary differs between `GridColumnDraft` (producer + // spelling) and `GridColumn` (what the slot declares), so the fold and the + // type transition are the same step (objectui#6004). + .map((col): GridColumn => { + const { type: producerType, ...rest } = col; + if (producerType == null) return rest; + const normalized = normalizeTableColumnType(producerType); + if (normalized === undefined) return rest; + return { ...rest, type: normalized }; }); // Apply persisted column order and widths @@ -3395,11 +3554,11 @@ export const ObjectGrid: React.FC = ({ // Tailwind md: / the responsive page+grid layout), render stacked cards // instead of a side-scrolling wide table. if (useCardView && data.length > 0 && !isGrouped) { - const displayColumns = generateColumns().filter((c: any) => c.accessorKey !== '_actions'); + const displayColumns = generateColumns().filter((c) => c.accessorKey !== '_actions'); // Build a lookup of column metadata for smart rendering - const colMap = new Map(); - displayColumns.forEach((col: any) => colMap.set(col.accessorKey, col)); + const colMap = new Map(); + displayColumns.forEach((col) => colMap.set(col.accessorKey, col)); // Identify special columns by inferred type for visual hierarchy const titleCol = displayColumns[0]; // First column is always the title diff --git a/packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts b/packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts new file mode 100644 index 000000000..a5e913018 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts @@ -0,0 +1,186 @@ +/** + * 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#6004 — what `ObjectGrid.generateColumns()` is allowed to EMIT. + * + * ⚠️ THE DEFECT IS TYPE-LEVEL, SO A RENDERING TEST IS BLIND TO IT. A grid that + * renders correctly renders exactly as correctly with the emit type deleted; + * every assertion in a render test stays green. What can actually fail is a + * COMPILE, so the pins below are compile-time and this file's value is that + * `tsc -p tsconfig.test.json` reads it. (It does — verified with `--listFiles`, + * not assumed: `tsconfig.json` EXCLUDES `**\/__tests__\/**`, so the package + * build's program never sees this file and only the test project checks it.) + * + * Each `@ts-expect-error` below is written to be refused for exactly ONE + * reason, because a directive refused for two reasons pins neither: it stays + * "used" when one of them is deleted, and a pin that survives the deletion of + * the thing it guards is a ghost. Where freshness (excess-property checking) + * would be a second reason, the fixture is routed through a non-fresh value + * first — which is also how the real emit reaches the type. + */ +import { describe, it, expect } from 'vitest'; +import type { ListColumn, TableColumn } from '@object-ui/types'; +import type { GridColumn, GridColumnDraft } from '../ObjectGrid'; + +/** True only for `any`. `any` is the one type both branches of a conditional accept. */ +type IsAny = 0 extends 1 & T ? true : false; + +/** Compile-time equality, exact in both directions. */ +type Expect = T; + +describe('objectui#6004 — the emit boundary is an instrument, not a decoration', () => { + /** + * ⭐ THE ROOT CAUSE, PINNED. + * + * `objectSchema` is `useState`, so the field type read off it is `any`. + * Spreading an `any` into an object literal collapses the ENTIRE literal to + * `any` — every other key in it silently stops being checked. That is why + * annotating `generateColumns()` changed nothing until the four inference + * locals were annotated: the emit literals were `any`, so no return type + * could bite on them. + * + * This is the non-obvious half of the card and nothing else in the repo + * states it, so it is pinned as an executable claim. If TypeScript ever + * stops collapsing here, this goes red and the annotations in + * `generateColumns()` can be revisited — a useful red, not noise. + */ + it('an `any` in a conditional spread collapses the whole object literal to `any`', () => { + // `any` is the SUBJECT of this test, not a shortcut in it: `unknown` does + // not reproduce the collapse, which is the whole behaviour being pinned. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const anyValue = JSON.parse('null') as any; + const collapsed = { header: 'h', accessorKey: 'a', ...(anyValue && { type: anyValue }) }; + type _CollapsesToAny = Expect>; + + // …and naming the value stops it, which is exactly what the fix does. + const named: string | null = anyValue; + const notCollapsed = { header: 'h', accessorKey: 'a', ...(named && { type: named }) }; + type _StaysReal = Expect extends true ? false : true>; + + expect(collapsed.accessorKey).toBe('a'); + expect(notCollapsed.accessorKey).toBe('a'); + }); + + /** + * ⭐ WHY THE TOMBSTONES EXIST — the measurement that says a bare + * `TableColumn` annotation could not have done this job. + * + * This assignment MUST COMPILE. `summary` is a `ListColumn` key that + * `TableColumn` does not declare, and here it reaches a `TableColumn[]` slot + * through the same `.map()` the real producer uses. Excess-property checking + * is a FRESHNESS check on a literal being assigned directly; a literal that + * has been through `.map()` is no longer fresh, so `TableColumn[]` accepts it + * silently. If this ever fails to compile, TypeScript has tightened and the + * docblock in `ObjectGrid.tsx` needs re-measuring. + */ + it('a bare `TableColumn[]` annotation accepts an undeclared key (the blind instrument)', () => { + const authored = [{ field: 'amount', summary: 'sum' }] as ListColumn[]; + const bare: TableColumn[] = authored.map((c) => ({ + header: 'Amount', + accessorKey: c.field, + ...(c.summary && { summary: c.summary }), + })); + expect(bare).toHaveLength(1); + }); + + /** + * The same emit, against the type the producer actually declares: REFUSED. + * Freshness cannot be the reason (the value went through `.map()` first), so + * the tombstone is the only thing that can refuse it — delete + * `{ [K in RetiredListColumnKey]?: never }` and this directive goes unused. + */ + it('the emit type refuses a tombstoned key arriving through a spread', () => { + const authored = [{ field: 'amount', summary: 'sum' }] as ListColumn[]; + const emitted = authored.map((c) => ({ + header: 'Amount', + accessorKey: c.field, + ...(c.summary && { summary: c.summary }), + })); + // @ts-expect-error objectui#6004 — `summary` is tombstoned on the emit. + const refused: GridColumnDraft[] = emitted; + expect(refused).toHaveLength(1); + }); + + /** + * Same refusal for a key WRITTEN OUT rather than spread, still routed through + * a non-fresh value so freshness cannot be the reason either. + */ + it('the emit type refuses a tombstoned key written out', () => { + const emitted = { header: 'Name', accessorKey: 'name', label: 'Name' }; + // @ts-expect-error objectui#6004 — `label` is tombstoned on the emit. + const refused: GridColumnDraft = emitted; + expect(refused.accessorKey).toBe('name'); + }); + + /** + * ⭐ DERIVED, NEVER HAND-LISTED. The tombstone set is + * `Exclude`, so a + * key ADDED to the spec's `ListColumn` tomorrow is refused by default and has + * to be adjudicated to escape. These two pin both halves of that rule: a + * `ListColumn` key that `TableColumn` does not declare is `never` on the + * emit, and a key both declare is untouched. + */ + it('every undeclared ListColumn key is tombstoned, and declared ones are not', () => { + type _FieldTombstoned = Expect; + type _LinkTombstoned = Expect; + type _ActionTombstoned = Expect; + type _PrefixTombstoned = Expect; + type _HiddenTombstoned = Expect; + // Shared with `TableColumn`, so NOT tombstoned — the Exclude has to keep them alive. + type _SortableLives = Expect; + type _WidthLives = Expect; + expect(true).toBe(true); + }); + + /** + * The three HELD keys. Each is undeclared by `TableColumn` and each has a + * measured live reader (or, for `wrap`, an open card that owns it), so the + * emit type must ACCEPT them — a tombstone set that swallowed these would be + * a behaviour change wearing a type change's clothes. + */ + it('accepts the three held keys — headerIcon, pinned, wrap', () => { + const held = { header: 'H', accessorKey: 'a', headerIcon: null, pinned: 'left' as const, wrap: true }; + const accepted: GridColumnDraft = held; + expect(accepted.pinned).toBe('left'); + }); + + /** + * ⭐ THE RETIRED KEY (`options`). Nothing on either side of the seam reads a + * column-level `options`: `data-table` has no such read, and this component's + * own `renderCellEditor` rebuilds the field from the object schema. Retiring + * it means the emit type must now REFUSE it — otherwise "retired" is just a + * deleted line that the next edit can put back for free. + * + * `options` is not a `ListColumn` key, so it is not covered by the derived + * tombstone above; it is refused because it is not a member of any of the + * emit type's parts. Freshness is deliberately left in play here — that is + * genuinely the only reason a non-member is refused, so this directive still + * has exactly one cause. + */ + it('the emit type refuses the retired `options` key', () => { + // @ts-expect-error objectui#6004 — `options` retired from this producer's emit. + const refused: GridColumnDraft = { header: 'S', accessorKey: 'stage', options: [] }; + expect(refused.accessorKey).toBe('stage'); + }); + + /** + * The pre-fold / post-fold split (objectui#5853's fold is what separates + * them). `GridColumnDraft.type` is the producer's raw inference vocabulary; + * `GridColumn.type` is the narrow union `TableColumn` declares. If someone + * collapses the two types into one, one of these goes red. + */ + it('the draft carries the producer vocabulary and the folded column carries the declared union', () => { + const draft: GridColumnDraft = { header: 'A', accessorKey: 'a', type: 'lookup' }; + expect(draft.type).toBe('lookup'); + + // @ts-expect-error objectui#5853 — `lookup` is not a declared `TableColumn.type`. + const folded: GridColumn = { header: 'A', accessorKey: 'a', type: 'lookup' }; + expect(folded.accessorKey).toBe('a'); + }); +}); From 356d0d93a2b831a20e431678093ee5c7a79edad6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 01:47:13 +0000 Subject: [PATCH 2/2] fix(plugin-grid): restore the emit-path null guards, and rename the emit type off a collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review items from the PM on objectui#6004. 1. NULL GUARDS RESTORED. The first pass narrowed `if (!col || …)` away in the enrichment map and the fold, on the grounds that the emit is now typed non-nullable. Put back, with a comment naming why they survive a non-nullable type: this producer had FIVE separate `any` leaks defeating this very boundary — one of them (`const generatedColumns: any[]`) inside `generateColumns()` itself, where it left a whole emit path unchecked even after the return was annotated. A file with that history is where a cheap runtime guard earns its place. Defence in depth BEHIND the typing, not a substitute for it: the tombstones and the removed casts still stand. Verified the guards survive minification into `dist/index.js` rather than being stripped by the `never` narrowing that makes them unreachable by type — a guard the bundler deletes would be no guard at all. 2. RENAMED off a collision. `GridColumn` collided with the pre-existing `packages/fields/src/widgets/GridField.tsx:82`, failing `scripts/__tests__/one-authority-per-exported-name-6273.test.ts`. The two are genuinely different shapes (a fields-widget column config vs this producer's emit type), so they are renamed apart rather than re-pointed at one authority: GridColumn -> ObjectGridColumn GridColumnDraft -> ObjectGridColumnDraft GridColumnHolds -> ObjectGridColumnHolds Matching this file's own precedent, `ObjectGridColumnState` (:257), rather than the aliased-import pattern the gate cites generically. `RetiredListColumnKey` is unique and keeps its name — it describes `ListColumn`'s keys, so an `ObjectGrid` prefix would misname it. ⛔ `KNOWN_COLLISIONS` is untouched. It is a SHRINK-ONLY baseline, so the entry the failure message offers would have weakened the gate rather than satisfied it; `scripts/` carries no diff in this PR. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- packages/plugin-grid/src/ObjectGrid.tsx | 50 +++++++++++++------ .../__tests__/columnEmitBoundary-6004.test.ts | 32 ++++++------ 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index f7c063048..4116bcfbf 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -582,7 +582,7 @@ function normalizeColumns( export type RetiredListColumnKey = Exclude; /** The undeclared-but-live keys this producer holds. See the docblock above. */ -export interface GridColumnHolds { +export interface ObjectGridColumnHolds { /** HELD, objectui#6424 — `data-table` renders it; `TableColumn` does not declare it. */ headerIcon?: React.ReactNode; /** HELD — consumed by this file's own reorder pass before the array reaches the slot. */ @@ -599,16 +599,16 @@ export interface GridColumnHolds { * separate from the enrichment map — so the pre-fold shape needs a name, and * this is it. */ -export type GridColumnDraft = +export type ObjectGridColumnDraft = Omit & { type?: string } - & GridColumnHolds + & ObjectGridColumnHolds & { [K in RetiredListColumnKey]?: never }; /** Post-fold: what actually reaches `DataTableSchema.columns: TableColumn[]`. */ -export type GridColumn = +export type ObjectGridColumn = TableColumn - & GridColumnHolds + & ObjectGridColumnHolds & { [K in RetiredListColumnKey]?: never }; /** The row heights this grid styles — the five `RowHeight` values the spec admits. */ @@ -1652,7 +1652,7 @@ export const ObjectGrid: React.FC = ({ if (message) console.warn(message); }, [schema.columns, columnDiagnosticBlockType, schema.objectName, columnDiagnosticLabel]); - const generateColumns = useCallback((): GridColumnDraft[] => { + const generateColumns = useCallback((): ObjectGridColumnDraft[] => { // Map field type to column header icon (Airtable-style) const getTypeIcon = (fieldType: string | null): React.ReactNode => { if (!fieldType) return ; @@ -1809,7 +1809,7 @@ export const ObjectGrid: React.FC = ({ // `any` — and an `any` SPREAD into an object literal collapses the // WHOLE literal to `any`, which silently un-checks every other key // in it. Measured: without this annotation the emit below infers - // `any[]`, and `GridColumnDraft` cannot bite on any member. Naming + // `any[]`, and `ObjectGridColumnDraft` cannot bite on any member. Naming // the producer vocabulary here stops `any` at this one boundary. const baseInferredType: string | null = col.type || objectDefField?.type || inferColumnType({ field: col.field }) || null; const formatHint = (col as any).format ?? objectDefField?.format; @@ -2111,7 +2111,7 @@ export const ObjectGrid: React.FC = ({ if (!objectSchema) return []; - const generatedColumns: GridColumnDraft[] = []; + const generatedColumns: ObjectGridColumnDraft[] = []; // Default columns priority (when schema doesn't specify columns): // 1. The object's `highlightFields` semantic role (ADR-0085). // 2. Otherwise, all schema fields with system-managed fields pushed to the end. @@ -2369,7 +2369,7 @@ export const ObjectGrid: React.FC = ({ ); } - const columns: GridColumn[] = generateColumns().map((col): GridColumnDraft => { + const columns: ObjectGridColumn[] = generateColumns().map((col): ObjectGridColumnDraft => { // Enrich each column with its field type so the data-table's type-aware // inline editor can pick the matching control (dropdown for select, // checkbox for boolean) the form uses, instead of a plain text box. @@ -2389,10 +2389,25 @@ export const ObjectGrid: React.FC = ({ // not an aside: cell renderers read translated options off the `fieldMeta` // built inside `generateColumns()`, and the inline editor reads them off // the object schema. Neither ever consulted `col.options`. - if (col.accessorKey === '_actions') return col; + // ⛔ THE `!col` GUARD IS KEPT ON PURPOSE — do not delete it as dead code. + // + // `ObjectGridColumnDraft` forbids null, so by the types this branch is + // unreachable, and that is exactly the reasoning that would remove it. The + // reason it stays is that this producer's type guarantee has been untrue in + // practice, repeatedly: objectui#6004 measured FIVE separate `any` leaks + // that defeated this boundary, one of them (`const generatedColumns: + // any[]`) INSIDE `generateColumns()` itself, where it left a whole emit + // path unchecked even after the return was annotated. Each was invisible + // until someone measured it. + // + // So this is defence in depth BEHIND the typing, not a substitute for it — + // the tombstones and the removed casts are the primary guard. Deleting this + // line converts a tolerated null into a throw, which is how the protection + // would be lost a second time for a perfectly good reason. + if (!col || col.accessorKey === '_actions') return col; const fieldDef = (objectSchema as any)?.fields?.[col.accessorKey]; if (!fieldDef) return col; - const next: GridColumnDraft = { ...col }; + const next: ObjectGridColumnDraft = { ...col }; if (next.type == null && fieldDef.type) next.type = fieldDef.type; // Read-only / computed / binary fields are not value-editable in place — // mark the column so the data-table never opens an editor (otherwise it @@ -2423,10 +2438,15 @@ export const ObjectGrid: React.FC = ({ // `normalizeTableColumnType` for why absence beats folding onto `'text'`. // // This pass is also where the draft becomes the real thing: `type` is the - // one member whose vocabulary differs between `GridColumnDraft` (producer - // spelling) and `GridColumn` (what the slot declares), so the fold and the + // one member whose vocabulary differs between `ObjectGridColumnDraft` (producer + // spelling) and `ObjectGridColumn` (what the slot declares), so the fold and the // type transition are the same step (objectui#6004). - .map((col): GridColumn => { + .map((col): ObjectGridColumn => { + // ⛔ Kept on purpose, same reason as the `!col` guard in the enrichment + // map above (objectui#6004) — unreachable by type, retained because this + // producer's types have not held in practice. Destructuring a null below + // would throw where the pre-#6004 code passed it through. + if (!col) return col; const { type: producerType, ...rest } = col; if (producerType == null) return rest; const normalized = normalizeTableColumnType(producerType); @@ -3557,7 +3577,7 @@ export const ObjectGrid: React.FC = ({ const displayColumns = generateColumns().filter((c) => c.accessorKey !== '_actions'); // Build a lookup of column metadata for smart rendering - const colMap = new Map(); + const colMap = new Map(); displayColumns.forEach((col) => colMap.set(col.accessorKey, col)); // Identify special columns by inferred type for visual hierarchy diff --git a/packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts b/packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts index a5e913018..da2c23ed9 100644 --- a/packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts +++ b/packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts @@ -26,7 +26,7 @@ */ import { describe, it, expect } from 'vitest'; import type { ListColumn, TableColumn } from '@object-ui/types'; -import type { GridColumn, GridColumnDraft } from '../ObjectGrid'; +import type { ObjectGridColumn, ObjectGridColumnDraft } from '../ObjectGrid'; /** True only for `any`. `any` is the one type both branches of a conditional accept. */ type IsAny = 0 extends 1 & T ? true : false; @@ -103,7 +103,7 @@ describe('objectui#6004 — the emit boundary is an instrument, not a decoration ...(c.summary && { summary: c.summary }), })); // @ts-expect-error objectui#6004 — `summary` is tombstoned on the emit. - const refused: GridColumnDraft[] = emitted; + const refused: ObjectGridColumnDraft[] = emitted; expect(refused).toHaveLength(1); }); @@ -114,7 +114,7 @@ describe('objectui#6004 — the emit boundary is an instrument, not a decoration it('the emit type refuses a tombstoned key written out', () => { const emitted = { header: 'Name', accessorKey: 'name', label: 'Name' }; // @ts-expect-error objectui#6004 — `label` is tombstoned on the emit. - const refused: GridColumnDraft = emitted; + const refused: ObjectGridColumnDraft = emitted; expect(refused.accessorKey).toBe('name'); }); @@ -127,14 +127,14 @@ describe('objectui#6004 — the emit boundary is an instrument, not a decoration * emit, and a key both declare is untouched. */ it('every undeclared ListColumn key is tombstoned, and declared ones are not', () => { - type _FieldTombstoned = Expect; - type _LinkTombstoned = Expect; - type _ActionTombstoned = Expect; - type _PrefixTombstoned = Expect; - type _HiddenTombstoned = Expect; + type _FieldTombstoned = Expect; + type _LinkTombstoned = Expect; + type _ActionTombstoned = Expect; + type _PrefixTombstoned = Expect; + type _HiddenTombstoned = Expect; // Shared with `TableColumn`, so NOT tombstoned — the Exclude has to keep them alive. - type _SortableLives = Expect; - type _WidthLives = Expect; + type _SortableLives = Expect; + type _WidthLives = Expect; expect(true).toBe(true); }); @@ -146,7 +146,7 @@ describe('objectui#6004 — the emit boundary is an instrument, not a decoration */ it('accepts the three held keys — headerIcon, pinned, wrap', () => { const held = { header: 'H', accessorKey: 'a', headerIcon: null, pinned: 'left' as const, wrap: true }; - const accepted: GridColumnDraft = held; + const accepted: ObjectGridColumnDraft = held; expect(accepted.pinned).toBe('left'); }); @@ -165,22 +165,22 @@ describe('objectui#6004 — the emit boundary is an instrument, not a decoration */ it('the emit type refuses the retired `options` key', () => { // @ts-expect-error objectui#6004 — `options` retired from this producer's emit. - const refused: GridColumnDraft = { header: 'S', accessorKey: 'stage', options: [] }; + const refused: ObjectGridColumnDraft = { header: 'S', accessorKey: 'stage', options: [] }; expect(refused.accessorKey).toBe('stage'); }); /** * The pre-fold / post-fold split (objectui#5853's fold is what separates - * them). `GridColumnDraft.type` is the producer's raw inference vocabulary; - * `GridColumn.type` is the narrow union `TableColumn` declares. If someone + * them). `ObjectGridColumnDraft.type` is the producer's raw inference vocabulary; + * `ObjectGridColumn.type` is the narrow union `TableColumn` declares. If someone * collapses the two types into one, one of these goes red. */ it('the draft carries the producer vocabulary and the folded column carries the declared union', () => { - const draft: GridColumnDraft = { header: 'A', accessorKey: 'a', type: 'lookup' }; + const draft: ObjectGridColumnDraft = { header: 'A', accessorKey: 'a', type: 'lookup' }; expect(draft.type).toBe('lookup'); // @ts-expect-error objectui#5853 — `lookup` is not a declared `TableColumn.type`. - const folded: GridColumn = { header: 'A', accessorKey: 'a', type: 'lookup' }; + const folded: ObjectGridColumn = { header: 'A', accessorKey: 'a', type: 'lookup' }; expect(folded.accessorKey).toBe('a'); }); });