refactor(plugin-grid): retire the redundant ObjectGridColumnHolds.headerIcon hold - #6686
Merged
Merged
Conversation
…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
…nothing `ObjectGridColumnHolds` is not part of the package entry's exported surface (measured: 0 occurrences in `dist/index.d.ts`, control `ObjectGridColumnState` 1), and the two emit types are byte-identical without the member, so nothing published moves. Empty frontmatter is the gate's own explicit exemption for that case. 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 14:51
This was referenced Aug 28, 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 free
to 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.
Part of #6424
Part of, not the closing keyword, and that is a deliberate deviation from the dispatch order. The order asked for the closing keyword on this card, on the premise that all three items would land. (That keyword is deliberately not spelled next to the card number anywhere in this body: GitHub's parser matches it regardless of any negation around it, and this card must stay open.) Item 1 did not land — its precondition is measurably absent on this ref (evidence below). Closing the card would bury an unfinished, already-ruled item, and the inbox filter only reads open cards. The card stays open for item 1.Two of the three ruled items land here, both measured rather than inherited.
Item 1 — the emit-side
fitContentcast: STOPPED, with evidenceThe line, located on my own ref
Both numbers in circulation are stale. Measured at base
9d86e1d79::3418— stale:3587— stale:3606—if ((col as any).fitContent) continue;Three independent measurements say do not remove it yet.
1a. The ruling's precondition is not met — PR #6673 has not merged
The ruling ties this cast to the
fitContentdeclaration: it "travels with this verdict". That declaration is still in flight — PR #6673 isstate=open, merged=false.Measured on my ref, each zero with a positive control in the same query shape:
fitContentinpackages/types/src/data-display.tsheaderIcon→ 2fitContentinpackages/types/dist/data-display.d.ts(the artefact plugin-grid actually resolves)headerIcon→ 2'fitContent'inkeyof TableColumn, compile-time'width'→ true;'zzNotAKeyZZ'→ false1b. The cast is a no-op, but NOT for the reason the ruling assumed
Read via the TypeScript checker at the site, cast stripped:
applyColumnChrome = (col: any) =>makesorderedColumnsanany[], and the loop adds a secondas any[]. So the cast never compensated for the undeclared key — it compensates for nothing. This is exactly the lesson PR #6673 measured ondata-table.tsx("cast count is not the instrument, the normalization is"), reproduced on the emit side. Removing it buys zero type safety, now or after #6673 lands.1c. It is load-bearing as another guard's only real-file control
columnReadBoundary-6458.test.tsuses this exact cast as its Anti-vacuity control 3, and its own comment says: "If that read is ever retired too, this control must be re-pointed at another real cast — never deleted, and never re-pointed back inside the region."Measured with the test's own scanner regex over the whole file:
(col as any).fitContentat:3606is the only(col as ...)cast read inObjectGrid.tsx. There is no other candidate to re-point at.Observed, not predicted — ablation with the mutation proven on disk and restoration observed:
Baseline on the clean tree for comparison:
Test Files 1 passed (1),Tests 8 passed (8), EXIT=0.⇒ Removing the cast today costs a live guard its only real-file anti-vacuity control and buys nothing. This needs the seat, not me — it is a re-point decision plus an ordering question against #6673.
Item 2 —
ObjectGridColumnHolds.headerIconremoved. MEASURED, not inheritedThe order was explicit that the previous round's type-algebra derivation must not be carried forward. It was not. The probe confirms the derivation — the ruling stands.
The ruled probe
Run through the same program the test project uses (
tsconfig.test.json,paths: {}, so@object-ui/typesresolves through the built.d.ts):EqualofObjectGridColumn['headerIcon']andTableColumn['headerIcon']EqualofObjectGridColumnDraft['headerIcon']andTableColumn['headerIcon']EqualofObjectGridColumn['headerIcon']andTableColumn['width']EqualofTableColumn['headerIcon']with itselfThe two controls are the point: a probe that can only answer
truemeasures nothing.The emit-type ablation — the claim that had never been observed
The actual claim is "the emit types are unchanged with the member absent". Instrument: the TypeScript checker resolves both emit types and prints every member with its resolved type, sorted, to JSON — a structural snapshot, not a derivation. Each leg proves its mutation on disk (anchored grep counts plus a moved blob hash) and its restoration by observation (
git diff HEADempty, hash back), under atrap ... EXIT INT TERMwith absolute paths, restoring via an explicitgit checkout HEAD -- absolute-path.ObjectGridColumn27,ObjectGridColumnDraft27pinnedinsteadpinnedgone from bothheaderIconThe positive control is what makes the identical result a measurement: the instrument demonstrably detects a holds-member removal that matters. The landed change re-snapshots to IDENTICAL against the pre-edit baseline.
The three facts, re-confirmed with controls rather than quoted
headerIconis inkeyof TableColumn(since #6615)'width'→ true;'zzNotAKeyZZ'→ falseheaderIconis not aListColumnmember'width'inkeyof ListColumn→ trueRetiredListColumnKeyband'wrap'in the band → trueObjectGridwrites it at 3 sites, all underschema.showColumnTypeIconsviagetTypeIcon:2129,:2221,:2271data-tablereads it at 1 render site, forwarded verbatim into aspan, never re-expresseddata-table.tsx:1955-1956Note on the read count: the card body says "2 reads", the ACCEPT says "1". Both are right at different granularities — 1 render site, 2 syntactic
col.headerIconoccurrences (a guard and the render). Stated precisely here so the next reader does not have to re-derive it.The
pinnedcontrast, stated explicitly — and untouchedpinnedstays load-bearing on both counts, which is exactly what makesheaderIcon's removal safe andpinned's unsafe:headerIconpinnedTableColumn?falseListColumnmemberExcludecarves it out; it is aListColumnmember (control: true)TableColumncarries itclassNameTwo different routes to the same "not in the band", and they must not be conflated — that conflation is how a measurement blurs the two keys.
pinnedis untouched by this PR.While the hold existed, the emit types declared
headerIconon their own account. They now get it fromTableColumnalone, so reverting #6615 would drop it from both emit types in silence whileObjectGridgoes on writing it at three sites. That is a new exposure created by this change, and the new pin's first assertion covers it — it is the guard the deleted member used to provide implicitly.The pin is not vacuous. Simulating that revert turns it red, mutation proven on disk and restoration observed:
And the pin really is compiled —
tsc -p tsconfig.test.json --listFiles:columnHoldsExpiry-6424.test.ts→ 1 (control:columnEmitBoundary-6004.test.ts→ 1; negative controlzzNoSuchFile→ 0). Green from a file the compiler never read would be worth nothing.Item 3 — the docblock's general claim, corrected
The claim that the held keys are "undeclared by
TableColumn" is false as written since #6615. PR #6651 corrected only the one sentence it was already editing; this PR owns the rest:ObjectGrid.tsxverdict block —headerIconmoves from HELD to DECLARED, with the expiry and the measurement that retired the hold recorded.ObjectGridColumnHoldsdocblock — names "undeclared byTableColumn" as the interface's entry condition, flags that it is a claim about another package that can lapse with nothing going red, and states the rule: re-check per key when the owning card closes, never inherit. Plus the ⛔ that a key whose only declaration is this interface is not in that position.pinnedmember doc — now records both counts that make it load-bearing, and the 27→26 measurement.columnEmitBoundary-6004.test.ts— its docblock still said the interface declares two keys and its test title said "the two held keys". Both false after this change; corrected, and the routes are now pinned in the new file rather than described in prose.Gate verdicts — each exit code captured before any pipe, each quoting the gate's own line
Union re-run after the final commit, at
83ad9ffec:plugin-grid type-check(tsc --noEmit && tsc -p tsconfig.test.json)error TSlines; script name echoed, so not a zero-match no-opvitest run packages/plugin-gridTest Files 97 passed (97)·Tests 890 passed (890)check-changeset-presence✅ 3 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:control-bytes✅ check-control-bytes: OK (scanned 5527 tracked text file(s); skipped 85 binary).plugin-grid lint✖ 676 problems (0 errors, 676 warnings)— all pre-existingcheck:published-dist✅ No published package's build output carries tooling material.check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.check:vi-mock-specifiers✅ check-vi-mock-specifiers: OK (3896 tracked source file(s) ...)check:spec-symbols✅ spec symbol derivation: 1316 files scanned against 4959 spec export namesDeclared narrowing on lint
Repo-wide
pnpm lintisturbo run lintacross 40 packages; I ran the one package this diff touches. Three pieces of evidence, not two:packages/plugin-grid.--format json: 134 files; my three changed files at 0 errors (ObjectGrid.tsx214 warnings, all pre-existing; the two test files 1 and 0).eslint.config.jshas noprojectService/parserOptions/project:— grep exit 1, no matches — so type-aware linting is off and this diff cannot move the verdict of any file it does not touch.--no-inline-config(an objectstack recipe, not this repo's) surfaces 3 errors — indemo/main.tsx,predicate-surface-parity.test.tsxandBulkActionDialog.tsx, none of them in this diff. This repo's lint script iseslint .without that flag, which is what CI runs and what returned 0.Changeset
Empty frontmatter — the gate's own explicit exemption, quoted from its help text: "If this change really should release nothing, say so — that is a pass, not a workaround." Justified by measurement, not assertion:
ObjectGridColumnHoldsis not part of the package entry's exported surface — 0 occurrences in the builtpackages/plugin-grid/dist/index.d.ts(positive control:ObjectGridColumnState, which the entry does re-export, → 1), andheaderIcon→ 0 there too. Runtime behaviour is untouched; this is types, comments and tests.Scope
packages/types,packages/components/src/renderers/complex/data-table.tsx,app-shell,i18n,sdui-parser,plugin-listand@objectstack/specare all untouched —git diff --name-onlyis exactly the threepackages/plugin-gridfiles plus the changeset.Generated by Claude Code
Generated by Claude Code