Repository navigation
fix(components,core,plugin-dashboard,plugin-detail): data-table reads the declared header; the producers translate label (#5351) - #5478
Merged
Conversation
… only its declared column keys (#5120, #5351) The adapter normalized `header: col.header || col.label` and `accessorKey: col.accessorKey || col.name`. `TableColumn` declares `header` and `accessorKey` and declares neither alias, so one key had two spellings — one the type admits, one only the runtime did (AGENTS.md #0.1). Both aliases retire. The translation moves to the producers, which is where the two vocabularies actually meet: `columnIdentity` already resolved the identity half in RelatedList (#5022), ObjectDataTable (#5120/PR5353) and ObjectGrid (#5068); a new `columnHeader` in `@object-ui/core` does the same for the display half, adapter-first so an author-supplied `header` is never overwritten. ObjectDataTable gains a fix along the way: `enrich` spreads `buildFieldMeta`'s own `label` over every column, so an authored `label` was overwritten before it ever reached the alias — a `{ field, label }` column rendered a blank header there even while the alias existed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
…; the `label` alias retires (#5351) The `col.name` retirement is measured and ready but cannot land yet: `skill-guide-data-table-binding.test.tsx` renders the JSON blocks lifted from skills/objectui/guides/{data-integration,schema-expressions}.md, and both teach a directly authored `data-table` with `{ name, label }` columns. The runtime must not refuse a spelling the shipped instruction set still teaches, and `skills/**` is another seat's surface. Measured split: with `col.label` retired and `col.name` held, that gate is 15/15 green — the label half is invisible to it. Only the name half breaks it, on 2 tests. #5351 therefore lands in full and #5120 keeps its remaining step. The held alias is pinned as still-read, so the day the guides move that pin goes red and names itself as the thing to delete. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
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 21, 2026 02:12
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 1, 2026
…ire the col.name alias) (objectstack-ai#6963) * fix(components)!: data-table reads only the declared accessorKey Retire the undeclared `col.name` tolerance on the data-table adapter, and migrate the instruction corpus that taught it in the same commit. `data-table`'s column normalization read `accessorKey: col.accessorKey || col.name` at two sites, but `TableColumn` (`@object-ui/types`) declares only `accessorKey`. The declared surface admitted one spelling while the runtime admitted two — the second de-facto contract AGENTS.md #0.1 forbids. The maintainer ruling of 2026-08-20 settled the direction for the family: retire the consumer-side alias, translate at the producers. `label` retired first; this closes the family. The two published skill guides that taught `{ name, label }` columns migrate to the declared `{ header, accessorKey }` here, not in a follow-up. They are executable fixtures — `skill-guide-data-table-binding.test.tsx` lifts their fenced JSON at run time and renders it — so moving the runtime without the instruction would leave the platform refusing a spelling it still ships. Blast radius is adapter-only: columns arriving through ObjectDataTable, RelatedList or ObjectGrid are unaffected, because each producer folds `name` into `accessorKey` via `columnIdentity` before delivery. What narrows is a column authored directly onto a `data-table` node. Tests follow the change rather than being retrofitted to it: - the HOLD pin in `data-table-declared-column-keys.test.tsx` becomes a retirement pin, plus a legibility pin (live header over blank cells, neighbour untouched) and a producer-resolved pin bounding the radius; - the guide gate's docblock claimed it "stays true whichever way objectstack-ai#5120 lands — nothing below asserts a column key spelling". That was false (objectui#5479): its assertions are on cell text, so they transitively pin the accessor spelling. Corrected, and an explicit spelling assertion added so the failure names the key and the card instead of diffing cell arrays; - `skill-guide-provider-envelope.test.tsx` carried a hardcoded `{ name, label }` COLUMNS fixture mirroring the guides; migrated with them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB * docs(components): ObjectGrid refuses the legacy spelling, it does not fold it Contract review on PR objectstack-ai#6963 caught a mechanism mis-attribution. The blast-radius CONCLUSION was verified correct — columns arriving through ObjectDataTable, RelatedList or ObjectGrid are all unaffected by the `col.name` retirement — but the stated REASON held for only two of the three. - ObjectDataTable and RelatedList genuinely resolve the legacy spelling, stamping `accessorKey` from `columnIdentity(col)` before delivery. - ObjectGrid does NOT fold. Since objectstack-ai#5068 it refuses undeclared spellings at intake: `resolvesToDataColumn` requires a non-empty string `field`, so a name-spelled entry is dropped before delivery and never reaches the adapter at all — before and after this PR. What it delivers carries `accessorKey` stamped from `field`. This matters beyond pedantry: the 2026-08-20 ruling's item 2 told ObjectGrid to "connect to the same columnIdentity resolution", and objectstack-ai#5478 measured that following it literally would have re-widened exactly what objectstack-ai#5068 narrowed. The claim has now been restated in the wrong direction by two separate seats. A third repetition is how it becomes settled fact, so it is corrected in the durable artifacts rather than only in a review thread. Corrected in the changeset (author-facing, and the one that would have shipped the false mechanism into release notes), and in the two test comments that carried it. Comment and changeset text only — no assertion, no source line, and no behaviour changes; the three affected pin files stay 44/44 green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Fixes #5351
Part of #5120
Deliberately
Part offor the second card: one of its two halves is held, and merging must not close it. The census-first fork fired again, on evidence that did not exist when the ruling was recorded. Details in "What is held, and why" below.The ruling this implements
What landed
data-tableno longer reads the undeclaredcol.label.TableColumndeclaresheader: stringand does not declarelabel, so the adapter accepted a spelling its own type refuses — AGENTS.md #0.1, one key with two spellings. The translation moved to the producers, which is where the two vocabularies actually meet.@object-ui/coregainscolumnHeader()next tocolumnIdentity(). It is adapter-first —headerwins overlabel— which is the opposite order fromcolumnIdentity, on purpose:columnIdentityfolds several metadata spellings of one metadata concept so the canonical metadata key wins, whilecolumnHeadercrosses between vocabularies, and an author who wrote the adapter's own key addressed the table directly. That is the same ruleaccessorKeyalready had.Producers, per the ruling's list of three:
normalizeColumnsnow stampsheaderas well asaccessorKey.normalizeColumngains the same stamp. See the scope note below; this file was outside the dispatch's stated surface.columnIdentitywould have been the wrong direction. finding(plugin-grid): ObjectGrid 在ObjectGridSchema.columns上容忍未声明的accessorKey/header拼写 —— 声明类型是 strict 的 ListColumn,#3104 的列身份门禁结构上看不见这条支路 #5068 made it read the declaredfieldand onlyfield, refusingname; routing it throughcolumnIdentity(which foldsnameandfieldName) would have re-widened the accepted set the ruling is narrowing. It already resolveslabelintoheaderitself atObjectGrid.tsx:1610. Measured identical before and after — see the matrix.Census, with its counter-probe
Corpora:
examples/(463),content/docs/(202),apps/(203),skills/(29),e2e/(30),packages/*/README.md(39) — 966 text files. Method: bracket-matched extraction of everycolumns: [ ... ], split into top-level entries, each classified by the enclosing node'stype.Counter-probe, through the same method: within the
data-tablebucket the control termsaccessorKeyandheaderreturn 31 entries each. The search works, so the smallname/labelcount is a reading and not a broken scan.typenamelabeldata-table?data-tableobject-data-tabletable(static renderer)object-gridlabelitselfgrid(form field)nameis its declared keycrudkanban,lookup,list,object-master-detail-form,exportExcelWithFormulasTwo hits needed individual adjudication rather than a bucket:
content/docs/api/schema-reference.md:1128— adetail-viewrelated[]entry oftype: 'table', which does reachdata-tablethrough RelatedList. Its columns are{ name, label }. This is the site that made the RelatedList producer change non-optional.content/docs/core/report-schema.mdx:327—type: 'table'inside a report;ReportViewerrenders its own HTML table element withcol.label || col.name, its own vocabulary. Not this adapter.Nothing was migrated.
content/docs/api/schema-reference.mdis #5352's this round and was not touched.Per-shape matrix, measured before and after
Rendered through the real renderers; headers and body cells read off the DOM.
Directly authored
data-tablenode (no producer) — the only surface where behaviour changes:{accessorKey}""→""Won,Lost→Won,Lost{name}""→""Won,Lost→Won,Lost(alias held){header,accessorKey}Stage→StageWon,Lost→Won,Lost{label,accessorKey}Stage→""Won,Lost→Won,Lost{field,label}Stage→"""",""→"",""(cells were already blank){name,label}Stage→""Won,Lost→Won,LostObjectDataTable — three shapes get better, none get worse:
{label,accessorKey}""→StageWon,Lostunchanged{field,label}""→StageWon,Lostunchanged{name,label}""→StageWon,LostunchangedThat blank header was a real defect and not merely the alias being bypassed:
enrichspreadsbuildFieldMeta's result over every column and that result carries its ownlabelbuilt fromcol.header, so an authoredlabelwas overwritten withundefinedbefore it ever reached the adapter's alias.ObjectGrid — all six shapes byte-identical before and after. RelatedList — all six shapes byte-identical before and after: the producer translation replaces exactly what the alias did, with no user-visible delta.
Reverse verification, each half separately
No build artifact sits between any edit and the thing under test: the root
vitest.config.mts(L254-290) aliases every@object-ui/*specifier to that package'ssrc/. Checked in the config, not assumed.Leg 1 — the
namehalf ablated (alias restored at both sites).Predicted red, by name:
does not resolve an accessor from name,LEGIBILITY: an unresolvable accessor keeps its header and its neighbour,sizes from the declared keys.Observed: exactly those 3 red, 20 green. Prediction matched.
Leg 2 — the
labelhalf ablated (alias restored at both sites).Predicted red:
does not resolve a header from label,LEGIBILITY: a headerless column still renders its cells and its neighbour. Predicted green: every producer test, because the producers now stampheaderthemselves.Observed: exactly those 2 red, 21 green — including all 15 producer tests. The producer half is alias-independent.
Leg 3 — producers reverted to
origin/main, adapter left retired.Predicted red: 2 in the new RelatedList file, 4 in the new ObjectDataTable file, plus the pre-existing
RelatedList.columnIdentityAccessor.test.tsx.Observed: those 6 exactly, and the pre-existing file went red on 4 tests where I predicted 2 — I under-predicted its blast radius; it also pins a mixed-spelling column set and a header-sort ordering. Reported as observed rather than trimmed to fit.
Leg 3 is the load-bearing one. #5120's
namehalf was measured invisible to CI (PR5353: 2122 tests green with both alias sites cut). #5351'slabelhalf is the opposite — the existing suite catches it. Retiringcol.labelwithout the RelatedList producer change would have gone red on four pre-existing tests, which is why that file could not be left alone.Tree restored to the committed state after every leg and verified clean.
What is held, and why
accessorKey: col.accessorKey || col.namestill stands. It is ready, measured and reverted only for sequencing.packages/components/src/__tests__/skill-guide-data-table-binding.test.tsxlifts the fenced JSON blocks out of the real guide files at run time and renders them. Two published guides teach a directly authoreddata-tablewhose columns are spelled{ name, label }:skills/objectui/guides/data-integration.md:212skills/objectui/guides/schema-expressions.md:410Retiring
nameturns that gate red on 2 tests. Measured split: withcol.labelretired andcol.nameheld, that gate is 15/15 green — the label half is invisible to it; only the name half breaks it. So the two cards separate cleanly at exactly this line.I did not migrate the guides.
skills/**is a customer-published instruction corpus with its own CI gate and its own owning seat, my dispatch names it in a do-not-migrate clause, and two sibling cards are editing that corpus this round under their own claims. The remaining step is small and precise — 4 column entries in 2 files — but it is not mine to take unbidden, and the dev report on #5120 recommended it as a docs-only change landing before the runtime retirement, which the maintainer accepted.The held alias is pinned as still-read, so the day the guides move that pin goes red and names itself as the thing to delete, rather than the retirement quietly never happening.
Scope note: a file outside the dispatch's stated surface
The dispatch fenced the surface to
data-table.tsx,ObjectDataTable.tsx,ObjectGrid.tsxand the shared resolution.packages/plugin-detail/src/RelatedList.tsxis edited here anyway, and that is a breach I am flagging rather than burying.The ruling names three producers and requires each to translate at delivery; RelatedList is one of them, and the ruling's note that it "already does" is true of identity only, not of the label half. Leg 3 measured the consequence: without this 14-line change the PR is a regression that four pre-existing tests catch. The dispatch also asked for the before/after matrix on all three producers, which puts RelatedList in the measured set. No other worktree holds
packages/plugin-detail; checked before editing.Tests
All from the repo root (a package-scoped run uses a different config than CI), at final
HEAD2370882 — the same tree this body describes.pnpm exec vitest run packages/core packages/components packages/plugin-dashboard packages/plugin-detail packages/plugin-grid --maxWorkers=2→ 507 files, 5779 tests, all passedtype-checkon all four touched packages → clean, after building the dependency closure first (the^...dependency-closure filter); all four scripts echoed by name, guarding the zero-match traplinton all four → 0 errors (513 / 898 / 342 / 809 pre-existing warnings)check:control-bytes(4524 files),check:phantom-deps,check:self-import,check:esm-specifiers,check:published-dist→ all OK.check:skills-paths,check:doc-types,check:doc-snippetsare not triggered: noskills/**orcontent/docs/**file is in this diff.No test is skipped, disabled or quarantined.
Generated by Claude Code