Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-grid): type ObjectGrid's column emit against the TableColumn[] slot it fills - #6461
Merged
Merged
Conversation
…mn[] slot it fills `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<any>`, 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…mit type off a collision 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Aug 26, 2026
os-support-ai
marked this pull request as ready for review
August 26, 2026 02:04
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6004
Verified at
356d0d93a(the final commit; every gate below was run on that tree).The finding this PR paid for
The obvious remedy is inert, and at this seam it is more inert than at #6373's. Measured on this program before choosing a shape, each leg restored byte-identically afterwards:
generateColumns(): TableColumn[], nothing else changedtsc --noEmitexit 0, zero diagnosticssortable: 'NOT_A_BOOLEAN')#6373 found that spread properties escape excess-property checking. Here freshness is gone entirely: every emit literal is a
.map()callback's return value, so it is inferred into the callback's return type first and reaches the annotation as a non-freshX[]. Written-out keys escape too.⭐ The greens above are real inertness, not a dead compiler: the control probe (
return 'DEFINITELY_NOT_AN_ARRAY'inside the same annotated function) raisedTS2322, so the annotation was being honoured.And underneath that, the reason no annotation could bite at all
objectSchemaisuseState<any>(:692), so the field type read off it isany— and ananyspread into an object literal collapses the entire literal toany, silently un-checking every other key in it. That is why the third probe stayed green on a declared key. Measured directly: with the return annotated but the locals untouched, path A's emit expression isany[].So the fix has three parts, and the first is load-bearing rather than cosmetic:
baseInferredType,inferredType,resolvedType×2, plusfieldType) soanystops at that boundary.& { [K in Exclude<keyof ListColumn, keyof TableColumn | 'wrap' | 'pinned'>]?: never }. Derived from the authored input type, never hand-listed, so a key added to the spec'sListColumntomorrow is refused by default. Tombstones bite by assignability, which never depended on freshness.anycast at the call sites gone.The census was wider than the card's
The card named three call sites. There are five
anys defeating the emit type::2108:2176:2227:3398filter((c: any) => …)generatedColumns: any[]generateColumns(); it left the schema-derived emit path unchecked even after the return was annotatedAdjudication — every key, with the read-count behind it
Read sets measured from source, comments stripped. Two consumers, because the array is read twice before it reaches the slot.
headerIcondata-table.tsx×2 (renders it into the header cell)TableColumnshould declare it is #6424's call; declared at the seam meanwhile so the hold is visible instead of anonymous.pinnedObjectGrid.tsx×5 (the left/right reorder + the frozen-column verdict);data-table×0classNamedata-tableactually reads.wrappm:blockedon #5415, whose outcome decides implement-vs-remove. Retiring it here would settle a blocked card from outside its own thread. Byte-for-byte unchanged.optionsdata-tablehas no column-leveloptionsread; its select/boolean editors come from the host viarenderCellEditor, and this component'srenderCellEditorrebuilds the field fromobjectSchema.fields[accessorKey], not from the columntypenameSecond road proven before retiring
options(the check is part of the rule, not an aside): cell renderers read translated options off thefieldMetabuilt insidegenerateColumns(), and the inline editor reads them off the object schema. Neither ever consultedcol.options. No rendering change.data-tablereads two column keysTableColumndoes not declare —headerIconandfitContent#6424 (headerIcon,fitContent) —headerIconis in my emit set and HELD here; theTableColumndeclaration decision stays finding(components):data-tablereads two column keysTableColumndoes not declare —headerIconandfitContent#6424's.fitContentis written on the_actionscolumn, outsidegenerateColumns(), untouched.(col as any)— the #6425 shape at the other producer #6458 rather than folded in.wrapinto the DataTable column object, but nothing indata-table.tsxever reads it #5453 —wrap, held as above.Census correction to the card
The card's suggested key list named
essential. It is not on the emit surface at all: it is read off the authored column and turned into aclassName, and it is not aListColumnmember. Recorded on #6458.Review items from the second pass
1. Null guards restored (
356d0d93a)The first pass narrowed away
if (!col || …)in the enrichment map and the fold, reasoning the emit is now typed non-nullable. Put back, with a comment naming why they survive a non-nullable type — otherwise the next reader correctly identifies them as unreachable-by-type and deletes them, losing the protection a second time for a good reason.The argument is this PR's own census: five separate
anyleaks defeated this boundary, one of them insidegenerateColumns()itself, where it left a whole emit path unchecked even after the return was annotated. Each was invisible until measured. Defence in depth behind the typing — the tombstones and the removed casts still stand.⭐ Verified the guards are not stripped. TypeScript narrows
coltoneverin those branches, which is exactly the condition under which one might expect a bundler to drop them. Checked in the built artifact:Both present. A guard the bundler deletes would be no guard at all.
2. Renamed off an exported-name collision (
356d0d93a)GridColumncollided with the pre-existingpackages/fields/src/widgets/GridField.tsx:82, failingscripts/__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:GridColumnObjectGridColumnGridColumnDraftObjectGridColumnDraftGridColumnHoldsObjectGridColumnHoldsMatching this file's own precedent,
ObjectGridColumnState(:257), rather than the aliased-import pattern the gate cites generically.RetiredListColumnKeyis unique and keeps its name — it describesListColumn's keys, so anObjectGridprefix would misname it.⛔
KNOWN_COLLISIONSis 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 (git diff --quiet <base> -- scripts/→ clean).Tests — the defect is type-level, so the pins are compile-time
packages/plugin-grid/src/__tests__/columnEmitBoundary-6004.test.ts. A test that merely renders the grid is blind to this: it stays green with the whole emit type deleted.Coverage measured, not assumed —
tsconfig.jsonexcludes**/__tests__/**, so only the test project checks this file:Each
@ts-expect-erroris written to be refused for exactly one reason — a directive refused for two pins neither, because it stays "used" when one is deleted (#6373's lesson). Where freshness 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.Ablation — tombstone deleted from both emit types, re-run on the final tree:
Predicted before running, and the isolation held: the
optionspin and thetype-fold pin stayed green, proving they are refused by their own independent machinery rather than riding on the tombstone.Forward reverse-verification — all five emit paths bite (predicted red before each run; every leg confirmed on disk and restored byte-identically):
sortablesummarylabellinkpush)Type 'true' is not assignable to type 'undefined'The test also pins the two mechanisms as executable claims, since nothing else in the repo states them: that an
anyin a conditional spread collapses the literal, and that a bareTableColumn[]annotation accepts the undeclared key. If TypeScript tightens either, these go red and the docblock gets re-measured — a useful red.Gates run (all on
356d0d93a)pnpm --filter @object-ui/plugin-grid type-check(tsc --noEmit && tsc -p tsconfig.test.json)pnpm exec vitest run packages/plugin-grid/ scripts/__tests__/one-authority-per-exported-name-6273.test.tsnode scripts/check-changeset-presence.mjspnpm --filter @object-ui/plugin-grid lintanycasts)type-check—app-shell,plugin-view,plugin-report,plugin-designer(downstream/prefix direction)Done, exit 0Repo-wide lint is CI's run, not attempted here. The narrowing is measured: eslint's population for this package is 129 files (from its own config, counted via
--format json), my diff touches 3 files all inside it, andeslint.config.jssets noparserOptions.project/projectService— type-aware linting is not enabled, so my diff cannot move the verdict of any file it does not contain.Out-of-scope findings filed (unassigned, not fixed here)
size, a keydata-tablenever reads — saved widths are dropped on the ungrouped path #6457 — ObjectGrid writes the persisted column width assize, a keydata-tablenever reads. User-visible: resize a column, reload, the width is gone on the ungrouped path. Grouped mode stampswidthand works, which is both the corroboration and the fix direction. (Visible again in this PR's built bundle:size: tin the persisted-width map.)(col as any)— the #6425 shape at the other producer #6458 — ObjectGrid honours four undeclared authored column keys via(col as any)(format,options,appearance,essential) — the finding(plugin-dashboard): ObjectDataTable honours five undeclared authored column keys as field-meta overrides #6425 shape at this producer. Also carries theessentialcensus correction.const dataTableSchema: any, so theDataTableSchemait imports is never applied #6459 —const dataTableSchema: anymeans theDataTableSchemathis file already imports is never applied to the object that receives the emit.Generated by Claude Code
Generated by Claude Code