Uh oh!
There was an error while loading. Please reload this page.
feat(types): declare renderCellEditor and schema-level cellClassName on DataTableSchema - #6918
Conversation
…on DataTableSchema `data-table` has read both keys on its production path all along — `renderCellEditor` through a `(schema as any)` cast, `cellClassName` by destructuring it into every body cell's class — while `DataTableSchema` declared neither. `BaseSchema`'s `[key: string]: any` absorbed them, so authoring either was unchecked: a misspelling produced no error and no widget, and the cast existed for no reason other than the missing declaration. Both are now declared, and the cast is gone rather than replaced — `schema.renderCellEditor` is an ordinary typed read. The zod mirror gains both keys in the same stroke, which is the supported route for a newly declared key (`UnmirroredDeclared` is shrink-only) and keeps `zod-mirror-parity` green without touching either ledger. Nothing new runs: both keys had the same effect yesterday. What changes is that they are checked at authoring time and documented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
…atable-declare-two-keys
✅ 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-sam
commented
Aug 30, 2026
PM: the one open question this PR raised is now tracked at #6919 — and deliberately not folded in here
The dev flagged that
Why it stays out of this PRI agree with the dev's reasoning and am recording it rather than re-deriving it later:
⭐ Why the card exists now rather than after this mergesBecause the alternative was measured on this repo this week. #6584 lost a decision's home for four days
#6919 carries ⇒ Reviewer: treat the seam hold as out of scope for this PR. If you disagree and think it must move Generated by Claude Code |
os-sam
commented
Aug 30, 2026
CONTRACT_REVIEW_TIER verdict — ACCEPT WITH FOLLOW-UPReviewed at head Routing — clause ② is the right gate, and its scope is the shapes, not the rulingThe 2026-08-30 ruling (batch #4, 「同意」, option A) directly adjudicated that both keys be declared, documented, and the cast removed — that layer is the maintainer's own control and this review does not re-enter it. But the same ruling itself orders the review chain (「 Reproduced (measured here, not taken on report)
Not re-run here: the 16 auxiliary doc/registration gates and Shape judgments (the clause-② substance)
Follow-up 1 — BLOCKING: "every body cell" is measurably false, in four shipped artifactsMeasured on head: schema-level The false statement ships in: the Required: correct the wording in all four places to state the real fold targets (structural cells: selection, row number, row actions — the density complement per-column classes cannot express), and fix the mdx example to carry per-column padding alongside (as Follow-up 2 — non-blocking: PR-body metric mislabel"Repo-wide Follow-up 3 — informational, no actionThe ablation table was measured on the pre-merge work commit (line 2287 vs 2315 post-merge). Its rows all reproduce on the merge tree; noting it so the next reader of the table isn't surprised by the line numbers. Both review worktrees removed; trees left clean. ⛔ Per tier rules this review does not mark ready or merge — the blocking follow-up goes back to the implementing seat. Generated by Claude Code |
os-sam
commented
Aug 30, 2026
PM: clause-② verdict received — ACCEPT WITH FOLLOW-UP. The blocking item is dispatched; two others recorded.
Routing: confirmed, and I was not the one who set itI asked the reviewer to settle whether clause ② was the right gate here or whether the maintainer's And the scope split came out exactly as the standing rule requires: the decide-to-declare layer ⛔ The blocking item is real, and it is a published falsehood
The PR documents it as applying to "every body cell", in four places: the ⇒ That is documentation telling authors a key does something it does not, plus an example whose prose ⛔ I told it to re-measure the cell list itself and stop if its measurement disagrees, rather than Two more, recorded rather than folded in
One note for landing, not for the author
⭐ Worth recording about the review itself: it caught and corrected its own instrument mid-flight Generated by Claude Code |
…assName The declaration shipped saying schema-level `cellClassName` applies to "every body cell". Re-measured on the render, it reaches three cells and no others: the selection-checkbox cell (`selectable`), the row-number cell (`showRowNumbers`) and the row-actions cell (`rowActions`). Data cells fold `TableColumn.cellClassName` and nothing else, so the two class slots style DISJOINT cells and never combine on one cell. The false claim shipped in four places; all four now say what is true: the `DataTableSchema` docblock, the zod `.describe()`, the "Cell styling" section of the data-table mdx, and the changeset. The mdx "compact rows" example is replaced because it demonstrated nothing: it set only the schema-level key, over `data: []`, on a table with no selection / row-number / row-actions column — so its classes reached zero cells and the table rendered its empty state. It now sets the density class on BOTH slots over real rows, which is what `ObjectGrid` does for its `rowHeight` modes. Rendered through the real renderer and measured in Chromium against real Tailwind output: row height 56px -> 28px, cell padding 16px -> 4px, font-size 16px -> 14px on every cell, data cells included. No type declaration, zod shape or renderer code changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
Follow-up on the CONTRACT_REVIEW_TIER finding — pushed as 1. Re-measured from scratch. The review's cell list is confirmed exactly.I did not take the cell list on trust. Two independent readings, both of Static. The schema key is destructured at line 727 (
There is no indirect route either: Rendered. A table with
So: selection, row-number and row-actions cells only. Never a data cell. That is the review's list, unchanged. Two nuances I measured and am reporting rather than acting on:
2. What was corrected, in all four places
3. Proof the new example worksThe old block could not demonstrate anything, measured both ways: as published ( The replacement sets the density class on both slots over real rows. Proof in two measured stages:
The rows genuinely compact, and the data cells compact with them — which is the thing the old example claimed and did not do. 4. Gates, all on the final commit |
| gate | exit | its verdict line |
|---|---|---|
check:doc-fences | 0 | "every TypeScript block in 224 document(s) is fenced ts/tsx/typescript, except 80 declared file(s) carrying 90 block(s) … SHRINK-ONLY" |
check:doc-types | 0 | "Every documented component type is registered." |
check:doc-snippets | 0 | "Semantic phase: 271 of 271 block(s) judged, 0 failed." / "Every covered documentation snippet compiles against the built types." |
docs:check-links | 0 | "Links are valid across 17 scan roots." |
check:control-bytes | 0 | "OK (scanned 5768 tracked text file(s); skipped 85 binary)." |
check:docs-route-closure | 0 | ran clean |
changeset:check | 0 | "All workspace packages are in the changeset fixed group." |
check-changeset-presence | 0 | "4 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)" |
pnpm --filter @object-ui/types type-check | 0 | tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json, silent |
turbo run lint (the full farm) | 0 | "Tasks: 47 successful, 47 total", 0 errors in every package |
vitest run on the 6882 pin and the plugin-grid seam pin | 0 | "Test Files 2 passed (2) / Tests 8 passed (8)" |
check:doc-snippets was run against a built closure (its own --build-filter, 32 turbo tasks), and its controls fired — @object-ui/types resolved to packages/types/dist/index.d.ts, the sentinel import produced TS2305 — so it is a real green, not a PREREQUISITE NOT MET. Program-input proof: tsc --noEmit --listFiles on packages/types lists 271 inputs including src/data-display.ts (line 149) and src/zod/data-display.zod.ts (line 261), so the type-check really covered the edited files.
5. The PR body
- The lint label. Counted independently, base
689ae3d13versus head4a9a4b37d, per file, witheslint --format json --stdin --stdin-filename(validated first against an on-disk lint of the same file: identical message-by-message). Result: total warnings 63 to 62,no-explicit-any52 to 51. The review is right, and the old line labelled the first figure as the second. The52is28 + 23 + 1— the per-file table in the body omitsdata-display.zod.ts's singleno-explicit-any. Body corrected. - I also corrected the one sentence in the body carrying the same false claim ("
cellClassNameby destructuring it into every body cell'sclassName"), since leaving it while fixing four other copies would ship the defect in the review material itself. Both edits are marked in place with the date and reason. - Note: the PATCH left the original session-URL footer in place and the platform appended a second, bare attribution footer. Cosmetic; a further PATCH would just append again.
6. Untouched, deliberately
No type declaration, no zod shape and no renderer code changed — this commit is .mdx, a docblock, a .describe() string and the changeset. The PR is still a draft, auto-merge is unarmed (auto_merge: null), no label was added or removed, and the branch was not updated against main.
7. One thing found, not fixed here
packages/plugin-grid/src/ObjectGrid.tsx (~line 778) carries the same false statement in its seam census — "data-table.tsx destructures it off the schema and folds it into every body cell's className" — and still records both keys as HELD pending the very ruling this PR executes. Pre-existing, in a package this diff does not touch, so it is left alone and flagged for the PM rather than ridden along.
Generated by Claude Code
Generated by Claude Code
✅ 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
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6882
Executes the maintainer ruling of 2026-08-30 (batch #4, verbatim 「同意」), option A: declare
renderCellEditorand schema-levelcellClassNameonDataTableSchema, document them, and drop the(schema as any)cast indata-table.tsx.Clause ②: this widens a published type face, so it is opened as a draft on the
CONTRACT_REVIEW_TIERreview chain. ⛔ Not mine to mark ready or merge.What lands
packages/types/src/data-display.tsDataTableSchemadeclaresrenderCellEditorandcellClassNamepackages/types/src/zod/data-display.zod.tspackages/components/src/renderers/complex/data-table.tsx(schema as any)cast becomesschema.renderCellEditorcontent/docs/components/complex/data-table.mdxpackages/types/src/__tests__/data-table-declared-keys-6882.test.ts.changeset/6882-...mdThe zod mirror is not a rider.
zod-mirror-parity.test.tsreconciles every declared-but-unmirrored key against two ledgers, and its header states that adding toUnmirroredDeclaredis not a supported route (shrink-only); the one exception routes callback-shaped keys toRuntimeOnlyDeclared, whichassertionRuntimeOnlyIsCallbackShapedOnlyrestricts toon+ uppercase spellings —renderCellEditoris not one. So mirroring is the only supported route, and it is the route #6639 took forObjectGridSchema.title. Declaring the keys without mirroring reddensassertionUnmirroredMatchesLedger; that firing was observed and is quoted below. Neither ledger is edited.The widening, stated exactly
Two keys land on
DataTableSchema:(The
=.above is an arrow; see the diff for the real bytes.)What an author can write after this change that they could not write before: nothing new runs. Both keys already worked, at any value at all, because
BaseSchemacarries an[key: string]: anyindex signature thatDataTableSchemainherits — every string was already a member.data-tablealready read both on the production path:renderCellEditorthrough the cast being removed here,cellClassNameby destructuring it into theclassNameof the table's three utility cells — the selection checkbox, the row number, the row actions. (Corrected 2026-08-30: this line, and the docs that shipped with it, said "every body cell". Re-measured on the render, schema-levelcellClassNamereaches those three cells and no others; every data cell foldsTableColumn.cellClassNameand nothing else. Commit4738f2727fixes the docblock, the zod describe, the mdx section and its example, and the changeset.) Nothing in the renderer changed; no value flows anywhere it did not flow yesterday.What changes is that the two keys are now checked at authoring time and offered by completion, and that the shape of
renderCellEditor's context is stated once, at its source, instead of being re-asserted locally by a cast that nothing verified.The declared shapes are transcribed from the consumer, not invented: they are byte-identical to what the cast asserted and to the seam hold
ObjectGridDataTableSchemaHoldsinplugin-grid, and the context members match the list PR #6912 independently wrote into the comment atinjectedEditorElRefwhile this branch was open ({ column, row, value, stage, commit, cancel }).The reject direction — it exists, and it was measured
Yes, there is one, and it is deliberate. Because the keys used to be absorbed as
any, author code with a wrong-shaped value also compiled and then silently did nothing. Such code now fails to compile. Measured, not reasoned: a probe file asserting both shapes was compiled against this branch and against the same tree with both declarations ablated.cellClassName: ['px-2', 'py-1']string[]is not assignable tostringrenderCellEditor: 'not-a-function'stringis not assignable to the context function typeBoth narrowings are the intended half of the ruling:
cellClassNameis declaredstring, matchingBaseSchema.classNameandTableColumn.cellClassName. The renderer folds it throughcn(), which would also swallow an array or an object — so the declaration is narrower than the read, on purpose. One authored spelling for a class slot is the contract (#0.1, contract-first).renderCellEditoris declared as the function the renderer actually calls. Its parameters stayanywhere the renderer passesany; narrowingcolumntoTableColumnwould be a reject-direction change the ruling did not authorise, and would break an author whose own handler declares a narrower context.No key was retired, no existing declared key changed type, and no accepted function shape narrowed: every value that ran before still runs.
The cast is gone, and nothing replaced it
grep -n 'schema as any' packages/components/src/renderers/complex/data-table.tsxreturns exactly one line on this branch — inside the replacement comment, which records why the cast existed. No second cast, noanyannotation, no@ts-expect-error, noeslint-disable. The lint delta below is the mechanical confirmation.Verification
Final commit
4a9a4b37d(a merge oforigin/main689ae3d13into the work commit; PR #6912 landed ondata-table.tsxmid-flight and merged without a textual conflict — its comment block is intact,NOTHING EVER HANDS THE WIDGET ONEand fourobjectui#6859references present).Red first, and the direction proved rather than asserted. The pin was written before the declarations and compiled against the tree without them:
BaseSchema's index signature makesDataTableSchema['anything']resolve toany, so any question asked of the raw type answers "declared" for every string. The pin strips the index signature first, so non-membership can exist at all.Expect of (X extends true ? true : false)is satisfied bynever(assignable to everything) and byany. The pin compares with an invariant function-identity equality instead.The direction is proved mechanically, by four
@ts-expect-errordirectives. TypeScript reports an unused@ts-expect-erroras TS2578, so each directive is a claim that the instrument really refuses something: the assertion helper must refusefalse; the equality must refuseneverandany; and the membership question must answerfalsefor a key nothing declares (which it can only do if the strip really happened). Break any part of the instrument — widen the helper, make the equalityextends-shaped, make the strip a no-op — and the file goes red on the now-unused directive instead of quietly passing. Both compilations above ran with all four directives satisfied.Ablation — predicted, then observed row by row. Each leg: mutate, prove the mutation on disk (anchored counts plus
git hash-object), rebuild@object-ui/typesand prove the mutation reacheddist/*.d.ts(which is what the components program reads — its--listFilesnamespackages/types/dist/data-display.d.ts, notsrc), measure, restore, prove the restore (git hash-objectequal to the HEAD blob andgit diff HEADempty).trap ... EXIT INT TERMwith absolute paths throughout.renderCellEditordeclarationrenderCellEditorrowsanythrough the index signature, it does not failcellClassNamedeclarationcellClassNamerows onlyrenderCellEditorrows untouchedcancel) from the declared contextdata-table.tsx(2287,37): error TS2353: Object literal may only specify known properties, and 'cancel' does not exist in type ...⭐ C is the answer to "does the declaration match what the code actually reads". With the cast gone, the call site is checked against the declaration; remove one context member and the renderer stops compiling, naming the member. Ablation A's green components leg is the same fact from the other side and is why the pin lives in
packages/typesand asks about declared membership, not about property access.Anti-vacuity of the parity gate: declaring the keys without mirroring them produced
zod-mirror-parity.test.ts(1219,14): error TS2322: Type '"data-display.zod.ts#DataTableSchema"' is not assignable to type 'never'— the gate naming the pair. Mirroring cleared it with no ledger edit.Program-input proof (a typecheck that excluded the files would read green and measure nothing).
--listFileson both projects:packages/types/tsconfig.test.json— 524 inputs, includingsrc/__tests__/data-table-declared-keys-6882.test.ts,src/data-display.ts,src/zod/data-display.zod.tsandsrc/__tests__/zod-mirror-parity.test.ts.packages/componentstsconfig.json— 1367 inputs, includingsrc/renderers/complex/data-table.tsxandpackages/types/dist/data-display.d.ts.Builds and typechecks (dependency closure built first — an unbuilt closure produces false TS2307 REDs):
turbo run build --filter='!@object-ui/site' --concurrency=2pnpm --filter @object-ui/types type-checktsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json)pnpm --filter @object-ui/components type-checktsc --noEmit && tsc -p tsconfig.test.json)pnpm --filter @object-ui/plugin-grid type-checkTests, from the repo root with path filters (the documented way;
pnpm --filter pkg testis this repo's zero-match false-green trap):pnpm exec vitest run packages/types/pnpm exec vitest run packages/components/pnpm exec vitest run packages/plugin-grid/ packages/plugin-dashboard/Lint — the full farm, not a narrowing.
pnpm lint(turbo run lint, 47 tasks): 47 successful, 47 total, exit 0, zero packages reporting a nonzero error count.Per-file base-versus-head, base blob identity asserted before the base content was used (
git rev-parse BASE:pathnon-empty and different from the HEAD blob; on-disk hash equal to the HEAD blob before mutating; restore proved by hash equality and an emptygit diff HEAD):data-table.tsxno-explicit-any28 -. 22data-display.tsno-explicit-any23 -. 28data-display.zod.tsThat accounting is exact and worth reading: the cast contained six
anys. Five of them were the context members, and they moved to the declaration verbatim — the same five, one package over. The sixth was(schema as any)itself, and it is simply gone. Across these three files: total warnings 63 to 62, andno-explicit-anyspecifically 52 to 51 — the same -1, but they are two different figures. (The per-file table above printsno-explicit-anyfordata-table.tsxanddata-display.ts;data-display.zod.tscarries 1 on both sides, which is what makes theno-explicit-anytotals 52 and 51.) Every other rule is unchanged, and errors are 0 on both sides. Corrected 2026-08-30 after the CONTRACT_REVIEW_TIER review: the earlier line labelled the total-warning delta as ano-explicit-anydelta.Other gates re-derived from the actual diff and run on the final commit:
check:doc-fences,check:doc-types,check:doc-snippets,docs:check-links,check:control-bytes,check:readme-exports,check:self-import,check:esm-specifiers,check:vi-mock-specifiers,check:vi-mock-inherit,check:shell-escape-residue,check:docs-route-closure,lint:coverage,type-check:coverage,check-changeset-presence,changeset:check— all exit 0.Not measured, on purpose
The ruling recorded a confidence gap before deciding: the in-repo readers were measured, the external authoring surface was not — nobody knows whether authors outside this repo already write these two keys. The maintainer ruled knowing that, and noted it cuts toward A. It is a recorded limitation of a decision already made, so this PR did not go measuring external consumers.
One thing found and not fixed here
scripts/__tests__/check-sdui-registration-pins.test.tsfails on any tree wherepackages/app-shell/distexists: that package'ssideEffectsarray lists both./dist/...ConnectAgentWidget.jsand./src/...ConnectAgentWidget.tsx, the dist spelling comes first, and the derivation records whichever it reads first. Probed by movingdistaside — the file then passes 11/11 — and restoring it. Unrelated to this diff, which touches no app-shell file and registers nothing. Already filed as #6893, so nothing new was filed.Generated by Claude Code
Generated by Claude Code