From ba596a8c0347f666bd72b30222cf2060f5db2f53 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 13:53:08 +0000 Subject: [PATCH 1/2] fix(react): mapDensity abstains on non-string rowHeight instead of coercing it (#4459) `mapDensity` opened with a truthiness guard, so any truthy non-string survived it and was then coerced into a lookup key -- both `hasOwnProperty.call` and the table index run `String(...)`. `['compact']`, a boxed `String('compact')` and `{ toString: () => 'compact' }` therefore each selected a real density, while core's `rowHeightToDensityMode` -- which opens with `typeof rowHeight !== 'string'` -- abstained for the same input. Replace the truthiness guard with core's type guard. The type guard subsumes the old one (undefined/null/0/false are all non-strings) and `''` keeps its answer by a different route: a string now, refused one line later by the existing `hasOwnProperty` guard because it is not one of the five spec keys. Extends the agreement pin with the non-string family, closing the gap between what the file's title claims and what it pinned -- both prior off-spec families are strings. Red-first: 7 failures pre-fix, all three coercing non-strings; 38/38 green after. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .changeset/tidy-apes-shave.md | 21 +++++ .../RowHeightDensityAgreement.test.ts | 86 ++++++++++++++++++- .../src/spec-bridge/bridges/list-view.ts | 15 +++- 3 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 .changeset/tidy-apes-shave.md diff --git a/.changeset/tidy-apes-shave.md b/.changeset/tidy-apes-shave.md new file mode 100644 index 000000000..f21a2344e --- /dev/null +++ b/.changeset/tidy-apes-shave.md @@ -0,0 +1,21 @@ +--- +'@object-ui/react': patch +--- + +fix(react): the spec bridge abstains on a non-string `rowHeight` instead of coercing it to a density, matching core + +`mapDensity` opened with a truthiness guard, so any **truthy non-string** survived it and was +then coerced into a lookup key — both `Object.prototype.hasOwnProperty.call` and the table index +run `String(...)`. `rowHeight: ['compact']`, a boxed `String('compact')` or +`{ toString: () => 'compact' }` therefore each selected a real density, while +`@object-ui/core`'s `rowHeightToDensityMode` — which opens with `typeof rowHeight !== 'string'` — +abstained for the same input. Two published surfaces, two answers for one input. + +The bridge now opens with core's type guard. An off-spec non-string `rowHeight` renders exactly +like an absent one, and the producer is where it gets fixed (AGENTS.md #0.1). Behaviour for the +five spec row heights and for off-spec **strings** is unchanged; `''` keeps its answer by a +different route (a string now, refused one line later because it is not one of the five keys). + +Note the direction against the previous fix in this function: that leak returned a *function*, +visibly wrong to everything downstream. This one returned a legitimate-looking `'compact'` that +nothing downstream could tell apart from an authored density. diff --git a/packages/react/src/spec-bridge/__tests__/RowHeightDensityAgreement.test.ts b/packages/react/src/spec-bridge/__tests__/RowHeightDensityAgreement.test.ts index 67cc8a393..fb43af712 100644 --- a/packages/react/src/spec-bridge/__tests__/RowHeightDensityAgreement.test.ts +++ b/packages/react/src/spec-bridge/__tests__/RowHeightDensityAgreement.test.ts @@ -42,6 +42,47 @@ function bridgeDensityFor(rowHeight: unknown): DensityMode | undefined { return 'density' in node ? (node.density as DensityMode | undefined) : undefined; } +/** + * The third off-spec family (#4459) — values that are not strings at all. + * + * The first two families are both strings, so for a while the invariant this + * file's title claims ("core and the spec bridge give one answer") was strictly + * broader than the invariant it pinned. A JSON-authored view can hold an array + * or an object at `rowHeight` just as easily as a bad string, and until #4459 + * the two surfaces disagreed on exactly that: core opens with a TYPE guard + * (`typeof rowHeight !== 'string'`), the bridge opened with a TRUTHINESS guard, + * so every truthy non-string walked past it and was then coerced to a key — + * `hasOwnProperty.call` and the index both run `String(...)`. + * + * Split into two groups on purpose, because they fail differently: + * + * - **coercing** — their string form IS one of the five spec keys, so the + * bridge answered `'compact'`: a density fabricated from an array. This is + * the quieter half of #4442's defect. A leaked function is visibly wrong to + * everything downstream; a fabricated `'compact'` is a perfectly legitimate + * value that nothing can tell apart from an authored one. + * - **non-coercing** — truthy non-strings whose string form is not a key + * (`'42'`, `'true'`), plus the falsy values the truthiness guard used to be + * there for. These already abstained. They are pinned anyway because #4459 + * REPLACES that guard rather than adding to it: the type guard has to keep + * catching everything the truthiness guard caught, and this is the row that + * says so. + */ +const NON_STRING_ROW_HEIGHTS: ReadonlyArray = [ + // Coercing: String(…) lands on a real spec key. + ["the array ['compact']", ['compact']], + ['the boxed String(compact)', new String('compact')], + ['an object whose toString() returns compact', { toString: () => 'compact' }], + // Non-coercing truthy non-strings. + ['the number 42', 42], + ['the boolean true', true], + // Falsy — what the retired truthiness guard existed to catch. + ['the number 0', 0], + ['the boolean false', false], + ['null', null], + ['undefined', undefined], +]; + describe('rowHeight → density: core and the spec bridge give one answer (#4440)', () => { describe('the five spec row heights — controls, green on both sides', () => { it.each([ @@ -105,7 +146,7 @@ describe('rowHeight → density: core and the spec bridge give one answer (#4440 // Same assertion phrased as the invariant itself: whatever the answer is, // it is ONE answer. A future edit that re-adds a fallback to either // surface breaks this even if it re-adds it to both differently. - for (const rowHeight of [ + const offSpec: unknown[] = [ 'comfortable', 'spacious', 'small', @@ -119,9 +160,50 @@ describe('rowHeight → density: core and the spec bridge give one answer (#4440 'isPrototypeOf', 'propertyIsEnumerable', 'toLocaleString', - ]) { + // #4459 — the non-string family, run through the same invariant. This + // is the assertion the truthiness guard actually broke. + ...NON_STRING_ROW_HEIGHTS.map(([, value]) => value), + ]; + for (const rowHeight of offSpec) { expect(rowHeightToDensityMode(rowHeight)).toBe(bridgeDensityFor(rowHeight)); } }); }); + + describe('non-string row heights — both abstain (#4459)', () => { + it.each(NON_STRING_ROW_HEIGHTS)( + 'neither surface invents a density for %s', + (_label, rowHeight) => { + expect(rowHeightToDensityMode(rowHeight)).toBeUndefined(); + expect(bridgeDensityFor(rowHeight)).toBeUndefined(); + }, + ); + + it.each(NON_STRING_ROW_HEIGHTS)( + 'never writes a density fabricated from %s onto the SchemaNode', + (_label, rowHeight) => { + // The boundary expression #4459 measured, read straight off the node + // rather than through the helper — the same reason #4442 states one + // separately. `bridgeListView` writes the key under `if (density)`, so + // a fabricated `'compact'` is not merely returned, it is STORED, and + // downstream it is indistinguishable from an authored density. + const node = new SpecBridge().transformListView({ name: 'x', rowHeight }); + expect(node.density).toBeUndefined(); + }, + ); + + it('abstains on the empty string by a different route, same answer', () => { + // `''` is the one input whose HANDLING changes without its ANSWER + // changing, so it is worth stating on its own. Before #4459 it was caught + // by the truthiness guard (`''` is falsy) and never reached the table; + // after, it is a string, so it passes the type guard and is refused one + // line later by `hasOwnProperty` — `''` is not one of the five keys. + // Both routes end in `undefined`, and core has always agreed. + expect(bridgeDensityFor('')).toBeUndefined(); + expect(rowHeightToDensityMode('')).toBeUndefined(); + expect( + new SpecBridge().transformListView({ name: 'x', rowHeight: '' }).density, + ).toBeUndefined(); + }); + }); }); diff --git a/packages/react/src/spec-bridge/bridges/list-view.ts b/packages/react/src/spec-bridge/bridges/list-view.ts index e6571b5b2..801e36954 100644 --- a/packages/react/src/spec-bridge/bridges/list-view.ts +++ b/packages/react/src/spec-bridge/bridges/list-view.ts @@ -88,11 +88,24 @@ const ROW_HEIGHT_TO_DENSITY: Record< * `metadata-admin/predicate.ts:305`, six more). Both rowHeight surfaces now * abstain identically on every off-spec spelling; the agreement is pinned by * `__tests__/RowHeightDensityAgreement.test.ts`. + * + * `typeof rowHeight !== 'string'`, not `!rowHeight` (objectui#4459): the + * truthiness guard this replaces rejected only FALSY values, so every truthy + * non-string walked past it into a lookup that coerces its key — both + * `hasOwnProperty.call` and the index run `String(...)`. `['compact']`, a boxed + * `String('compact')` and `{ toString: () => 'compact' }` therefore each + * selected a real density, where core's twin (same file, opening with the type + * guard) abstained. Note the direction against #4442: that leak produced a + * FUNCTION, visibly wrong to everything downstream; this one produced a + * legitimate-looking `'compact'` that nothing can tell apart from an authored + * value. The type guard subsumes the old one — `undefined`, `null`, `0` and + * `false` are all non-strings — and `''` keeps its answer while changing route: + * a string, so it passes here and is refused by `hasOwnProperty` one line down. */ function mapDensity( rowHeight?: RowHeight, ): 'compact' | 'comfortable' | 'spacious' | undefined { - if (!rowHeight) return undefined; + if (typeof rowHeight !== 'string') return undefined; if (!Object.prototype.hasOwnProperty.call(ROW_HEIGHT_TO_DENSITY, rowHeight)) { return undefined; } From 38eb8ff5ea4c83aeaf1a0bbbc392090b55fa9eeb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 13:56:38 +0000 Subject: [PATCH 2/2] docs(react): cite core's twin by file path in the mapDensity comment The #4459 note said core's twin lives in the "same file". It does not -- `rowHeightToDensityMode` is in `@object-ui/core` (`packages/core/src/utils/normalize-list-view.ts:83`). Comment-only; cite the path the way the rest of this file's notes do. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- packages/react/src/spec-bridge/bridges/list-view.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react/src/spec-bridge/bridges/list-view.ts b/packages/react/src/spec-bridge/bridges/list-view.ts index 801e36954..e72e4aac3 100644 --- a/packages/react/src/spec-bridge/bridges/list-view.ts +++ b/packages/react/src/spec-bridge/bridges/list-view.ts @@ -94,8 +94,9 @@ const ROW_HEIGHT_TO_DENSITY: Record< * non-string walked past it into a lookup that coerces its key — both * `hasOwnProperty.call` and the index run `String(...)`. `['compact']`, a boxed * `String('compact')` and `{ toString: () => 'compact' }` therefore each - * selected a real density, where core's twin (same file, opening with the type - * guard) abstained. Note the direction against #4442: that leak produced a + * selected a real density, where core's twin abstained + * (`packages/core/src/utils/normalize-list-view.ts:83`, which has opened with + * the type guard since #4440). Note the direction against #4442: that leak produced a * FUNCTION, visibly wrong to everything downstream; this one produced a * legitimate-looking `'compact'` that nothing can tell apart from an authored * value. The type guard subsumes the old one — `undefined`, `null`, `0` and