Uh oh!
There was an error while loading. Please reload this page.
refactor(plugin-grid): retire the dead per-column wrap forward, measured against data-table's read set - #6651
Merged
Conversation
`generateColumns()` copied a per-column `wrap` onto the column object handed to `data-table`, and `data-table.tsx` never read it. Measured on this ref with comments stripped: a column-level `wrap` scores 0 reads, against 34 for `accessorKey`, 5 for `align`, 4 for `header`, 4 for `className` and 2 for `fitContent` in the same query shape. There is also nothing for it to switch on. `data-table`'s cell wrapper is a two-way `isFit ? 'w-full whitespace-nowrap' : 'truncate w-full'`, with a `title` tooltip as the only concession to overflow, and the file does not read `density` or `rowHeight` at all — no clamp, no expand, no wrap affordance. Unlike `pinned` (also unread by `data-table`, but consumed by this file's own reorder pass and re-expressed as a sticky `className`), `wrap` had no second road to any consumer, so enforce-or-remove resolves to remove. Retired rather than merely deleted: `wrap` is no longer carved out of `RetiredListColumnKey`'s Exclude, so the derived tombstone band now types it `never` and re-adding the forward is a compile error naming the key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
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-sales
marked this pull request as ready for review
August 28, 2026 09:11
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 28, 2026
os-sales pushed a commit
that referenced
this pull request
Aug 28, 2026
…derIcon hold `ObjectGridColumnHolds` exists for keys `data-table` reads that `TableColumn` does not declare. `headerIcon` stopped meeting that entry condition when objectui#6615 declared it on `TableColumn`, and nothing went red at the moment of loss — the same silent expiry recorded for `options`. Measured rather than derived (the previous round could only derive it, because the file was fenced behind another claim): - `Equal<ObjectGridColumn['headerIcon'], TableColumn['headerIcon']>` is `true`, and so is the `ObjectGridColumnDraft` form. Instrument controls in the same shape answer `false`/`true` as expected, so the probe has resolution. - Ablation on the resolved emit types (every member's type printed via the TypeScript checker, through the same program the test project uses): with the member deleted both types are byte-identical at 27 members. Positive control — deleting `pinned` instead takes them to 26, `pinned` gone. - `headerIcon` is not a `ListColumn` member (control: `width` is), so it was never in the derived `RetiredListColumnKey` band (control: `wrap` is). `pinned` is untouched and stays load-bearing on both counts: `TableColumn` does not declare it and the band carves it out, so the hold is its only declaration on the emit types, and this file's reorder pass re-expresses it as the sticky `className` `data-table` actually reads. Removing the hold moves a liveness claim rather than deleting one: the emit types now get `headerIcon` from `TableColumn` alone. `columnHoldsExpiry-6424` pins that dependency, so reverting the declaration is loud instead of silent. Also corrects the docblock's general claim that the held keys are "undeclared by `TableColumn`", false as written since #6615, in the places #6651 left alone — including the emit-boundary pin that still said the interface declares two keys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
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#5453
ObjectGrid.generateColumns()copied a per-columnwraponto every column object handed todata-table, andpackages/components'data-table.tsxnever read it. This retires the forward.The measurement that decided the fork
Triage graded this card with two legs and told the dev to pick by measurement, not preference: does
data-table.tsxtoday offer any clamp / expand / wrap affordance for long cell text? If no, delete the forward; if yes, the implement leg becomes natural and is a behaviour addition that must stop and report. The answer is no, on two independent counts.1. Nothing reads the key. Every column-key read in
data-table.tsx, comments stripped so prose mentions cannot be counted as reads:accessorKeywidthalignheaderclassNamesortablefitContentheaderIconcellClassNamewrapThe sibling counts are the positive control the dispatch asked for: the same query shape does find the keys that are genuinely consumed, so the zero is a measurement and not a mis-aimed grep. The raw string
wrapdoes occur in that file — every occurrence isflex-wrap,whitespace-nowrap, or a variable namedwrapper.2. There is nothing for it to switch on. The implement leg was
wrap: truedropping thetruncate, which presupposes the renderer has somewhere to put a second line. It does not. The cell content wrapper is a two-way switch with no third state:isFitiscol.fitContent, and it selects single-line, no clipping — the opposite of wrapping. A nativetitletooltip is the only concession to overflow. There is noline-clamp, nowhitespace-normal, nobreak-words, no expand affordance, and the file does not readdensityorrowHeightat all. So the enforce-or-remove default applies and the removal leg fires.Why removed rather than declared
wraphad no second road to a consumer. That check is what separates this verdict frompinned, whichdata-tablealso never reads (0 reads, same query) and which is nonetheless kept:ObjectGrid's own reorder pass consumespinnedbefore the array reaches the slot and re-expresses it as the stickyclassNamethe renderer does read.wraphad no such pass anywhere in the repo.Retired, not merely deleted — the key is typed, as both gradings required
Both the original grading and triage's restatement said either resolution must also type the key, since it was read off an untyped
col. #6637 (#6459) landed the machinery for that yesterday, so this card had only to movewrapacross it:wrapis no longer carved out ofRetiredListColumnKey'sExclude, so the derived tombstone band now types itnever.ObjectGridColumnHoldsmember is gone — that hold existed only because this card waspm:blocked.Re-adding
...(col.wrap !== undefined && { wrap: col.wrap })is now a compile error namingwrap. Otherwise "retired" is just a deleted line the next edit can put back for free. Because the band derives from the authoredListColumn, removing the carve-out was the whole edit — no hand-maintained list to update.Shown able to fail
The pin is compile-time, so a rendering test is blind to it. The refusal was ablated: putting the
'wrap'carve-out back into theExclude(mutation confirmed on disk by blob hasha4072b0eto59da928d, plus anchored grep counts in both directions) turnstsc -p tsconfig.test.jsonred, exit 2, with both predicted diagnostics:Restore proven byte-identical to
HEAD(blob back toa4072b0e,git diff HEADempty). Note the standing property this relies on: TS2578 makes an unused@ts-expect-erroran error, so a green type-check is itself proof the tombstone bites.--listFilesconfirms the pin file is in the test program (tsconfig.jsonexcludes__tests__,tsconfig.test.jsonincludes it), so the check is not silently skipping it.What is deliberately NOT changed
The authorable spelling is untouched.
@objectstack/specstill declaresListColumn.wrap, andpackages/react's spec-bridge still forwards it into the grid schema — itsSpecBridgepin test still passes unchanged. Whether that spec property should keep being declared with no renderer anywhere is a spec-side enforce-or-remove question outside this card's fence; filed as #6650 rather than settled from inside this renderer.Runtime behaviour is unchanged: an authored
wrapdid nothing before and does nothing now. What changed is that the code no longer impliesdata-tablemight consume it.Verification
All runs from the repo root. Final head
fc721d2e; the union below was re-run on that head.tsc --noEmit+tsc -p tsconfig.test.json(plugin-grid)VERDICT command-exit 0vitest run packages/plugin-grid/src/__tests__/vitest runcomponents/renderers/complex + spec-bridgeturbo run type-check --filter=...@object-ui/plugin-grideslint(plugin-grid, whole package)check-control-bytescheck-vi-mock-specifierscheck-package-self-importcheck-changeset-presence/-no-major/-fixedThe type-check filter is the downstream direction — verified by enumerating it rather than assuming:
console,app-shell,plugin-designer,plugin-report,plugin-view,site, and three example apps. Consumers read a rebuilt.d.ts, sincetype-checkdependsOn^build.The changeset gate ruled one owed and it is declared
minor(majoris mechanically refused repo-wide): runtime behaviour is unchanged, but@object-ui/plugin-grid's exported emit types narrow —ObjectGridColumnHoldsloseswrap, andObjectGridColumnDraft['wrap']becomesnever.One stale claim corrected in passing
The docblock this PR edits asserted the held keys are "undeclared by
TableColumn". That is no longer true ofheaderIcon—TableColumndeclares it today (packages/types/src/data-display.ts:369, inside the interface closing at 414), so itsObjectGridColumnHoldsentry is redundant rather than load-bearing. Only the sentence was corrected, since the hold itself is #6424's to settle; flagged here rather than silently restated.Generated by Claude Code