fix(plugin-detail): an empty array is not a cell value on the record page - #8482
fix(plugin-detail): an empty array is not a cell value on the record page#8482os-justin wants to merge 1 commit into
Conversation
`hasCellValue` — THE definition of emptiness for the record page — opened with
`if (value !== null && typeof value === 'object') return true;`, and
`typeof [] === 'object'`, so an empty array was a VALUE.
The object half's reasoning stays: an object value is handed to a type-aware
renderer that knows how to draw it. But every example it reasons about is a
POPULATED object, and for `[]` the renderer has nothing to draw —
`SelectCellRenderer` tests `value == null || value === ''`, which `[]` passes,
then maps it over zero entries. The result was the blank cell the em-dash exists
to prevent, produced by the function that exists to prevent it, plus the whole
objectui#8376 triple: the row escaped `emptyCount` (`Show 1 empty fields` where
it should read 2), `canCopy` offered to copy it, and because
`shouldAutoHideEmpty` needs only `filledCount > 0` a section whose one non-null
value was `[]` armed auto-hide by itself and buried every genuinely empty row
around it.
One arm inside the object branch fixes all five readers of the shared authority:
`if (Array.isArray(value) && value.length === 0) return false;`.
`{}` deliberately does NOT move — measured, not assumed. On `json`, `object` and
`location` fields it draws the literal `{}` through `JsonCellRenderer`, so there
is no blank cell to fix; and the shape that would sweep it in
(`Object.keys(value).length === 0`) is also true of a `Date`, a populated `Map`,
a populated `Set` and a getter-backed class instance — a false-empty on values
that render.
Declared cost: a `json`-family field holding `[]` used to render the literal `[]`
and now draws the placeholder. Intended, and pinned.
`RelatedList.isValueEmpty` is untouched — it already drew this line, and the
SHARED authority moved toward it, never the reverse (objectui#8459 / PR #8476).
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. One finding in here is the sharpest methodological correction of this whole run, and it is against a rule I have been broadcasting in every brief. ⭐ The amplification fixture is necessary and NOT sufficientI told you a fixture showing one blank row misses the amplification, and to build the one that drives auto-hide. That was right. What I did not know, and your ablation B measured:
⇒ the most impressive-looking assertion in the file is precisely the one that cannot tell the fix from its worst caricature. What refuses B are the two non-regression axes (a populated array and a populated object; I have been citing "build the amplification fixture" as if it were the strong assertion. It is the vivid one, not the discriminating one. Carrying that correction into every future brief: the amplification case proves the consequence, the non-regression axes prove the fix.
|
2 participants
Fixes #8474
Cut from
mainatc90395b20. Final commita0ccd5f1c.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 NARROW array arm.
{}was measured and deliberately did not moveOne clause, inside the object branch of
hasCellValue(packages/plugin-detail/src/emptiness.ts):The object half's reasoning stays intact and is now stated as what it always was: it is about populated objects.
typeof [] === 'object'was the one case it swept in without meaning to.Where the predicate lives on this base
packages/plugin-detail/src/emptiness.ts. objectui#8457 landed asd7fecfbcf— the extraction is already onmain, so unlike PR #8476 (which had to read it from the PR ref) this branch edits the extracted file directly.DetailSection.tsxnow only imports it.The defect, measured in real DOM
Rendering a real
DetailSectionatc90395b20, desktop width pinned to 1280:tags: []on amultiselectfield"…industryManufacturingtags"— zero em-dashes, zeroNo valueaffordances; the cell is a childless flex-wrap DIV"…tags—", one affordancecanCopyon that rowemptyCount(4 fields,tags: []+ one absent)Show 1 empty fields, the[]row visible and uncountedShow 2 empty fields, hidden with the rest, revealed by the toggletags: []non-null"Detailstags"+Show 3 empty fields— one label over a blank, three genuinely empty rows buriedThe amplification row is the one a single-blank-row fixture misses:
shouldAutoHideEmptyneeds onlyfilledCount > 0, so[]armed auto-hide by itself. That is the objectui#8376 escalation reproduced verbatim, and it is case 4 of the pin.{}— measured, not assumed, and it is a VALUEThe brief said to measure rather than reason by symmetry. Measured, on the DOM:
{}jsonJsonCellRenderer{}object{}locationLocationCellRendererfalls back toJsonCellRendererwhen the lat/lng chain yields nothingSo
{}draws. Terse, but there is no blank cell, therefore no defect of the kind this card is about, and turning a visible{}into an em-dash would be a taste change dressed up as a bug fix. It did not move.⛔ And the shape that would have swept it in is separately unsafe.
Object.keys(value).length === 0is also true of:Object.keys().lengthnew Date(0)Mapwith one entrySetwith one entryAll four measured. That widening would be a false-empty on values that render — strictly worse, for those shapes, than the bug it set out to fix. Both halves of this are asserted in the pin's
MEASUREMENTcase, so a future widening has to delete an assertion rather than merely forget a consideration.The three consumers, before and after
hasCellValueis not exported outside@object-ui/plugin-detail(verified by grep across the workspace), so the blast radius is exactly its five documented readers. The three the card names:[]DetailSectionaffordance (displayValue'sisEmptybranch)SelectCellRenderer, which draws a childless flex-wrap DIVaria-labelandtitleofNo valueDetailSectionemptyCount/ auto-hide[]counted as FILLED: the toggle reads one too low, and one[]can arm auto-hide alone[]keeps its all-empty skeletonDetailSectioncanCopy[]The other two readers move the same way and for free:
HeaderHighlight's strip draws the em-dash one band higher,DetailView's summary chips stop spending the status slot on a valueless field,HistoryTimelineshows the placeholder for an empty-array audit value,RecordMetaFooterstops treating[]as an actor.Declared cost, stated rather than discovered
A
json-family field holding[]used to render the literal two-character text throughJsonCellRendererand now draws the placeholder. Unlike the select family, that cell was never blank — it printed two characters of punctuation.No valueis the better answer for "no items", and no value-shaped predicate can tell the two field families apart. Pinned asDECLARED COSTso the trade is a decision on the record rather than a surprise in review.Would an implementation strictly worse than the bug pass? No — measured three ways
Each mutation is on the READ SITE, from the committed implementation, proved on disk by hash and by removed/injected grep counts in both directions with the matched line printed; each restore verified by state (
git hash-objectequal togit rev-parse HEAD:PATH,git diff HEADempty), never by an exit code; all undertrap … EXIT INT TERMwith absolute paths.HEAD blob for
packages/plugin-detail/src/emptiness.ts=91e9fdde8ae7672e40e9ceef51bd3a007dc0bddf.f119dc261ecd97b279e349051b1513300066acfahasCellValuereturnsfalsefor everything (strictly worse than the bug)b5230f00db5c68029dec3be447c4ad1ecf6212faArray.isArray(value)alone, so every array is EMPTY (the over-correction)7dbf02ed52b73b8a00fb1d396d102eb0ee305fa8A reddens:
AFFORDANCE,COPY AFFORDANCE,COUNTER,⭐ AMPLIFICATION,DECLARED COST,MEASUREMENT.B reddens all of those except
⭐ AMPLIFICATION, plusNON-REGRESSION — a POPULATED array is still a value, and so is a populated object,NON-REGRESSION — 0 and false are still values, andTHE BOUNDARY — {} is a VALUE.C reddens exactly
NON-REGRESSION — a POPULATED array …andMEASUREMENT.THE BOUNDARY. C is the second caricature — "every array is empty" — and only the populated-array axis refuses it.RelatedListis untouched — and the direction is the pointgit difftouches three paths, none of themRelatedList.tsx:RelatedList.isValueEmpty(RelatedList.tsx:913-917) spells(Array.isArray(v) && v.length === 0)and has for some time. The two predicates now agree on every probe measured —[]empty;{},[1],{ a: 1 },0and aDateall values — because the shared authority moved toward the local one. PR #8476 measured that the local shape is the better-shaped answer for a grid and declined to delegate precisely because of this hole; that measurement stands, and nothing here asks it to change.mergeable_state: behindat the time of writing), so its pin fileRelatedList.emptinessAgreement-8459.test.tsxis not on this base. Nothing in this branch depends on it; the agreement above is asserted againstRelatedList.tsxas it stands onc90395b20, and the two branches touch no common file.Tests
Run from the repo root, paths relative to the root, nothing after
--.pnpm exec vitest run packages/plugin-detail/→ 140 files / 1270 tests passedpnpm exec vitest run packages/plugin-detail/src/__tests__/DetailSection.emptyArray-8474.test.tsx→ 9/9, and 9/9 again after the ablation harness restored the treepnpm --filter @object-ui/plugin-detail run type-check→ exit 0 (tsc --noEmit && tsc -p tsconfig.test.json)pnpm --filter @object-ui/plugin-detail run lint→ exit 0,✖ 905 problems (0 errors, 905 warnings); the new test file contributes nonepnpm --workspace-concurrency=2 --filter '@object-ui/plugin-detail^...' build→ exit 0pnpm run check:control-bytes→ exit 0, plus a direct scan of the three changed paths with a lit control that fired on an injected escape-spelled U+0001pnpm run check:unreferenced-sources→ exit 0Declared narrowing.
app-shell,console,plugin-kanban,plugin-calendar,plugin-ganttandexamples/console-starterdepend on@object-ui/plugin-detail, so they are affected packages.hasCellValueis not exported outside the package, and no test outside it reads theNo valueaffordance (grep forNo value/noValueacrossapp-shell,consoleand the three sibling plugins matches six FILES — a CHANGELOG, two metadata-admin inspectors, an i18n table,paramValueShape.tsandresolveActionParams.test.ts— none of them a detail-page render test). I ran five app-shell record-detail files anyway —RecordDetailView.actionUser,RecordDetailView.activityMapIdentity-5878,RecordDetailView.approvalDeclaredActions,auditHistoryDisplay,deriveRelatedLists.relatedListFilter-4664→ 5 files / 61 tests passed. The rest of those packages is left to CI; this narrowing is declared, not silently skipped.Changeset
.changeset/8474-empty-array-is-not-a-cell-value.md,@object-ui/plugin-detail: minor— the landed precedent for aplugin-detailrender change (objectui#8457, and PR #8476 for the same surface). The gate's own verdict line:node scripts/check-changeset-no-major.mjs→✅ No changeset declares a "major" bump.majoris forbidden in this repo (one fixed group).skip-changesetwas not applied — it is a phantom label here, and this change is user-visible anyway.node scripts/check-governed-queue-guard.mjs --test THE-THREE-CHANGED-PATHS(passed explicitly, never bare) →✅ NOT GOVERNED — 3 path(s) checked against 5 governed surface(s); none matched.Out-of-scope finding, filed unassigned
objectui#8481 —
SelectCellRenderer's own opening guard isvalue == null || value === '', which[]passes, so the renderer itself draws a childless flex-wrap DIV. This card and PR #8476 both guard it with an upstream pre-check inside@object-ui/plugin-detail; the shared renderer in@object-ui/fieldsis still wrong for every consumer that does not.ObjectGridis one, verified: it calls the resolved renderer with the raw value at five sites and its onlyEmptyValuefallback is the no-renderer default path, whose guard has the same hole.Notes for review
d7fecfbcf), soemptiness.tsis a real file onmainand no PR-ref reading was needed. PR fix(plugin-detail): RelatedList's column pruning and its cell placeholder share ONE definition of emptiness #8476, by contrast, is still open, so its pin file is not available to lean on.issue_writecreate (objectui#8481) they came back intact through a read-back to the end of the body. One observation is not a refutation — the rule may describe an intermittent or endpoint-specific behaviour — so I kept the mitigation everywhere anyway, since spelling shapes out in words costs nothing. Recording it because a rule nobody can reproduce eventually gets ignored for the wrong reason.packages/specis not involved; this is a renderer-side emptiness decision, not a metadata contract, so contract-first does not apply.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code