Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-dashboard): type ObjectDataTable's column emit against the TableColumn[] slot it fills - #6427
Merged
Conversation
… slot it fills
`enrich()` returned `NormalizedColumn` (`[key: string]: any`), so nothing checked
what this producer wrote into `DataTableSchema.columns: TableColumn[]`.
`{ ...col, ...fieldMeta }` wrote six keys `TableColumn` does not declare —
`label`, `options`, `referenceTo`, `format`, `currency`, `decimals`.
All six retire from the emit: the consumer's measured read set contains none of
them, and declaring a key nothing reads is the same `declared != enforced`
defect facing the other way. Rendering is unchanged — every one of those values
still reaches the cell through the `FieldMeta` the `cell` closure captures.
`type` is objectui#5853's and unchanged. `name` is objectui#5120's, still held,
still written, now declared at the seam instead of arriving inside a spread.
The emit type carries ADR-0049 `?: never` tombstones rather than being a bare
`TableColumn` annotation: measured on this program, a bare annotation raises no
error here at all, because excess-property checking exempts spread properties.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q…orce The full-`FieldMeta` spread pin is refused for two independent reasons — the tombstones AND `FieldMeta.type?: string` not fitting the union objectui#5853 narrowed `TableColumn.type` to. Measured by removing the tombstones: that directive stays used, so on its own it pinned "the spread is refused" without pinning why, and would have survived the enforcement being deleted. Adds the isolating pin (`Omit<FieldMeta, 'name' | 'type'>` — exactly the six retired members) and states which machinery refuses which case. 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
|
os-support-ai
marked this pull request as ready for review
August 25, 2026 23:58
Uh oh!
There was an error while loading. Please reload this page.
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#6373
ObjectDataTable.enrich()returnedNormalizedColumn, whose[key: string]: anyaccepts anything, so nothing checked what this producer wrote intoDataTableSchema.columns: TableColumn[].{ ...col, ...fieldMeta }wrote seven keysTableColumndoes not declare. Six retire; the seventh is #5120's and is unchanged.⭐ The rule, so #6004 gets the same answer on a different key set
This is #6004's remedy applied to the second producer, and #6004's own key set (
headerIcon,options,pinned,wrap,essential) is different. So the criterion is stated as a rule rather than as seven verdicts:The measured read set of
data-table.tsx(comments stripped, everycol.<key>):accessorKey28,width6,align5,header4,className4,sortable3,editable3,cell2,name2,headerIcon2,fitContent4,cellClassName/resizable/type1 each. Not one of the six retired keys appears at any count.Verdicts
labelheaderis the adapter's spelling, and #5351 already retiredlabel's alias on the consumer side.optionsfieldMeta; pinned below by renderingTechnologywithoptionsgone from the column.referenceTo$expandwhitelist that does care readsschema.columns(the AUTHORED list) incomputeLookupExpand, never the enriched output.formatcellclosure.currencydecimalstypenormalizeTableColumnTypefold stands unchanged, and the value is now written out explicitly instead of arriving in the spread.namedata-tablereadscol.accessorKey || col.nameand holds that alias while two published skill guides teach a{ name, label }column. The producer keeps writing it, byte for byte what the spread wrote; the hold is now DECLARED at the seam instead of arriving anonymously.Annotating the return
TableColumnand leaving the spread in place raises no error at all. TypeScript's excess-property check is a freshness check on the properties an object literal writes OUT; properties arriving through a spread are exempt. Probed on this program before the fix was written:So the emit type carries ADR-0049
?: nevertombstones — this repo's convention for a key that is refused rather than absent (StaticTableColumn,crud.tsconfirm) — which bite by assignability instead of freshness. The member is derived (Exclude<keyof FieldMeta, keyof TableColumn | 'name'>), never hand-listed, so a futureFieldMetamember is tombstoned by default and has to be adjudicated to escape.Reverse verification
Both ablations state their predicted direction first, prove the mutation reached disk by grepping the injected AND removed text (a zero-hit
perl -iexits 0), and restore by comparing the file's hash against the HEAD blob, withgit diff HEADproving the restore. No build/diststep is involved: the test imports../ObjectDataTablerelatively, so the mutation is in the module under test.Ablation A — put
{ ...fieldMeta }back. Predicted: the runtime census red, and the source type-check red too. Observed both; the third failure was not predicted and is explained below.I predicted two red census tests and got three: the authored-column case also censuses the keys the producer ADDS beyond the author's, so restoring the spread reddens it too. Same direction, one more instrument than expected.
Ablation B — delete the tombstones only, keep everything else. This one falsified my first prediction and changed the test.
Predicted: the source type-check stays green (proving a bare annotation enforces nothing) and the test project goes red with two unused-directive errors. Observed: both green. Diagnosis: the full-
FieldMetaspread pin is refused for two independent reasons — the tombstones, andFieldMeta'stype?: stringnot fitting theTableColumnTypeunion #5853 narrowed this key to. So that directive stays used without the tombstones: on its own it pinned "the spread is refused" without pinning why, and would have gone on passing after the enforcement was deleted. That is the "passes for the wrong reason" trap this repo's own headers warn about, caught only because the ablation was run.The pin was split (second commit). Re-run of ablation B against the corrected instrument, prediction stated first and met exactly:
Exactly one directive — the isolating one (
Omit<FieldMeta, 'name' | 'type'>, i.e. precisely the six retired members). The hand-written-key pin is refused by excess-property checking alone and is documented as such, so the three type pins now say which machinery covers which case.What is pinned, and why it is not a blind instrument
packages/plugin-dashboard/src/__tests__/ObjectDataTable.emitBoundary-6373.test.tsx(10 tests). A test that merely renders the table would stay green with the annotation removed, so nothing here is a render smoke test:TableColumnSchema.shape(@object-ui/types/zod), whichzod-mirror-parity.test.tskeeps in step with the interface — so there is no key list in the file to drift. It censuses the objects the widget actually hands todata-table, which catches a future spread, a computed write or anas anydetour by what LANDED. The census runs on the auto-derive path, where every key on the object was written byenrich, so it is exact rather than "the author's keys plus ours"..shaperead that silently answered{}would make "none of the retired keys is declared" pass vacuously.optionsgone from the column, the select cell still rendersTechnology(nottech), and the lookup cell still rendersAda(notu1).{ format: '$0,0', currency: 'EUR', field: 'amount' }keeps all three. Retirement is about what the producer writes, andnormalizeColumns' documented rule that authored spellings survive is unchanged.@ts-expect-errorpins with a positive control, checked bytsc -p tsconfig.test.json— chained from this package'stype-checkscript and run by CI's Type Check job. Coverage was measured, not assumed:tsc -p tsconfig.test.json --listFilesincludes this file, andcheck-type-check-coverage.mjsreports41/41 packages compile their tests, 0 declared debt. (A test header in this package still claims the opposite; filed as finding(plugin-dashboard): a test header still says this package's tests are compiled by nothing — that debt was paid #6426.)Local gates — run at
c2fcae536, the final commitUnion re-run after the last commit, on a clean tree:
vitest run packages/plugin-dashboard/Test Files 81 passed (81)/Tests 764 passed (764)Test Files 1 passed (1)/Tests 10 passed (10)Test Files 6 passed (6)/Tests 265 passed (265)tsc --noEmit(plugin-dashboard)tsc -p tsconfig.test.jsonpnpm --filter @object-ui/plugin-dashboard lint✖ 393 problems (0 errors, 393 warnings), exit 0check-control-bytes✅ OK (scanned 5315 tracked text file(s); skipped 85 binary)check-changeset-presence✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major✅ No changeset declares a major bump.check-changeset-fixed✅ All workspace packages are in the changeset fixed group.check-type-check-coverage✅ test type-check coverage: 41/41 packages compile their tests, 0 declared debtcheck:vi-mock-specifiers✅ OK (3795 tracked source file(s), 2098 test-named; 447 carry a mock …)check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.check:self-import(*)
skill-guide-data-table-binding,widget-dom-leak-sweep,table-declared-equals-enforced,table-column-type-read-set,zod-mirror-parity,table-column-type-canonical.Declared narrowing. Repo-wide
pnpm lint(turbo run lint, every package) was narrowed to this one package'seslint .. Three pieces of evidence, not an assumption: the population came from eslint's own config resolution, not from my guess about which files count;--format jsonreports 111 files linted in this package; andeslint.config.jsusestseslint.configs.recommendedwithlanguageOptions: { ecmaVersion, globals }and noparserOptions.project/projectService, i.e. type-aware linting is off — so a type-only edit here cannot move a verdict on any file it did not touch, and no other package contains a changed file. CI runs the full farm regardless.One caveat recorded rather than buried: an exploratory
eslint . --no-inline-configrun exited 1 onMetricCard.tsx/MetricWidget.tsx, files this PR does not touch — the flag disables their existingeslint-disable-next-linecomments. That flag is not what this repo's lint script runs (eslint ., no flag), so it was a self-inflicted red, not a finding.Out of scope, filed
data-tablereads two column keysTableColumndoes not declare —headerIconandfitContent#6424 —data-tableREADS two keysTableColumndoes not declare (headerIcon2 reads,fitContent4, the latter through twoas any). The consumer-side mirror of this card, at the same slot; finding(plugin-grid): ObjectGrid'sgenerateColumns()is untyped (any[]), so nothing type-checks what it writes intoDataTableSchema.columns: TableColumn[]— the hole that hid #5853 and #5453 #6004'sheaderIconverdict overlaps on one of the two and the two must agree.ObjectDataTableHONOURS five undeclared authored column keys (format,options,referenceTo,currency,decimals) asbuildFieldMetaoverrides — tested behaviour ($150,000/60%) that the published types refuse and the zod mirror strips. The read half of this seam; deliberately not folded in, because unlike the six retired writes these have a live consumer and declare-or-retire is a public-surface decision.⛔ Out of scope for this PR: #6004 remains open (different package, different keys, independent adjudication), #5120 remains open (
namestays held), and #5453 remains open. None of those three is addressed here — the wording is deliberate, so that no closing keyword sits next to a card number this PR must not close.Behaviour
None changes. Rendering is driven by the
cellclosure overfieldMeta, untouched. Authored keys still pass through. The only observable difference is that the objects handed todata-tableno longer carry six keys that nothing read — keys that, becausebuildFieldMetaalways returns all eight members, were present on every emitted column even when the schema said nothing about them.Generated by Claude Code
Generated by Claude Code