fix(fields): an empty array is not a cell value in the shared read renderers - #8495
Conversation
…nderers `SelectCellRenderer`, `LookupCellRenderer` and `UserCellRenderer` each open a multi-value container and map their entries into it, but their opening guards tested only null/undefined/empty-string. `[]` passed, the array branch mapped over zero entries, and the renderer's whole output was a childless container — a visually blank cell with no glyph and no accessible name. `@object-ui/plugin-detail` had already grown two private upstream pre-checks against this. Every consumer that does not pre-check reached the renderer directly; ObjectGrid (desktop and mobile card view), ObjectGallery and ObjectKanban were each verified by rendering. One private helper, `isEmptyMultiValue`, answers exactly one question for the three renderers that ask it. It is deliberately not exported and deliberately not a package-wide predicate: measured by rendering every registered field type against `[]`, this package holds at least seven private answers to "is this empty", and several disagreements are intentional (`JsonCellRenderer` draws the array literal, `FileCellRenderer` states a count). Both are pinned as the declared boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
✅ 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
|
|
PM contract review — accepted, flipped to ready, auto-merge armed. The census is what makes this PR worth more than its diff. The card as titled would have fixed one of threeRendering all 53 registered field types through ⇒ a one-renderer fix would have left a lookup column and a user column blank on exactly the surfaces this card exists to protect. That is my framing corrected, and it is the difference between closing a hole and moving it. The fence is measured rather than asserted: those three are precisely the renderers whose "Not a package predicate" is the measured answer to my open questionI asked whether the fix belonged in one renderer or in a shared check the The discrimination question, answered explicitly rather than implied
⇒ the most quotable evidence in the PR cannot tell the fix from its worst caricature — objectui#8474's lesson reproduced independently one package over. My brief predicted the populated multiselect would be the axis that refuses it; you confirmed and sharpened it to four populated axes plus the one-entry-array case plus the ⭐ And the part I want other seats to copy: two cases discriminate against nothing and say so. Instrument calibration is the other reusable bit: the grid pin's first draft asserted "no childless flex-wrap anywhere in the grid" and failed against the correct implementation, because ObjectGrid's toolbar renders a legitimately empty one. Scoping every lookup to one row, with the reason written into the file, is the fix — and the near-miss is exactly the kind that ships as a mysterious flake when it is not recorded. Three corrections to my brief, all accepted
Rendering each consumer twice (HEAD vs ablation A) rather than inferring from source is what turned "ObjectKanban changes" into "ObjectKanban changes only 2 of 3" — the picklist branch never reaches a cell renderer at all. That distinction only exists because you rendered it. The shared-predicate question goes to the maintainerYour recommendation is B (the weakest common claim, below every consumer) as its own card, and the constraint you measured is what makes it more than a preference: the predicate can only ever be the weakest common claim, because the disagreements above it are deliberate — the detail page hides Filing it with your costing, marked for the maintainer — where a cross-package predicate lives is a placement decision, not a PM one. objectui#8489, objectui#8490 and objectui#8491 are all labelled and queued. Generated by Claude Code |
1 participant
Fixes #8481
Cut from
mainat7cf6f38fb. Final commitc1337dfbb.DOM shapes are spelled out in words rather than written literally throughout — tag-shaped fragments are eaten from GitHub bodies (AGENTS.md, "六种已实测的改写" ①). Card references other than the one line above deliberately carry no closing keyword.
Verdict: the CHILDLESS-CONTAINER class — three renderers, one private helper, not a package predicate
The card names
SelectCellRenderer. Censused by rendering, it is one of three:[]on7cf6f38fbselect,status,multiselect,radio,checkboxes,tagsSelectCellRendererflex flex-wrap gap-1, no childrenlookup,master_detail,treeLookupCellRendererflex flex-wrap gap-1, no childrenuserUserCellRendererflex -space-x-2, no childrenAll three open a multi-value container and map their entries into it. That is the fence, and it is measured rather than asserted: those are exactly the renderers in this file whose
Array.isArraybranch returns a container of children.FileCellRendereralso has an array branch but returns the text0 files— a statement, not a blank — andImageCellRenderer's array branch already testsimgs.length === 0.One private helper,
isEmptyMultiValue, answers one question at those three opening guards. ⛔ Not exported, and deliberately not the package's general emptiness predicate — see the census below for why.The census: this package holds SEVEN private answers to "is this empty"
Measured by rendering every one of the 53 registered field types through
getCellRendereragainst[],{}and['alpha','beta']— 159 rendered outputs, before and after.value == null || value === ''coerceToSafeValue(value) == null || === ''value == null!value!value || imgs.length === 0Array.isArray(v) ? v.length : 0repeaterregistry entryObject.keys(v).length === 0AddressCellRenderer's second clauseThe brief asked whether the fix belongs in one renderer or in a shared check the package's renderers "ought to agree on". Measured answer: they must not all agree, and several disagreements are deliberate.
JsonCellRendererdraws the two-character array literal on purpose (objectui#8474 measured that and kept it);FileCellRendererstates a count;BooleanCellRenderermust keepfalsea value whileDateCellRenderer's!valuedeliberately treats the epoch as empty. A single predicate over all of them would flip settled decisions. ⇒ A shared predicate is reported, not built — see "What a shared predicate would have to look like" below.Blast radius: 10 of 159 rendered outputs moved, and nothing else
Re-running the same census after the change, diffed row by row:
[]block, all from a childless container to the sharedNo valueaffordance:select,status,multiselect,radio,checkboxes,tags,lookup,master_detail,tree,user.{}block.The consumers, verified by RENDERING
Each row is a real component rendered twice against the same fixture: once at HEAD, once with ablation A applied (the permissive guards restored verbatim, proved on disk and restored by state). Neither leg is inferred from the source.
ObjectGrid, desktop tablegetCellRenderer(...)and the raw value[]row painted a childless flex-wrap cellNo valueaffordance in that row, zero in the populated rowObjectGrid, card layout below 768pxObjectGalleryObjectGallery.tsx:571,if (value == null || value === '') return null;— with the SAME holeflex flex-wrap gap-1,flex -space-x-2), 0 affordancesObjectKanbanObjectKanban.tsx:604,if (raw == null || raw === '') continue;— same holeIn every leg the populated card still drew its
Alphabadge, so no leg is measuring an all-empty renderer.optionstakes the board's own picklist-badge branch and never reaches a cell renderer at all. For[]that branch resolves an empty label and pushes a badge anyway, so the card renders a fully coloured pill with nothing in it. Measured on a real board:topics(multiselect, no options) andowners(user) now show the affordance;tags(multiselect with options) still shows an empty indigo pill — it is the one childless container left in the AFTER column above, and its class string is the badge pill's, not a renderer's. Filed as objectui#8489 with the rendered evidence — deliberately not addressed here, because it isplugin-kanban's own code and a different decision.Stated explicitly, because the brief's pin says the vivid assertion is rarely the discriminating one — and it is right again here.
Ablation B makes
isEmptyMultiValuereturntruefor every value (strictly worse than the bug). Under it:THE DEFECT — TYPE holding [] renders the No-value affordancecases, and the desktop-grid case. The most quotable evidence in this PR — ten field types across the shared renderer now say "No value" — cannot tell the fix from its worst caricature.multiselectbadges,lookupchips,useravatars, a scalarselectvalue), the one-entry-array axis,THE BOUNDARY — {} is untouched, the grid's⚠️ DISCRIMINATINGpopulated row, and the mobile case (which carries a populated assertion of its own).⇒ The brief predicted "a populated multiselect rendering its badges is the likely axis". Confirmed, and sharpened: it is four populated axes plus the
{}boundary, and the single case red under all three ablations is the mobile one, because it is the only case that asserts both halves in one render.Two cases discriminate against nothing and are honest about it:
THE BOUNDARY — json still draws the literalandTHE BOUNDARY — file still states its countstay green under A, B and C. They are scope declarations, not instruments — they exist so a later widening has to delete an assertion rather than merely forget a consideration.Ablations — mutation on disk proved both ways, restore proved BY STATE
Every mutation is on the READ SITE, from the committed implementation, under
trap … EXIT INT TERMwith absolute paths. Each proves it reached disk by hash and by removed/injectedgrep -ccounts in both directions with the matched line printed; each restore is verified bygit hash-objectequal togit rev-parse HEAD:PATHandgit diff HEADempty — never by an exit code.vitest.config.mts:432maps@object-ui/fieldsto./packages/fields/src, so the pins resolve the source these mutations edit. Ablation A reddening 12 rows is itself the proof that the tests read the mutated bytes rather than a staledist.HEAD blob for
packages/fields/src/index.tsx=b37e2c62150f6da348143abce3bd518085d62560.0201d431761468c70bac6ea6e4a501e4150d8a88isEmptyMultiValueanswers EMPTY for everythinga8cfa76e38247f951742e7c7f78b8c3fd7ec0023Array.isArray(value)alone, so every array is EMPTY64711bb4f8246f5df6dc1127fc5c6bb41c866d53A reddens all ten
THE DEFECTrows plus the grid'sDESKTOPandMOBILE CARD VIEW.B reddens
NON-REGRESSION — a POPULATED multiselect…,…a POPULATED lookup…,…a POPULATED user field…,…a SCALAR select value…,…a ONE-entry array…,THE BOUNDARY — {} is untouched…,⚠️ DISCRIMINATING — the POPULATED row…andMOBILE CARD VIEW….C reddens the same set minus
…a SCALAR select value…(a scalar is not an array) and minusTHE BOUNDARY — {}…(an object is not an array) — the two cases that separate "every array is empty" from "every value is empty".An instrument that had to be calibrated, recorded because it read backwards
The grid pin's first draft asserted "no childless flex-wrap element anywhere in the grid". It failed against the correct implementation:
ObjectGrid's toolbar renders adiv.flex.flex-wrapthat is legitimately empty when no filter chips are active. An unscoped structural assertion was reading the chrome. Every lookup in that file is now scoped to one ROW, and the reason is written into the file so the next reader does not un-scope it.What a shared predicate would have to look like (reported, not built)
⛔
plugin-detail'shasCellValuewas not imported —@object-ui/fieldsis upstream of it, and objectui#8459 measured that the two surfaces want different answers. Reporting the shape instead:The five surfaces that have now each independently answered "is this empty" are
hasCellValue(plugin-detail),RelatedList.isValueEmpty(plugin-detail),ObjectGallery's inline guard,ObjectKanban's inline guard, and this package's seven renderer guards. A predicate they could share would have to live below all of them —@object-ui/typesor@object-ui/core— and it could only ever be the weakest common claim (null,'',[]), because every consumer legitimately extends it: the detail page also hides{}-shaped nothings,JsonCellRendererdeliberately does not. That is a real change with a real design decision in it, and it is bigger than this card.Tests
Run from the repo root, paths relative to the root, nothing after
--.pnpm exec vitest run packages/fields/→ 139 files / 2370 tests passed, exit 0 (the changed package, in full)pnpm exec vitest run packages/plugin-grid/ packages/plugin-detail/ packages/plugin-list/ packages/plugin-kanban/ packages/plugin-gantt/src/ObjectGantt.persistfilters.test.tsx→ 358 files / 3388 tests passed, exit 0pnpm exec vitest run packages/fields/src/__tests__/cellRenderers.emptyArray-8481.test.tsx→ 18/18pnpm exec vitest run packages/plugin-grid/src/__tests__/emptyArrayCell-8481.test.tsx→ 3/3; 21/21 together, and 21/21 again after each ablation leg restored the treepnpm --workspace-concurrency=2 --filter '@object-ui/fields^...' build→ exit 0, and later--filter '@object-ui/plugin-grid^...' build→ exit 0pnpm --filter @object-ui/fields run type-check→ exit 0 (tsc --noEmit && tsc -p tsconfig.test.json, script name echoed)pnpm --filter @object-ui/plugin-grid run type-check→ exit 0pnpm --filter @object-ui/fields run lint→ exit 0,✖ 933 problems (0 errors, 933 warnings); the new test file adds 2, both theas anyfixture cast every sibling test in that directory already usespnpm --filter @object-ui/plugin-grid run lint→ exit 0,✖ 781 problems (0 errors, 781 warnings); the new test file adds 3, same castpnpm run check:control-bytes→ exit 0,scanned 6716 tracked text file(s); plus a directgrep -naPscan of the four changed paths → 0 hits, with a lit control that fired: a real U+0001 written into a scratch file matched the same class (a TAB would not have — that character is deliberately outside it)pnpm run check:unreferenced-sources,check:self-import,check:vi-mock-specifiers,check:side-effects-array,check:esm-specifiers,check:handler-key-reads→ all exit 0packages/fields/dist/index.d.tsafter a real build:isEmptyMultiValue→ 0 hits, control symbolSelectCellRenderer→ 1 hit. The helper is private by measurement, not by intent alone.check:readme-exports,check:sdui-registration-pinsandcheck:eager-closureexited non-zero withPREREQUISITE NOT MET-shaped output — each printsrun pnpm build first/pnpm --filter @object-ui/console buildbecause it reads builtdistartifacts, and this worktree has only the two dependency closures above built. Reported as unmeasured rather than as failures; CI builds the workspace and grades them properly. My firstplugin-gridtype-check run failed the same way (sixTS2307 Cannot find modulelines) and went green once its closure was built — recorded because that exact shape is what a real breakage would also look like.Declared narrowing.
turbo ls --affectedagainst7cf6f38fbnames 20 packages. I ran the changed package plus the four consumer packages that resolve cell renderers, and the narrowing is measured rather than guessed: grepping every test file in the workspace for an empty-array value on a multi-value field found exactly five candidates outsidepackages/fields— two inplugin-detail, two inplugin-grid, one inplugin-gantt— and all five are inside the runs above (the gantt one by explicit path). The rest of the 20 is left to CI.Changeset
.changeset/8481-empty-array-is-not-a-cell-value.md,@object-ui/fields: minor. This moves a shared renderer's rendered output for ten field types on every surface that does not pre-check, so it is user-visible;minorfollows this repo's single-version-group convention and the landed precedent for a render change (objectui#8457, and objectui#8474 / objectui#8459 for the same defect family). ⛔majoris forbidden here (one fixed version group), andskip-changesetis a phantom label in this repo — no workflow or script reads it — so it was not applied.The gate's own verdict line:
node scripts/check-changeset-no-major.mjs→✅ No changeset declares a "major" bump.node scripts/check-governed-queue-guard.mjs --test THE-FOUR-CHANGED-PATHS(passed explicitly, never bare) →✅ NOT GOVERNED — 4 path(s) checked against 5 governed surface(s); none matched.Out-of-scope findings, filed unassigned
The
[]census produced three defects outside this card's fence. All three were dedup-searched first; REST is 403 from this dev container, so one targeted MCPsearch_issueswas used and is declared as a channel switch — it returned objectui#8481, objectui#8474 and objectui#8475 as live control hits.ObjectKanban's own picklist branch never reaches a cell renderer, so a picklist field holding[]still paints an empty coloured badge pill. The common case on a kanban board, and untouched by this PR.@object-ui/fieldsrenderers fabricate a value for[]rather than draw blank: a checked checkbox forboolean, the digit0fornumber/currency/percent, livemailto:/tel:/ empty-href anchors with no text, and a colourless swatch. Different defect class from this card (wrong value, not no value) and a contested per-renderer ruling, not a mechanical fix.ObjectGridspells the empty placeholder by hand at three sites (lines 2479, 2497, 2697) instead of usingEmptyValue, so those cells carry noaria-labeland render one type size smaller and italic. Same class as objectui#8475; a repo-wide grep says those two files are the only carriers, so the class can be retired completely.Notes for review — what contradicts the brief
ObjectGalleryandObjectKanbanhave "no emptiness predicate mentioned at all". Measured: each has one, and each carries the identical hole (ObjectGallery.tsx:571,ObjectKanban.tsx:604). They are not innocent pass-throughs — they are the fourth and fifth private re-answers of the same question. That strengthens the card's thesis rather than weakening it, but it changes what "the third consumer" means: there were already five.LookupCellRendererandUserCellRenderercarry the byte-identical defect in the same file, both reached byObjectGrid,ObjectGallery,ObjectKanbanandObjectDataTable. Repairing onlySelectCellRendererwould have left alookupcolumn and ausercolumn blank on the very surfaces this card exists to protect.7cf6f38fb, which is that merge commit — soRelatedList.emptinessAgreement-8459.test.tsxis on this base and ran green in theplugin-detailsweep above rather than being unavailable to lean on.packages/specis not involved: this is a renderer-side emptiness decision, not a metadata contract, so contract-first (AGENTS.md #0.1) does not apply. Nothing here adds a lenient fallback for off-spec input —[]is a perfectly spec-valid stored value for a multi-value field.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code