diff --git a/.changeset/8920-grid-format-hint-shared-resolution.md b/.changeset/8920-grid-format-hint-shared-resolution.md new file mode 100644 index 0000000000..59dd9f6421 --- /dev/null +++ b/.changeset/8920-grid-format-hint-shared-resolution.md @@ -0,0 +1,9 @@ +--- +"@object-ui/plugin-grid": patch +--- + +fix(plugin-grid): honour a column's `format` hint on every `ObjectGrid` render path + +`@object-ui/fields` publishes a two-step resolve — `getCellRenderer(resolveCellRendererType(field))` — because a textual base type carrying a `format` hint (`phone`, `email`, `url`, `currency`, `percent`) maps to a richer renderer than its declared type does. `ObjectGrid` spelled that resolve six times with three conventions, and four of them passed the declared type straight to `getCellRenderer`. On those paths a `text` + `format: 'phone'` column silently fell back to plain truncated text: no error, no warning, no `tel:` link. + +All six sites now route through one module that owns "declared type + format hint -> renderer", so a hinted column renders the same way whether its columns were declared as objects, as strings, derived from an authored `fields` projection, derived from the object schema, or read in the record-detail panel. Numeric alignment and the header type icon follow the same resolved renderer key on every path, while the type forwarded to the inline editor stays the declared one — a hinted text column still edits as text. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 064b293dcd..55031aa3d0 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -27,7 +27,13 @@ import { isSystemManagedField, normalizeTableColumnType } from '@object-ui/types import type { I18nLabel } from '@objectstack/spec/ui'; import { SchemaRenderer, useDataScope, useNavigationOverlay, useAction, useSafeFieldLabel, usePredicateScope, useRelatedRecordActions } from '@object-ui/react'; import { createSafeTranslation } from '@object-ui/i18n'; -import { getCellRenderer, resolveCellRendererType, formatCurrency, formatCompactCurrency, formatDate, formatPercent, humanizeLabel, getBadgeColorClasses, getBadgeHexAppearance, FieldEditWidget, hasFieldEditWidget, DISCRETE_EDIT_TYPES, coerceToSafeValue } from '@object-ui/fields'; +// objectui#8920 — the grid reaches a cell renderer through THIS module and +// nowhere else. `getCellRenderer` / `resolveCellRendererType` are deliberately +// NOT imported here: six sites spelling the resolve three different ways is +// what dropped a `format`-hinted column's renderer, and one shared owner is +// what stops a seventh site picking a convention of its own. +import { resolveGridCellRendering, gridCellRendererForFixedKey, BADGE_PREFIX_RENDERER_KEY } from './cellRendererResolution'; +import { formatCurrency, formatCompactCurrency, formatDate, formatPercent, humanizeLabel, getBadgeColorClasses, getBadgeHexAppearance, FieldEditWidget, hasFieldEditWidget, DISCRETE_EDIT_TYPES, coerceToSafeValue } from '@object-ui/fields'; import { useLocalization, useDisplayLocale, resolveFieldCurrency } from '@object-ui/i18n'; import { stateMachineNextValues, isFieldInlineEditable } from './inline-edit-options'; import { @@ -2483,7 +2489,8 @@ export const ObjectGrid: React.FC = ({ // Type-based cell renderer: explicit col type > objectDef type > heuristic inference. // Format hints (e.g. `text` + `format: 'phone'`) promote to the - // richer renderer (PhoneCellRenderer) via resolveCellRendererType. + // richer renderer (PhoneCellRenderer) via the grid's one shared + // resolve, `./cellRendererResolution` (objectui#8920). const objectDefField = objectSchema?.fields?.[col.field]; // ⭐ ANNOTATED, and the annotation is load-bearing (objectui#6004). // `objectSchema` is `useState`, so `objectDefField?.type` is @@ -2497,10 +2504,11 @@ export const ObjectGrid: React.FC = ({ // object-field fallback below is now the only road, which is what // every measured author already used. const formatHint = objectDefField?.format; - const inferredType: string | null = baseInferredType - ? resolveCellRendererType({ type: baseInferredType, format: formatHint }) - : null; - const CellRenderer = inferredType ? getCellRenderer(inferredType) : null; + // Both answers, from the one shared resolve (objectui#8920): + // `baseInferredType` is the DECLARED type the inline editor reads, + // `inferredType` the renderer key it promotes to. + const { rendererType: inferredType, Renderer } = resolveGridCellRendering({ type: baseInferredType, format: formatHint }); + const CellRenderer = inferredType ? Renderer : null; // Build field metadata for cell renderers with objectDef enrichment const fieldMeta: Record = { name: col.field, type: inferredType || 'text' }; @@ -2636,12 +2644,17 @@ export const ObjectGrid: React.FC = ({ const prefixConfig = col.prefix; if (prefixConfig?.field) { const baseCellRenderer = cellRenderer; - const PrefixRenderer = prefixConfig.type === 'badge' ? getCellRenderer('select') : null; + // ⭐ The one site whose contract is NOT "declared type + format + // hint": a FIXED registry key for the badge, owned by this + // component rather than by the prefixed field (objectui#8920). + // Named and routed through the same module so it reads as the + // declared exception it is, not as a fifth silent convention. + const PrefixRenderer = prefixConfig.type === 'badge' ? gridCellRendererForFixedKey(BADGE_PREFIX_RENDERER_KEY) : null; cellRenderer = (value: any, row: any) => { const prefixValue = row[prefixConfig.field]; const prefixEl = prefixValue != null && prefixValue !== '' ? PrefixRenderer - ? + ? : {String(prefixValue)} : null; return ( @@ -2698,16 +2711,29 @@ export const ObjectGrid: React.FC = ({ const rawHeader = rawFieldLabel || fieldName.charAt(0).toUpperCase() + fieldName.slice(1).replace(/_/g, ' '); const header = schema.objectName ? resolveFieldLabel(schema.objectName, fieldName, rawHeader) : rawHeader; - // Resolve type: objectDef type > heuristic inference (consistent with ListColumn path) - // Annotated for the same reason as path A's `baseInferredType` - // above: `fieldDef` is `any`, and an `any` reaching the `...(resolvedType - // && { type: resolvedType })` spread below collapses the emit literal - // to `any` (objectui#6004). - const resolvedType: string | null = fieldDef?.type || inferColumnType({ field: fieldName }) || null; - const CellRenderer = resolvedType ? getCellRenderer(resolvedType) : null; + // TWO resolves, two names (objectui#8920). "Resolve type" here means + // objectDef type > heuristic inference — WHICH TYPE THE FIELD HAS, + // and that is `declaredType`. The published second step, WHICH + // RENDERER THE TYPE MAPS TO, is `rendererType`; this path used to + // skip it entirely, so a `text` + `format: 'phone'` column got + // `TextCellRenderer` and the hint vanished with no diagnostic. + // A local called `resolvedType` holding only the FIRST answer is the + // trap that hid that for four of the six sites. + // + // The `string | null` annotation path A's `baseInferredType` needs + // (objectui#6004: `fieldDef` is `any`, and an `any` reaching the + // `...(declaredType && { type: declaredType })` spread below + // collapses the emit literal) now lives on `GridCellRendering`'s + // members — it moved into the helper's return type, it did not go + // away. + const { declaredType, rendererType, Renderer } = resolveGridCellRendering({ + type: fieldDef?.type || inferColumnType({ field: fieldName }), + format: fieldDef?.format, + }); + const CellRenderer = rendererType ? Renderer : null; // Build field metadata with objectDef enrichment - const fieldMeta: Record = { name: fieldName, type: resolvedType || 'text' }; + const fieldMeta: Record = { name: fieldName, type: rendererType || 'text' }; if (fieldDef) { if (fieldDef.label) fieldMeta.label = fieldDef.label; if (fieldDef.currency) fieldMeta.currency = fieldDef.currency; @@ -2721,16 +2747,16 @@ export const ObjectGrid: React.FC = ({ // reads the schema def directly, see `renderCellEditor` (objectui#7154). applyRelationalMeta(fieldMeta, fieldDef as any); // Auto-generate select options from data when no options defined - if (resolvedType === 'select' && !fieldMeta.options) { + if (rendererType === 'select' && !fieldMeta.options) { const uniqueValues = Array.from(new Set(data.map(row => row[fieldName]).filter(Boolean))); fieldMeta.options = uniqueValues.map((v: any) => ({ value: v, label: humanizeLabel(String(v)) })); } - if ((resolvedType === 'select' || resolvedType === 'status') && (fieldDef as any)?.appearance != null) { + if ((rendererType === 'select' || rendererType === 'status') && (fieldDef as any)?.appearance != null) { fieldMeta.appearance = (fieldDef as any).appearance; } const numericTypes = ['number', 'currency', 'percent']; - const inferredAlign = resolvedType && numericTypes.includes(resolvedType) ? 'right' as const : undefined; + const inferredAlign = rendererType && numericTypes.includes(rendererType) ? 'right' as const : undefined; // Auto-link primary field (first column) to record detail const isPrimaryField = colIndex === 0; @@ -2768,9 +2794,12 @@ export const ObjectGrid: React.FC = ({ return { header, accessorKey: fieldName, - // Forward the resolved field type for the type-aware inline editor. - ...(resolvedType && { type: resolvedType }), - ...(schema.showColumnTypeIcons && resolvedType && { headerIcon: getTypeIcon(resolvedType) }), + // Forward the DECLARED type for the type-aware inline editor — the + // renderer key would make a `format`-hinted text column edit as a + // phone/currency control it never declared. Path A forwards + // `baseInferredType` for exactly this reason (objectui#8920). + ...(declaredType && { type: declaredType }), + ...(schema.showColumnTypeIcons && rendererType && { headerIcon: getTypeIcon(rendererType) }), ...(inferredAlign && { align: inferredAlign }), ...(cellRenderer && { cell: cellRenderer }), sortable: fieldDef?.sortable !== false, @@ -2859,13 +2888,18 @@ export const ObjectGrid: React.FC = ({ }); return fieldsToShow.map((fieldName) => { const fieldDef = objectSchema?.fields?.[fieldName]; - // Annotated for the same reason as paths A and B (objectui#6004). - const resolvedType: string | null = fieldDef?.type || inferColumnType({ field: fieldName }) || null; - const CellRenderer = resolvedType ? getCellRenderer(resolvedType) : null; + // The same two resolves as path B, through the same shared owner + // (objectui#8920) — and the same objectui#6004 annotation, now + // carried by `GridCellRendering`'s `string | null` members. + const { declaredType, rendererType, Renderer } = resolveGridCellRendering({ + type: fieldDef?.type || inferColumnType({ field: fieldName }), + format: fieldDef?.format, + }); + const CellRenderer = rendererType ? Renderer : null; const header = fieldDef?.label || fieldName.charAt(0).toUpperCase() + fieldName.slice(1).replace(/_/g, ' '); // Build field metadata with objectDef enrichment - const fieldMeta: Record = { name: fieldName, type: resolvedType || 'text' }; + const fieldMeta: Record = { name: fieldName, type: rendererType || 'text' }; if (fieldDef) { if (fieldDef.label) fieldMeta.label = fieldDef.label; if (fieldDef.currency) fieldMeta.currency = fieldDef.currency; @@ -2879,23 +2913,26 @@ export const ObjectGrid: React.FC = ({ // reads the schema def directly, see `renderCellEditor` (objectui#7154). applyRelationalMeta(fieldMeta, fieldDef as any); // Auto-generate select options from data when no options defined - if (resolvedType === 'select' && !fieldMeta.options) { + if (rendererType === 'select' && !fieldMeta.options) { const uniqueValues = Array.from(new Set(data.map(row => row[fieldName]).filter(Boolean))); fieldMeta.options = uniqueValues.map((v: any) => ({ value: v, label: humanizeLabel(String(v)) })); } - if ((resolvedType === 'select' || resolvedType === 'status') && (fieldDef as any)?.appearance != null) { + if ((rendererType === 'select' || rendererType === 'status') && (fieldDef as any)?.appearance != null) { fieldMeta.appearance = (fieldDef as any).appearance; } const numericTypes = ['number', 'currency', 'percent']; - const inferredAlign = resolvedType && numericTypes.includes(resolvedType) ? 'right' as const : undefined; + const inferredAlign = rendererType && numericTypes.includes(rendererType) ? 'right' as const : undefined; return { header, accessorKey: fieldName, - // Forward the resolved field type for the type-aware inline editor. - ...(resolvedType && { type: resolvedType }), - ...(schema.showColumnTypeIcons && resolvedType && { headerIcon: getTypeIcon(resolvedType) }), + // Forward the DECLARED type for the type-aware inline editor — the + // renderer key would make a `format`-hinted text column edit as a + // phone/currency control it never declared. Path A forwards + // `baseInferredType` for exactly this reason (objectui#8920). + ...(declaredType && { type: declaredType }), + ...(schema.showColumnTypeIcons && rendererType && { headerIcon: getTypeIcon(rendererType) }), ...(inferredAlign && { align: inferredAlign }), ...(CellRenderer && { cell: (value: any) => }), sortable: fieldDef?.sortable !== false, @@ -2980,9 +3017,12 @@ export const ObjectGrid: React.FC = ({ && !perms.checkField(schema.objectName, fieldName, 'read')) return; // Annotated for the same reason as paths A-C (objectui#6004): `field` is - // `any`, so this value has to be named before it reaches a spread below. - const fieldType: string | undefined = field.type; - const CellRenderer = getCellRenderer(field.type); + // `any`, so these values have to be named before they reach a spread + // below — the naming now lives on `GridCellRendering`'s `string | null` + // members. `fieldType` is the DECLARED type the emit forwards to the + // inline editor; the renderer comes from the `format`-promoted key, which + // this path used to skip (objectui#8920). + const { declaredType: fieldType, rendererType, Renderer: CellRenderer } = resolveGridCellRendering(field); const numericTypes = ['number', 'currency', 'percent']; const translatedField = field.options ? { ...field, options: translateOptions(schema.objectName, fieldName, field.options) } @@ -2993,7 +3033,9 @@ export const ObjectGrid: React.FC = ({ accessorKey: fieldName, // Forward the field type for the type-aware inline editor. ...(fieldType && { type: fieldType }), - ...(numericTypes.includes(field.type) && { align: 'right' as const }), + // Aligned on the RENDERER key, like paths A-C: a `text` column with + // `format: 'currency'` renders as currency, so it aligns as currency. + ...(!!rendererType && numericTypes.includes(rendererType) && { align: 'right' as const }), cell: (value: any) => , sortable: field.sortable !== false, }); @@ -4433,11 +4475,17 @@ export const ObjectGrid: React.FC = ({ // Use objectSchema field type for type-aware rendering const fieldDef = objectSchema?.fields?.[key]; - if (fieldDef?.type) { - const CellRenderer = getCellRenderer(fieldDef.type); - if (CellRenderer) { - return ; - } + // Through the shared resolve, so the panel honours a `format` hint the + // same way the row above it does (objectui#8920). `rendererType` is null + // exactly when the key has no declared type, which is the guard this + // used to spell as `if (fieldDef?.type)`. The old inner + // `if (CellRenderer)` was DEAD — `getCellRenderer` ends in + // `standardMap[key] || TextCellRenderer` and never returns anything + // falsy — and `GridCellRendering.Renderer` states that totality in the + // type, so the dead branch goes with it. + const { rendererType, Renderer } = resolveGridCellRendering(fieldDef); + if (rendererType) { + return ; } // Fallback: infer from value and key name diff --git a/packages/plugin-grid/src/__tests__/cellRendererResolutionBoundary-8920.test.ts b/packages/plugin-grid/src/__tests__/cellRendererResolutionBoundary-8920.test.ts new file mode 100644 index 0000000000..4fcb66cc32 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/cellRendererResolutionBoundary-8920.test.ts @@ -0,0 +1,165 @@ +/** + * 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. + */ + +/** + * objectui#8920 — `plugin-grid` reaches a cell renderer through ONE module. + * + * ## Why a source scan rather than a rendering test + * + * `formatHintedColumnRenderer-8920.test.tsx` renders every path and proves the + * hint is honoured today. It cannot see the property this file guards: a + * SEVENTH call site added tomorrow, spelled `getCellRenderer(field.type)`, + * renders perfectly for every type that carries no `format` hint — which is + * most of them — and nothing observable distinguishes it from a correct one + * until an author writes the hint. That is exactly how four sites survived: the + * defect is *which resolve was spelled*, and the source is where that is + * measurable. + * + * The bound is therefore structural: `packages/plugin-grid/src` calls + * `getCellRenderer` / `resolveCellRendererType` from `cellRendererResolution.ts` + * and nowhere else. Adding a site is still easy; adding one that picks its own + * convention is not. + * + * ## Anti-vacuity — the expected answer OUTSIDE the helper is ZERO + * + * A zero and a broken scanner render identically, so three controls guard it, + * each able to fail on its own: + * + * 1. the population is enumerated and asserted non-trivial BY COUNT, and the + * helper module and `ObjectGrid.tsx` are both asserted present in it — + * a scan over an empty or wrong directory fails here, loudly; + * 2. the matcher is proved able to find a direct call on a synthetic input + * whose answer is known, in both directions; + * 3. the matcher is proved able to find direct calls in a REAL file at real + * scale — necessarily `cellRendererResolution.ts`, since it is now the + * only place they may appear. ⛔ If that module ever stops calling them, + * do not delete this control: the bound it guards has gone with it and + * this whole file needs rewriting, which is what its failure will say. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +// packages/plugin-grid/src/__tests__ -> packages/plugin-grid/src +const SRC = path.resolve(here, '..'); +const HELPER = 'cellRendererResolution.ts'; +const GRID = 'ObjectGrid.tsx'; + +/** + * Every non-test source file under `packages/plugin-grid/src`, as repo-relative + * paths from `src`. Tooling directories are excluded the way this repo spells + * that exclusion everywhere else — by DIRECTORY (`__tests__`, `__mocks__`), not + * by filename pattern — plus the `*.test.*` files that sit beside their + * subjects in this package. + */ +function sourceFiles(dir = SRC, prefix = ''): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const rel = prefix ? `${prefix}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + if (entry.name === '__tests__' || entry.name === '__mocks__' || entry.name === '__benchmarks__') continue; + out.push(...sourceFiles(path.join(dir, entry.name), rel)); + continue; + } + if (!/\.(ts|tsx)$/.test(entry.name)) continue; + if (/\.test\.tsx?$/.test(entry.name)) continue; + out.push(rel); + } + return out; +} + +/** + * The symbol immediately followed by `(` — a CALL SHAPE, wherever it appears. + * + * ⚠️ Deliberately NOT comment-aware, and the bound is stated to match: prose + * inside the guarded files may NAME either resolver but must not spell it with + * a trailing `(`. Two reasons this is the right trade rather than a shortcut. + * A comment stripper written for TypeScript has to tokenise strings, templates + * and regex literals to be correct, and every wrong version of it fails toward + * a FALSE NEGATIVE — a real call swallowed by a mis-parsed line, reported as a + * clean zero, which is precisely the reading this file exists to make + * trustworthy. And the rule it costs is one this card wants anyway: a comment + * that spells `getCellRenderer(field.type)` inside the grid is re-teaching the + * convention that dropped the hint. Name it without the parenthesis. + * + * The helper module is exempt by construction — it is scanned only by control + * 3, which reads the SET of symbols found, so its prose may quote the calls it + * is documenting. + */ +const DIRECT_CALL_RE = /\b(getCellRenderer|resolveCellRendererType)\s*\(/g; + +function directCalls(source: string): string[] { + return [...source.matchAll(DIRECT_CALL_RE)].map((m) => m[1]); +} + +const FILES = sourceFiles(); +const read = (rel: string) => readFileSync(path.join(SRC, rel), 'utf8'); + +describe('objectui#8920 — one owner for the grid\'s cell-renderer resolution', () => { + it('the population is enumerated and non-trivial (anti-vacuity control 1)', () => { + // Every "no such call" claim below is worthless if this list is empty or + // points at the wrong tree, so it is asserted first and BY COUNT. + expect(FILES.length).toBeGreaterThan(20); + expect(FILES).toContain(HELPER); + expect(FILES).toContain(GRID); + expect(read(GRID).length).toBeGreaterThan(100_000); + }); + + it('the matcher finds a direct call, and only a call (anti-vacuity control 2)', () => { + expect(directCalls('const R = getCellRenderer(t);')).toEqual(['getCellRenderer']); + expect(directCalls('resolveCellRendererType({ type, format })')).toEqual(['resolveCellRendererType']); + // A bare mention is not a call shape — this is the direction that keeps the + // bound satisfiable while the guarded source still explains itself in prose. + expect(directCalls('// getCellRenderer is deliberately not imported here')).toEqual([]); + expect(directCalls('import { getCellRenderer } from "@object-ui/fields";')).toEqual([]); + // …and the documented cost, asserted so nobody has to rediscover it from a + // confusing failure: parenthesised prose DOES count. + expect(directCalls('// was: getCellRenderer(field.type)')).toEqual(['getCellRenderer']); + }); + + it('the matcher finds real calls at real scale (anti-vacuity control 3)', () => { + // The helper is now the only place these calls may appear, so it is the + // only real-file control available — and that is not a weakness: if it + // stops calling them, the bound below has nothing left to mean. + // The SET, not the multiset: the helper's own documentation quotes the two + // calls it wraps, and counting those quotations would pin prose. + expect( + [...new Set(directCalls(read(HELPER)))].sort(), + `Anti-vacuity control 3 has lost its anchor: \`${HELPER}\` no longer calls ` + + 'either published resolver, so the matcher is not proved able to find ' + + 'anything at real scale and every "no direct call" claim in this file ' + + 'is a green no-op. ⛔ Do not delete this control — rewrite the bound ' + + 'it guards (objectui#8920).', + ).toEqual(['getCellRenderer', 'resolveCellRendererType']); + }); + + it('⭐ ObjectGrid.tsx makes no direct resolver call', () => { + // The card's own subject: six sites in this one file, three conventions. + expect( + directCalls(read(GRID)), + 'ObjectGrid.tsx called a published resolver directly. That is how a ' + + '`format`-hinted textual column lost its renderer in four of six ' + + 'sites (objectui#8920): `getCellRenderer(field.type)` type-checks and ' + + 'renders correctly for every unhinted type, so nothing else can catch ' + + 'it. Route through `./cellRendererResolution` instead.', + ).toEqual([]); + }); + + it('⭐ no file under plugin-grid/src outside the helper makes one either', () => { + const offenders = FILES.filter((f) => f !== HELPER).filter((f) => directCalls(read(f)).length > 0); + expect( + offenders, + 'These files resolve a cell renderer without going through ' + + `\`${HELPER}\`. One owner is what stops a seventh site from picking a ` + + 'convention of its own (objectui#8920).', + ).toEqual([]); + }); +}); diff --git a/packages/plugin-grid/src/__tests__/formatHintedColumnRenderer-8920.test.tsx b/packages/plugin-grid/src/__tests__/formatHintedColumnRenderer-8920.test.tsx new file mode 100644 index 0000000000..7b13667898 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/formatHintedColumnRenderer-8920.test.tsx @@ -0,0 +1,264 @@ +/** + * 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. + */ + +/** + * objectui#8920 — a `format`-hinted textual column must reach its renderer + * through EVERY path `ObjectGrid` can pick, not just the one that happened to + * apply the published two-step. + * + * ## The defect + * + * `@object-ui/fields` publishes `getCellRenderer(resolveCellRendererType(f))`. + * Skip the second step and a `text` + `format: 'phone'` column resolves to + * `TextCellRenderer`, which destructures only `value` and therefore cannot read + * `field.format`. Nothing throws, nothing warns: the cell just renders plain + * truncated text where a `tel:` anchor belongs. Six sites in `ObjectGrid.tsx` + * resolved a renderer with three conventions and only ONE of them ran the + * promotion, so which of the grid's paths honoured an author's `format` hint + * depended on how the columns had been declared. + * + * ## Why a RENDERING test, and why one case per path + * + * `getCellRenderer(field.type)` type-checks — `field.type` is a `string` and + * the parameter is a `string` — so every type-level and doc-level gate reads + * the broken sites green. **A green that cannot go red on this defect is not + * evidence.** The only oracle that can fail is rendered output: which renderer + * drew the cell, observed as whether a `tel:` anchor exists. + * + * And one case is not enough. A single case through one path proves the shared + * helper works; it says nothing about whether the divergence is gone. The + * divergence IS the defect, so the population — every path — is the subject, + * and each path gets its own case. + * + * ## The control, and why it is phone-SHAPED + * + * Every case carries an unhinted control column, `note`, whose value is a + * second, DIFFERENT phone number. So: + * + * - the hinted assertion is `a[href="tel:"]` — present; + * - the control assertion is `a[href="tel:"]` — absent, while the + * control's text still renders. + * + * A control holding a non-phone string would only show that the fix did not + * reach it. This one shows the promotion is driven by the DECLARED `format` + * and not by the value's shape, which is the other way this could have been + * "fixed" and would have been worse. + * + * ## Reaching each path (measured, not presumed) + * + * `generateColumns()` picks its source with `normalizeColumns(schema.columns)` + * first, then an inline-data branch, then the object-schema branch: + * + * A `columns` as objects — `columns: [{ field }]` + * B `columns` as strings — `columns: ['work_phone']` + * C inline-data projection — no `columns`, `fields` present, `data` inline + * (`fields` is what keeps `rowKeysWouldOutrank + * SchemaPolicy` false once the schema lands) + * D object-schema policy — no `columns`, no `fields`, rows handed down + * E record-detail panel — `renderRecordDetail` -> `renderFieldValue` + * P compound-cell prefix — `col.prefix.type === 'badge'` + * + * ## PREDICTED before running, on the pre-fix tree: 4 red, 2 green-both-sides + * + * A and P were ALREADY correct — A is the one site that ran the promotion, and + * P asks for a fixed registry key with no field type to promote. Their cases + * are PINS (must-not-change), not evidence that anything was fixed; they are + * here because a shared helper that silently changed either of them would be a + * regression this file has to catch. B, C, D and E are the genuinely red ones. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider } from '@object-ui/react'; +import { ObjectGrid } from '../ObjectGrid'; + +registerAllFields(); + +beforeAll(() => { + if (!Element.prototype.hasPointerCapture) { + Element.prototype.hasPointerCapture = vi.fn(() => false) as any; + } + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +afterEach(() => cleanup()); + +/** The hinted column's value — the one that MUST become a `tel:` anchor. */ +const HINTED_PHONE = '+15551234567'; +/** The control column's value — phone-SHAPED, declared without a hint. */ +const CONTROL_PHONE = '+15559876543'; + +/** + * `work_phone` is the subject: a TEXTUAL base type carrying a `format` hint, + * which is exactly the pair `resolveCellRendererType` exists to promote. + * `note` is its control — same base type, same value shape, no hint. + */ +const CONTACT_SCHEMA = { + name: 'contacts', + label: 'Contact', + fields: { + name: { type: 'text', label: 'Name' }, + work_phone: { type: 'text', format: 'phone', label: 'Work Phone' }, + note: { type: 'text', label: 'Note' }, + stage: { + type: 'select', + label: 'Stage', + options: [{ value: 'new', label: 'New' }], + }, + }, +}; + +const ROWS = [ + { + id: 'c-1', + name: 'Alice', + work_phone: HINTED_PHONE, + note: CONTROL_PHONE, + stage: 'new', + }, +]; + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: ROWS, total: ROWS.length })), + getObjectSchema: vi.fn(async () => CONTACT_SCHEMA), + } as any; +} + +/** Every `tel:` href currently in the tree, in DOM order. */ +function telHrefs(root: HTMLElement): string[] { + return Array.from(root.querySelectorAll('a[href^="tel:"]')).map( + (a) => a.getAttribute('href') ?? '', + ); +} + +function renderGrid(schemaOverrides: Record, props: Record = {}) { + return render( + + + , + ); +} + +/** + * The assertion every path shares: the hinted column drew a `tel:` anchor, the + * unhinted control did not, and the control's value is still on screen (so the + * absence half cannot be satisfied by the column having vanished). + */ +async function expectHintHonoured(root: HTMLElement) { + await waitFor(() => expect(telHrefs(root)).toContain(`tel:${HINTED_PHONE}`)); + expect(telHrefs(root)).not.toContain(`tel:${CONTROL_PHONE}`); + expect(root.textContent).toContain(CONTROL_PHONE); +} + +describe('objectui#8920 — every ObjectGrid path honours a `format` hint', () => { + /** PATH A — object `columns`. PIN: already correct before the fix. */ + it('A: object `columns` (must-not-change pin — this path already resolved)', async () => { + const { container } = renderGrid({ + columns: [{ field: 'name' }, { field: 'work_phone' }, { field: 'note' }], + data: { provider: 'value', items: ROWS }, + }); + + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + await expectHintHonoured(container); + }); + + /** PATH B — string `columns`. */ + it('B: string `columns`', async () => { + const { container } = renderGrid({ + columns: ['name', 'work_phone', 'note'], + data: { provider: 'value', items: ROWS }, + }); + + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + await expectHintHonoured(container); + }); + + /** + * PATH C — the inline-data projection. + * + * `fields` (not `columns`) is what selects this branch and keeps it selected: + * `rowKeysWouldOutrankSchemaPolicy` is `!schemaFields && objectName && + * objectSchema`, so an authored projection is exactly the condition under + * which the inline path survives the schema landing. + */ + it('C: inline-data projection (`fields`, no `columns`)', async () => { + const { container } = renderGrid({ + fields: ['name', 'work_phone', 'note'], + data: { provider: 'value', items: ROWS }, + }); + + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + await expectHintHonoured(container); + }); + + /** + * PATH D — the object-schema default-columns policy. + * + * Rows handed down as a prop with NO authored projection: that is + * `rowKeysWouldOutrankSchemaPolicy === true`, which skips path C and lands + * here (objectui#6677's ordering). + */ + it('D: object-schema default columns (rows handed down, no projection)', async () => { + const { container } = renderGrid({}, { data: ROWS }); + + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + await expectHintHonoured(container); + }); + + /** + * PATH E — the record-detail panel. + * + * `work_phone` and `note` are deliberately NOT columns here, so the only + * place either value can reach the DOM is the panel: no table cell can + * satisfy these assertions on the panel's behalf. The panel renders through + * a portal, so it is read by test id rather than from `container`. + */ + it('E: record-detail panel', async () => { + renderGrid({ + columns: [{ field: 'name' }], + data: { provider: 'value', items: ROWS }, + navigation: { mode: 'drawer' }, + }); + + fireEvent.click(await screen.findByText('Alice')); + await waitFor(() => expect(screen.getByTestId('record-detail-panel')).toBeInTheDocument()); + await expectHintHonoured(screen.getByTestId('record-detail-panel')); + }); + + /** + * PATH P — the compound-cell prefix badge. PIN: this site asks for a FIXED + * registry key (`select`), not for a field's renderer, so there is nothing to + * promote and its output must be byte-identical across the fix. It is here + * because routing it through the shared module is what keeps it from reading + * as a convention nobody chose — and a shared module that quietly changed it + * would be a regression. + */ + it('P: compound-cell prefix badge still draws the select renderer (must-not-change pin)', async () => { + const { container } = renderGrid({ + columns: [{ field: 'name', prefix: { field: 'stage', type: 'badge' } }, { field: 'work_phone' }], + data: { provider: 'value', items: ROWS }, + }); + + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + // The badge renderer translates the option value to its label; a plain + // text prefix would print the raw `new`. + await waitFor(() => expect(container.textContent).toContain('New')); + // …and the hinted column in the same grid is still promoted. + expect(telHrefs(container)).toContain(`tel:${HINTED_PHONE}`); + }); +}); diff --git a/packages/plugin-grid/src/cellRendererResolution.ts b/packages/plugin-grid/src/cellRendererResolution.ts new file mode 100644 index 0000000000..5d8596238a --- /dev/null +++ b/packages/plugin-grid/src/cellRendererResolution.ts @@ -0,0 +1,146 @@ +/** + * 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. + */ + +/** + * THE GRID'S SINGLE ANSWER TO "WHICH CELL RENDERER?" (objectui#8920). + * + * ## The defect this module exists to close + * + * `@object-ui/fields` publishes a TWO-STEP resolve: + * `getCellRenderer(resolveCellRendererType(field))`. The second step exists + * because a field's DECLARED type is not always the renderer's key — a textual + * base type carrying a `format` hint (`phone`, `email`, `url`, `currency`, + * `percent`) maps to the richer renderer. Skip it and the cell gets + * `TextCellRenderer`, which destructures only `value` and therefore never reads + * `field.format`: the hint is dropped, nothing throws, nothing warns, and the + * column silently renders as plain truncated text instead of a `tel:` / + * `mailto:` / formatted value. + * + * `ObjectGrid` used to spell that resolve SIX times with THREE conventions — + * one two-step, four raw declared types, one fixed registry key. Patching the + * four to match the one leaves the divergence in place and lets a seventh site + * pick either convention, so the resolution itself lives here instead: every + * `getCellRenderer` call the grid makes is made from this module, and + * `ObjectGrid.tsx` no longer imports `getCellRenderer` at all. That invariant + * is pinned by `__tests__/cellRendererResolutionBoundary-8920.test.ts`. + * + * ## Two resolutions, two names — the trap that hid the defect + * + * The word "resolve" named two different things in this file's caller, and the + * collision is most of why four sites read as correct: + * + * - "resolve which TYPE the field has" — `objectDef type > heuristic + * inference`. That answer is `declaredType`, and it is what the inline + * editor and the emitted `type:` key read. + * - "resolve which RENDERER the type maps to" — the published two-step. + * That answer is `rendererType`, and it is what `getCellRenderer`, + * `fieldMeta.type`, the header icon and the numeric alignment read. + * + * A local called `resolvedType` that held only the first one reads exactly like + * a value the second step had already been applied to. Both are named here so + * no call site has to hold the distinction in its head. + */ + +import type React from 'react'; +import { + getCellRenderer, + resolveCellRendererType, + type CellRendererProps, +} from '@object-ui/fields'; + +/** + * A field-shaped input. Deliberately narrower than `FieldMetadata`: the two + * members below are the whole of what "declared type + format hint → renderer" + * reads, and naming them stops the `any` that every caller's `objectSchema` + * read carries (`useState`) from collapsing the emit literals downstream + * (objectui#6004). + */ +export interface GridCellFieldLike { + /** The field's DECLARED type, already through any heuristic inference. */ + type?: string | null; + /** The field's `format` hint, as the object definition declares it. */ + format?: string | null; +} + +/** Everything the grid needs to know about one column's renderer. */ +export interface GridCellRendering { + /** + * The DECLARED type, unpromoted — `null` when the column has none. This is + * what the inline editor reads, so a `text` + `format: 'phone'` column keeps + * editing as text. + */ + declaredType: string | null; + /** + * The RENDERER key — the declared type promoted by its `format` hint. + * `null` exactly when `declaredType` is `null`. + */ + rendererType: string | null; + /** + * The cell renderer for `rendererType`. + * + * TOTAL, mirroring `getCellRenderer` itself, which ends in + * `standardMap[key] || TextCellRenderer` and therefore never returns a falsy + * renderer. A caller that wants "no declared type ⇒ no type-aware cell" + * asks `rendererType`, not this — the two questions are different and the + * grid's paths answer them differently on purpose. + */ + Renderer: React.FC; +} + +/** + * Resolve one column's renderer from its declared type and `format` hint. + * + * ⚠️ A field with NO declared type does not promote, even when it carries a + * `format`. `resolveCellRendererType` would promote it — `''` is a member of + * its textual base set — but every grid path already treats "no type at all" + * as "no type-aware renderer", and reversing that is a behaviour change wider + * than the one objectui#8920 rules on. The bound is deliberate, not an + * oversight; the unresolvable key still lands on the text renderer below, + * byte-identical to the `getCellRenderer(field.type)` this replaced when + * `field.type` was absent. + */ +export function resolveGridCellRendering( + field: GridCellFieldLike | null | undefined, +): GridCellRendering { + const declaredType: string | null = field?.type || null; + const rendererType: string | null = + declaredType === null + ? null + : resolveCellRendererType({ type: declaredType, format: field?.format ?? undefined }); + return { + declaredType, + rendererType, + Renderer: getCellRenderer(rendererType ?? ''), + }; +} + +/** + * ⭐ THE ONE SITE WITH A DIFFERENT CONTRACT, NAMED SO IT IS NOT A FIFTH SILENT + * CONVENTION. + * + * The compound-cell prefix badge asks for a FIXED registry key, not for a + * field's renderer: `prefix.type === 'badge'` is the author saying "draw this + * neighbouring column's value as a badge", and the key it needs is a constant + * of this component, not anything the prefixed field declares. There is no + * declared type to promote and no `format` to read, so + * `resolveGridCellRendering` cannot own it without misrepresenting the call as + * a field resolution. + * + * It routes through this module anyway, and that is the point: the grid has + * exactly two ways to reach a cell renderer, both of them here, both of them + * documented — rather than one documented way plus a bare `getCellRenderer` + * call that reads like a fifth convention nobody chose. + */ +export function gridCellRendererForFixedKey( + rendererType: string, +): React.FC { + return getCellRenderer(rendererType); +} + +/** The registry key the compound-cell prefix badge draws with. */ +export const BADGE_PREFIX_RENDERER_KEY = 'select';