refactor(core): rowHeightToDensityMode abstains for an off-spec rowHeight, matching the spec bridge (#4440)

.changeset/rowheight-density-one-answer-4440.md

@@ -0,0 +1,42 @@
+---
+'@object-ui/core': minor
+'@object-ui/plugin-list': minor
+---
+
+`rowHeightToDensityMode` answers only for the five spec row heights — the coerce-to-`comfortable` fallback is gone
+
+Two surfaces narrow a list view's `rowHeight` onto the renderer's three-step
+density vocabulary, and since objectui#4352 they answered differently for the
+same off-spec input: `@object-ui/react`'s spec bridge declined to answer, while
+`@object-ui/core`'s `rowHeightToDensityMode` rehabilitated anything unknown into
+`comfortable`. One metadata-driven system, two answers for one input
+(objectui#4440).
+
+The strict answer wins, per AGENTS.md #0.1: a renderer-side rehabilitation of
+off-spec metadata is a second de-facto contract, and one strict contract beats N
+dialects — a bad `rowHeight` gets fixed at the producer, where the schema already
+rejects it. The five mappings themselves are untouched (`compact`/`short` →
+`compact`, `medium` → `comfortable`, `tall`/`extra_tall` → `spacious`), and the
+table keeps its `Record< RowHeight, … >` typing, so a row height added upstream
+still fails the build here.
+
+**Breaking semantics, deliberately graded `minor`** (this repo never publishes
+`major` — its major tracks `@objectstack`). Two things change:
+
+- **Published type.** `rowHeightToDensityMode` is exported from
+  `@object-ui/core`, and its return widens from `DensityMode` to
+  `DensityMode | undefined`. A host assigning the result straight into a
+  `DensityMode` now has to say what an off-spec row height should mean to it.
+- **Rendered output, for input the spec already rejects.** `ListView` — the one
+  in-repo caller — used to render an off-spec `rowHeight` one step looser than an
+  ABSENT one (`comfortable`, 40px rows, vs `compact`, 32px). It now renders it
+  exactly like an absent one, `compact`, which is also `ObjectGrid`'s own default.
+  A sweep of this repo, the `objectstack` example apps and one downstream app
+  found zero authored off-spec values, and the legacy `densityMode` alias cannot
+  produce one (`DENSITY_MODE_TO_ROW_HEIGHT` is typed
+  `Record< DensityMode, RowHeight >`).
+
+Also closed while retiring the branch: the lookup guarded membership with `in`,
+which walks the prototype chain, so `rowHeight: 'toString'` returned
+`Object.prototype.toString` — a function — from something typed `DensityMode`. It
+is an own-property check now.

packages/core/src/utils/__tests__/normalize-list-view.test.ts

@@ -9,6 +9,7 @@
 import { describe, it, expect } from 'vitest';
 import {
   normalizeListViewSchema,
+  rowHeightToDensityMode,
   DENSITY_MODE_TO_ROW_HEIGHT,
   ROW_HEIGHT_TO_DENSITY_MODE,
 } from '../normalize-list-view.js';
@@ -290,3 +291,48 @@ describe('normalizeListViewSchema (#2890)', () => {
     });
   });
 });
+
+describe('rowHeightToDensityMode (#4440)', () => {
+  describe('the five spec row heights — the mapping itself, unchanged', () => {
+    it.each([
+      ['compact', 'compact'],
+      ['short', 'compact'],
+      ['medium', 'comfortable'],
+      ['tall', 'spacious'],
+      ['extra_tall', 'spacious'],
+    ] as const)('maps the spec row height %s to %s', (rowHeight, density) => {
+      expect(rowHeightToDensityMode(rowHeight)).toBe(density);
+    });
+  });
+
+  describe('off-spec input — abstain, do not rehabilitate', () => {
+    // The retired branch answered `'comfortable'` here, while
+    // `@object-ui/react`'s spec bridge answered "no density at all" for the
+    // same string after #4352 (PR #4439). One metadata-driven system, two
+    // answers for one input, was #4440; this is the surviving answer.
+    it.each([
+      'comfortable', // the OUTPUT vocabulary, never a spec row height
+      'spacious',
+      'small',
+      'large',
+      'gargantuan', // a string in neither vocabulary
+      '',
+    ])('gives no density for the off-spec rowHeight %j', (rowHeight) => {
+      expect(rowHeightToDensityMode(rowHeight)).toBeUndefined();
+    });
+
+    it('gives no density for non-string metadata', () => {
+      // Stored view definitions reach this function from a database that
+      // TypeScript never saw, so the runtime guard is load-bearing, not
+      // decoration: `ListViewSchema.rowHeight` is statically `RowHeight`.
+      for (const value of [undefined, null, 42, true, {}, ['medium']]) {
+        expect(rowHeightToDensityMode(value)).toBeUndefined();
+      }
+    });
+
+    it('does not answer for an inherited Object.prototype key', () => {
+      expect(rowHeightToDensityMode('toString')).toBeUndefined();
+      expect(rowHeightToDensityMode('constructor')).toBeUndefined();
+    });
+  });
+});

packages/core/src/utils/normalize-list-view.ts

@@ -53,17 +53,38 @@ export const ROW_HEIGHT_TO_DENSITY_MODE: Record<RowHeight, DensityMode> = {
 };
 
 /**
- * Tolerant reader for {@link ROW_HEIGHT_TO_DENSITY_MODE}. View metadata is
- * user-authored, so `rowHeight` is not guaranteed to be one of the five spec
- * values — an unknown one lands on `comfortable` rather than rendering an
- * undefined density. Keeping the fallback here means every surface collapses
- * the same way.
+ * Runtime reader for {@link ROW_HEIGHT_TO_DENSITY_MODE}: the spec's five row
+ * heights narrowed onto the renderer's three densities, and **nothing else**.
+ *
+ * The parameter stays `unknown` because this is the boundary user-authored view
+ * metadata actually crosses — `ListViewSchema.rowHeight` is statically a
+ * `RowHeight`, but the value arrives from stored view definitions TypeScript
+ * never saw (`ObjectView`: `viewDef.rowHeight ?? listSchema.rowHeight`). The
+ * type-level half of the guarantee is the `Record<RowHeight, …>` on the table
+ * above, which fails the build when the spec grows a row height.
+ *
+ * An off-spec value gets NO density (objectui#4440). It used to be coerced to
+ * `comfortable`, which is the opposite of what `@object-ui/react`'s spec bridge
+ * answers for the same string after objectui#4352 — one metadata-driven system
+ * holding two answers for one input. AGENTS.md #0.1 decides which one survives:
+ * a renderer-side rehabilitation of off-spec metadata is a second de-facto
+ * contract, and one strict contract beats N. The producer is where a bad
+ * `rowHeight` gets fixed.
+ *
+ * Callers apply their own "nothing was said" default to `undefined`, so an
+ * off-spec row height now renders exactly like an absent one — `'compact'` in
+ * both `ListView` and `ObjectGrid`.
+ *
+ * `hasOwnProperty`, not `in`: `in` walks the prototype chain, so `'toString'`
+ * used to come back as `Object.prototype.toString` — a FUNCTION returned from
+ * something typed `DensityMode`.
  */
-export function rowHeightToDensityMode(rowHeight: unknown): DensityMode {
-  if (typeof rowHeight === 'string' && rowHeight in ROW_HEIGHT_TO_DENSITY_MODE) {
-    return ROW_HEIGHT_TO_DENSITY_MODE[rowHeight as RowHeight];
+export function rowHeightToDensityMode(rowHeight: unknown): DensityMode | undefined {
+  if (typeof rowHeight !== 'string') return undefined;
+  if (!Object.prototype.hasOwnProperty.call(ROW_HEIGHT_TO_DENSITY_MODE, rowHeight)) {
+    return undefined;
   }
-  return 'comfortable';
+  return ROW_HEIGHT_TO_DENSITY_MODE[rowHeight as RowHeight];
 }
 
 const isRecord = (v: unknown): v is Record<string, unknown> =>

packages/plugin-list/src/ListView.tsx

@@ -1034,10 +1034,18 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
   // legacy `densityMode` is folded into it by `normalizeListViewSchema` above —
   // it used to be read FIRST here, so a view carrying both rendered the legacy
   // value, backwards from every other pair's canonical-wins precedence.
-  const resolvedDensity = React.useMemo(() => {
-    if (schema.rowHeight) return rowHeightToDensityMode(schema.rowHeight);
-    return 'compact';
-  }, [schema.rowHeight]);
+  //
+  // `rowHeightToDensityMode` answers only for the five spec row heights and
+  // abstains for anything else (#4440), so an off-spec value lands on the same
+  // `'compact'` an ABSENT `rowHeight` has always landed on — which is also
+  // `ObjectGrid`'s own default (`schema.rowHeight ?? 'compact'`). Do NOT drop
+  // the `??` and let `undefined` reach `useDensityMode`: its parameter default
+  // is `'comfortable'`, so the coercion #4440 retired would simply reappear one
+  // frame lower, and the two surfaces would disagree again.
+  const resolvedDensity = React.useMemo(
+    () => rowHeightToDensityMode(schema.rowHeight) ?? 'compact',
+    [schema.rowHeight],
+  );
   const density = useDensityMode(resolvedDensity, {
     onChange: schema.onDensityChange,
   });

packages/plugin-list/src/__tests__/ListView.density.offspec.test.tsx

@@ -0,0 +1,110 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import { SchemaRendererProvider } from '@object-ui/react';
+import type { ListViewSchema } from '@object-ui/types';
+import { ListView } from '../ListView';
+
+/**
+ * What an off-spec `rowHeight` actually RENDERS — the empirical half of
+ * objectui#4440, pinned on the live path rather than argued from the mapping.
+ *
+ * `ListView` is the only in-repo caller of `@object-ui/core`'s
+ * `rowHeightToDensityMode`. Before #4440 that function coerced an unknown
+ * `rowHeight` to `'comfortable'`, so a garbage value rendered the toolbar (and
+ * the rows it forwards its density to) one step looser than an ABSENT
+ * `rowHeight` did — absent has always resolved to `'compact'` here, and
+ * `ObjectGrid` independently defaults `schema.rowHeight ?? 'compact'` too.
+ *
+ * After #4440 the mapping abstains and the caller's own "nothing was said"
+ * default applies, so off-spec renders exactly like absent. That is the
+ * measured consequence of aligning with `@object-ui/react`'s spec bridge, and
+ * this file is where it is visible: the density button's `aria-label` is
+ * `density.mode` verbatim.
+ *
+ * Note the abstain deliberately does NOT fall through to `useDensityMode`'s own
+ * parameter default, which is `'comfortable'` — landing there would have
+ * re-created the retired coercion one stack frame further down and left the two
+ * surfaces disagreeing exactly as before, while looking like a fix.
+ */
+
+const mockDataSource = {
+  find: vi.fn().mockResolvedValue([]),
+  findOne: vi.fn(),
+  create: vi.fn(),
+  update: vi.fn(),
+  delete: vi.fn(),
+};
+
+const renderListView = (rowHeight?: unknown) => {
+  const schema = {
+    type: 'list-view',
+    objectName: 'contacts',
+    viewType: 'grid',
+    columns: ['name', 'email'],
+    ...(rowHeight === undefined ? {} : { rowHeight }),
+  } as ListViewSchema;
+
+  return render(
+    <SchemaRendererProvider dataSource={mockDataSource}>
+      <ListView schema={schema} />
+    </SchemaRendererProvider>,
+  );
+};
+
+/** The toolbar's density button labels itself `Density: <mode>`. */
+const renderedDensity = () =>
+  screen
+    .getByRole('button', { name: /^Density: / })
+    .getAttribute('aria-label')
+    ?.replace('Density: ', '');
+
+describe('ListView density for an off-spec rowHeight (#4440)', () => {
+  describe('spec row heights — controls, unaffected by the change', () => {
+    it.each([
+      ['compact', 'Compact'],
+      ['short', 'Compact'],
+      ['medium', 'Comfortable'],
+      ['tall', 'Spacious'],
+      ['extra_tall', 'Spacious'],
+    ])('renders %s as %s', (rowHeight, expected) => {
+      renderListView(rowHeight);
+      expect(renderedDensity()).toBe(expected);
+    });
+
+    it('renders an absent rowHeight as Compact', () => {
+      renderListView(undefined);
+      expect(renderedDensity()).toBe('Compact');
+    });
+  });
+
+  describe('off-spec rowHeight renders exactly like an absent one', () => {
+    // Every one of these rendered `Comfortable` before #4440. No authored
+    // metadata in objectui, objectstack or the surveyed downstream app spells
+    // any of them (the PR #4439 sweep, re-run for objectui and objectstack),
+    // so this is the rendered consequence of a value that does not exist —
+    // documented because it was measured, not because it was reachable.
+    it.each(['comfortable', 'spacious', 'small', 'large', 'gargantuan'])(
+      'renders the off-spec rowHeight %s as Compact',
+      (rowHeight) => {
+        renderListView(rowHeight);
+        expect(renderedDensity()).toBe('Compact');
+      },
+    );
+
+    it('renders a non-string rowHeight as Compact', () => {
+      // Stored view definitions cross an untyped boundary
+      // (`ObjectView`: `viewDef.rowHeight ?? listSchema.rowHeight`), so the
+      // value is only statically a `RowHeight`.
+      renderListView(42);
+      expect(renderedDensity()).toBe('Compact');
+    });
+  });
+});

packages/react/src/spec-bridge/__tests__/RowHeightDensityAgreement.test.ts

@@ -0,0 +1,90 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { rowHeightToDensityMode, type DensityMode } from '@object-ui/core';
+import { SpecBridge } from '../SpecBridge';
+
+/**
+ * The agreement pin for objectui#4440.
+ *
+ * Two surfaces narrow a list view's `rowHeight` onto the renderer's three-step
+ * density vocabulary, and for a while they answered differently for the same
+ * off-spec input: `@object-ui/core`'s `rowHeightToDensityMode` coerced anything
+ * unknown to `'comfortable'`, while this package's `mapDensity` — after #4352
+ * (PR #4439) — declined to answer at all. One metadata-driven system, two
+ * answers for one input. #4440 retired the coercion; this test is what stops
+ * the disagreement regrowing silently on either side.
+ *
+ * It lives HERE, not in `@object-ui/core`, because the dependency direction
+ * decides: `@object-ui/react` depends on `@object-ui/core`, so this package can
+ * see both surfaces, and core cannot import react without inverting the graph.
+ * The pin therefore imports core's function by its PUBLISHED specifier
+ * (`@object-ui/core`, a declared dependency of this package) and reaches the
+ * bridge through `SpecBridge.transformListView`, whose parameter is `any` —
+ * the untyped boundary a host's stored JSON actually crosses, and after #4352
+ * the only way an off-spec `rowHeight` can enter the bridge at all.
+ */
+
+/** What the bridge answers for a `rowHeight`, in core's return shape. */
+function bridgeDensityFor(rowHeight: unknown): DensityMode | undefined {
+  const node = new SpecBridge().transformListView({
+    name: 'row_height_agreement',
+    rowHeight,
+  });
+  // The bridge writes the key only when it has an answer, so an absent key and
+  // an explicit `undefined` are the same statement: "no density, use yours".
+  return 'density' in node ? (node.density as DensityMode | 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([
+      ['compact', 'compact'],
+      ['short', 'compact'],
+      ['medium', 'comfortable'],
+      ['tall', 'spacious'],
+      ['extra_tall', 'spacious'],
+    ] as const)('both surfaces map %s to %s', (rowHeight, expected) => {
+      expect(rowHeightToDensityMode(rowHeight)).toBe(expected);
+      expect(bridgeDensityFor(rowHeight)).toBe(expected);
+    });
+  });
+
+  describe('off-spec row heights — both abstain', () => {
+    // `comfortable` / `spacious` / `small` / `large` are the four spellings
+    // #4352 deleted from the bridge; `gargantuan` is a string in neither
+    // vocabulary. Before #4440 core answered `'comfortable'` for every one of
+    // them while the bridge answered nothing.
+    it.each(['comfortable', 'spacious', 'small', 'large', 'gargantuan'])(
+      'neither surface invents a density for the off-spec rowHeight %s',
+      (rowHeight) => {
+        expect(rowHeightToDensityMode(rowHeight)).toBeUndefined();
+        expect(bridgeDensityFor(rowHeight)).toBeUndefined();
+      },
+    );
+
+    // NOT pinned here, deliberately: `Object.prototype` member names
+    // (`toString`, `constructor`, …). Core abstains for them since #4440, but
+    // the bridge still indexes its table with an unguarded key and hands back
+    // `Object.prototype.toString` — a FUNCTION — as the density. That is a
+    // different defect from the coercion this file is about, it lives in source
+    // outside #4440's surface, and it is filed as #4442. Extending the two
+    // `it.each` lists above with those keys is the assertion that fails until
+    // #4442 lands, and is the natural test half of its fix.
+
+    it('agrees for every off-spec input without either side being read first', () => {
+      // 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 ['comfortable', 'spacious', 'small', 'large', 'gargantuan', '']) {
+        expect(rowHeightToDensityMode(rowHeight)).toBe(bridgeDensityFor(rowHeight));
+      }
+    });
+  });
+});