Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-grid): resolve off-spec rowHeight at the state boundary instead of styling it as medium (#4443) - #4458
Merged
yinlianghui merged 1 commit intoAug 12, 2026
Conversation
…stead of styling it as medium (#4443) ObjectGrid seeded its density state with `schema.rowHeight ?? 'compact'`, so one component answered one question two ways: an ABSENT rowHeight landed on `compact`, an OFF-SPEC one skipped every arm of the density ternaries and came out at their terminal `else` — the `medium` styling. That is the absent-vs-off-spec split #4440 removed from ListView, and it made a standalone grid a third answer to a question `@object-ui/core` (`rowHeightToDensityMode`, abstains) and the `@object-ui/react` spec bridge (#4352, abstains) had already settled. Both entry points — the initial state and the effect that re-syncs when the prop changes — now go through one resolver that admits only the five spec row heights, guarded with `hasOwnProperty` rather than `in`. The ternary chains are untouched: `medium` stays a real value and the terminal `else` stays its arm. Red-first, and the two off-spec spellings failed differently before the fix: `'toString'` reached `Object.prototype.toString` through the toolbar icon map's prototype chain and rendered as medium (the defect as filed), while a plain `'garbage'` was not a key of that map either, so `<RowHeightIcon />` was `undefined` and the standalone grid threw `Element type is invalid` rather than rendering as medium at all. Both are inert now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
commented
Aug 12, 2026
CollaboratorAuthor
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. Slot NOT refilled per maintainer's wind-down instruction. rowHeight family state after this lands: core, bridge, ListView, and standalone ObjectGrid all give one answer on off-spec strings; #4459 (non-string coercion in the bridge) remains the pooled residual. Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 10:39
Uh oh!
There was an error while loading. Please reload this page.
yinlianghui
deleted the
claude/issue-4443-objectgrid-rowheight-boundary
branch
August 12, 2026 10:40
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes#4443
The defect
ObjectGridseeded its density state withschema.rowHeight ?? 'compact', so one component answered one question two ways:rowHeightlands oncompactrowHeightskips every arm of the density ternaries and comes out at their terminalelse— themediumstylingThat is the absent-vs-off-spec split #4440 removed from
ListView, and it made a standalone grid the third answer to a question the rest of the system had already settled:@object-ui/core'srowHeightToDensityModeabstains for an off-spec value, the@object-ui/reactspec bridge (#4352) abstains, andListViewdefaults that abstention tocompact.Only the standalone path is affected. When
ListViewowns the grid it overwrites the prop with a value derived fromdensity.mode, so nothing off-spec survives that hop.The fix — narrow at the boundary (ruling option 1)
One resolver that admits only the five spec row heights, applied at both entry points that feed author-supplied
rowHeightinto the state:useStateseed, androwHeightprop changes (e.g. a parentListView's density toggle) — this was a second unvalidated writer, now routed through the same resolver.The ternary chains are deliberately untouched:
mediumstays a real row height with its own styling arm, and a leaf renderer's terminalelsestays that arm. What changes is that nothing unrecognized can reach it.Membership is tested against
ROW_HEIGHT_TO_DENSITY_MODErather than a local list, so the admitted values keep one definition in the repo; that table is typedRecord< RowHeight, DensityMode >, so the build fails if the spec grows a sixth row height without the resolver being taught about it.hasOwnProperty, notin— same reason@object-ui/coreuses it.Red-first, and a correction to the issue's premise
The two off-spec spellings failed differently before this change, which the issue did not distinguish. Both are covered, and the boundary fix closes both:
'toString'rendered asmedium— the defect exactly as filed. The toolbar's row-height icon map is looked up by the same unvalidated state, and a prototype member resolves through that map's prototype chain toObject.prototype.toString, a function React accepts as a component. So it survived to the ternaries.'garbage'did not render at all — it is not a key of the icon map either, so the lookup producedundefinedand rendering the icon threwElement type is invalid. The toolbar is shown precisely whenschema.rowHeightis defined, so this crash and the off-spec case coincide exactly.The issue's "renders an off-spec rowHeight as
medium" is therefore true only for prototype-member spellings; a plain off-spec value crashed the standalone grid. Verbatim pre-fix red (5 failed, 6 passed):Post-fix: 11 passed (11).
Both ternary copies are asserted independently
They are separately observable in the DOM, so neither assertion is vacuous:
rowHeightCellClass) is the only one carrying theh-*row-height floor, and lands on the data columns —h-9vsh-11;dataTableSchema.cellClassName) is the default for columns declaring none — the narrow row-number column — and carries padding/leading only, noh-*.Verification
pnpm vitest run packages/plugin-grid/src/__tests__/rowHeightOffSpecBoundary.test.tsx— red first (5 failed / 6 passed), green after (11 passed)pnpm vitest run --maxWorkers=2 packages/plugin-grid— 60 files, 571 passedtsc --noEmitandtsc -p tsconfig.test.json— both cleanpnpm --filter @object-ui/plugin-grid lint— 0 errorscheck:control-bytes,check:phantom-deps,check:spec-symbols— all green.d.tsdiffed before/after the change: byte-identical, so the changeset ispatchas graded, no escalation to minorGenerated by Claude Code