fix(plugin-grid): one owner for ObjectGrid's cell-renderer resolution, so a format-hinted column keeps its renderer on every path - #9010
Merged
Conversation
…at hint -> renderer" `@object-ui/fields` publishes a two-step resolve — `getCellRenderer(` applied to `resolveCellRendererType(...)` — because a textual base type carrying a `format` hint maps to a richer renderer than its declared type does. ObjectGrid spelled that resolve six times with three conventions: one two-step, four raw declared types, and one fixed registry key. On the four raw sites a `text` + `format: 'phone'` column silently resolved to `TextCellRenderer`, which reads only `value` — the hint was dropped with no error, no warning and no `tel:` link, in the flagship grid. Patching four sites to match one would have left the divergence in place, so the resolution itself moves into `./cellRendererResolution`: five sites take its field entry point, the compound-cell prefix badge takes its named fixed-key entry point (a different contract, documented rather than left silent), and ObjectGrid no longer imports either published resolver. The module names the two answers the word "resolve" used to collide on — `declaredType` (which type the field has) and `rendererType` (which renderer that type maps to). A local called `resolvedType` holding only the first is what made four broken sites read as correct. Alignment and the header type icon now follow `rendererType` on every path, as the object-columns path already did; the type forwarded to the inline editor stays `declaredType`, so a hinted text column still edits as text. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
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-steve
marked this pull request as ready for review
September 10, 2026 17:59
os-steve
deleted the
claude/issue-8920-grid-cell-renderer-resolution
branch
September 10, 2026 18:17
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 #8920
The defect, re-measured on today's tree
@object-ui/fieldspublishes a two-step resolve —getCellRendererapplied to the output ofresolveCellRendererType— because a field's declared type is not always the renderer's key: a textual base type carrying aformathint (phone,email,url,currency,percent) maps to the richer renderer. Skip step two and the cell getsTextCellRenderer, which destructures onlyvalueand so can never readfield.format. Nothing throws, nothing warns; the column just renders as plain truncated text where atel:anchor belongs.Every count below is my own, re-derived on
origin/maine8b7b0785with controls — the card'spath:lineanchors are a snapshot and were not cited.Non-test
getCellRenderercall sites underpackages/*/src, excludingpackages/fieldsitself: 15 lines across 10 files. Nine files hold exactly one each and all nine resolve first.ObjectGrid.tsxholds the other six, and they used three conventions:e8b7b07852503inferredType, fromresolveCellRendererType({ type, format })2639'select'2707resolvedType=fieldDef?.type || inferColumnType(...)2864resolvedType=fieldDef?.type || inferColumnType(...)2985field.type4437fieldDef.typeArithmetic closes: 9 files + A = 10 resolving, 4 raw, 1 fixed key = 15.
Instrument validated in both directions before any zero was quoted. A first pathspec (
packages/*/src) returned 0 call sites — agitpathspec that matches no file path, not a reading. A known-present control token under the same pathspec also returned nothing, which is what exposed it; the corrected enumeration is the table above. AndresolveCellRendererTypeis imported and called insideObjectGrid.tsxitself, so "this site does not resolve" is a real reading rather than a broken probe. Independently, the boundary pin added here counts 6getCellRenderer+ 1resolveCellRendererTypecall shapes in the pre-fix file when run against it — the same six, from a second instrument.What changed: the shared resolution, not four call sites
Per the triage ruling, patching four sites to match one leaves the divergence in place. The resolution itself now lives in
packages/plugin-grid/src/cellRendererResolution.ts, andObjectGrid.tsxno longer imports either published resolver.resolveGridCellRendering(field)— A, B, C, D, E. It returns three things:declaredType(which type the field has),rendererType(which renderer that type maps to,formatapplied), and a totalRenderer.prefix.type === 'badge'is the author saying "draw the neighbouring column's value as a badge", and the key is a constant of this component, not anything the prefixed field declares. There is no declared type to promote and noformatto read, so a helper keyed on "declared type + format hint" cannot own it without misrepresenting the call. It routes through the same module's second, documented entry point —gridCellRendererForFixedKey(BADGE_PREFIX_RENDERER_KEY)— so the grid has exactly two ways to reach a renderer, both in one file, neither of them a bare call.The naming trap, closed
The card asked whether the name was part of the defect. It was.
resolvedTypeheld the answer to "resolve which type the field has" while reading exactly like the answer to "resolve which renderer the type maps to", sogetCellRenderer(resolvedType)slid past review. Both answers are now named, and the two are used deliberately:rendererTypedrives the renderer,fieldMeta.type, the header icon and numeric alignment;declaredTypeis what the emit forwards astype:to the inline editor, so atext+format: 'phone'column still edits as text. Path A already made exactly that split; B, C and D now agree with it.Convergence beyond the renderer, stated explicitly
Alignment and the header type icon read
rendererTypeon every path now, as path A already did — atext+format: 'currency'column right-aligns like the currency it renders as. Path E's innerif (CellRenderer)guard is deleted:getCellRendererends instandardMap[key] || TextCellRendererand never returns anything falsy, and the helper's return type now states that totality.Acceptance: one assertion per path, each with its own control
packages/plugin-grid/src/__tests__/formatHintedColumnRenderer-8920.test.tsxrenders a realObjectGrid(happy-dom) once per path. A single case through one path would prove the helper works, not that the divergence is gone — the population is the subject, so each path gets a case.The oracle is rendered output, because it has to be:
getCellRenderer(field.type)type-checks, so every type-level and doc-level gate reads the broken sites green, and a green that cannot go red on this defect is not evidence.The control column,
note, is declared without a hint and holds a second, different phone number. So each case asserts three things: the hinted value becametel:+15551234567; the control value did not becometel:+15559876543; and the control's text is still on screen, so the absence half cannot be satisfied by the column having vanished. A control holding a non-phone string would only have shown the fix did not reach it; this one shows the promotion is driven by the declaredformatand not by the value's shape.columnsas objectscolumnsas stringsfields, nocolumns)A and P are must-not-change pins, not evidence: A is the site that already resolved, and P has nothing to promote. They are present because a shared helper that quietly changed either would be a regression this file has to catch.
packages/plugin-grid/src/__tests__/cellRendererResolutionBoundary-8920.test.tsadds the structural half — the rendering test cannot see a seventh site added tomorrow, which renders perfectly for every unhinted type. It bounds direct resolver calls underpackages/plugin-grid/srcto the helper module alone, with three anti-vacuity controls (population asserted non-trivial by count; matcher proved on synthetic input in both directions; matcher proved against a real file at real scale).Ablation — predicted before running, and matched
Predicted 4 red / 2 green-both-sides on the render file. Measured, by restoring
ObjectGrid.tsxtoe8b7b0785(the commit pinned at branch creation, never a moving remote-tracking name):55031aa3dto the base's064b293dc, and the injected/removed text was counted in both directions (getCellRenderer(occurrences 0 to 6;resolveGridCellRendering(occurrences 6 to 0);AssertionError: expected [] to include 'tel:+15551234567';[ 'resolveCellRendererType', 'getCellRenderer' x 6 ];git checkout HEAD --on an absolute path, thengit diff HEADempty and the on-disk hash back to55031aa3d. The script carried atrap ... EXIT INT TERMrestore for the crash path.No ablation artefact is left in the tree; the mutation is not resolved through any
dist/, since the root vitest config aliases these packages to source (the tests ran green before any package was built).Gates
turbo run type-check --filter=@object-ui/plugin-gridtsc --noEmitplustsc -p tsconfig.test.json, so the new tests are in the programvitest run packages/plugin-grid/(repo root)vitest run packages/plugin-list/ packages/plugin-view/ObjectGridturbo run lint --filter=@object-ui/plugin-gridno-explicit-anyin the sameas anyidiom every suite in this package usesnode scripts/check-control-bytes.mjsnode scripts/check-changeset-presence.mjsnode scripts/check-governed-queue-guard.mjs --test(5 paths)pnpm check:doc-snippetspnpm check:doc-typespnpm check:doc-examplespnpm check:new-line-citationsHeavy runs went through
../objectstack/scripts/pm/os-verify-lock.shwith a stable slot, per AGENTS.md.Serial adjacency, re-derived here
Derived from the live open-PR list and each PR's own changed-file set against its merge base — no snapshot cited. No open pull request touches
packages/plugin-grid/src/orpackages/fields/src/. #8941 and #7058 touchpackages/plugin-grid/package.json, but only thelucide-reactdependency range; #8996 ispackages/core+packages/data-objectstack, #8998 ispackages/create-plugin, #8999 ispackages/plugin-dashboard, #8984 ispackages/types+packages/plugin-detail. Each adds its own distinctly named changeset. Zero overlap with the five paths here.Scope
Out of scope and untouched, as the card fences:
packages/fields— both published helpers are correct as they stand; the nine already-correct call sites in the other five plugin packages; and the contract-review verdict on PR objectui#8915, one of whose to-clear items this executes. What moved is the six call sites inplugin-grid, plus the module that now owns them.Acceptance notes
Observations recorded here rather than filed as cards — neither is a defect, and no queued pull request or person is heading for either file:
resolveCellRendererTypepromotes a field that declares no type but does carry aformat, because the empty string is a deliberate member of its textual base set.resolveGridCellRenderingdeclines to promote in that case and says why in its own doc comment: every grid path already treats "no declared type" as "no type-aware renderer", and reversing that is wider than this card rules on. Recorded as a bounded decision, not a defect.|| def.type,|| 'text',|| ftype || 'text'). All nine are correct today; this is a shape observation about a surface the card explicitly fences off, with no current successor. Noted, not filed.showColumnTypeIconson, an untyped column gets the generic type icon on the object-columnspath and no icon on the string-columnsand inline-data paths — path A spreadsheaderIconunguarded where B and C guard on the type. Pre-existing, cosmetic, unchanged by this pull request. Noted, not filed.🤖 Generated with Claude Code
https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Generated by Claude Code