diff --git a/.changeset/5876-one-lookup-predicate.md b/.changeset/5876-one-lookup-predicate.md new file mode 100644 index 0000000000..a774fa8767 --- /dev/null +++ b/.changeset/5876-one-lookup-predicate.md @@ -0,0 +1,34 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +The dashboard package now holds ONE relation predicate instead of two that +agreed only because a sweep had just aligned them (objectui#5876). + +`computeLookupExpand` in `ObjectDataTable.tsx` carried its own `isLookup`, +byte-identical to the exported `isLookupType` in `recordFields.tsx` after +objectui#5692 pointed both at `@object-ui/core`'s `EXPANDABLE_FIELD_TYPES`. +Nothing kept them aligned: a future edit to either — a member added, the +retirement gate moved — would have re-forked the `$expand` decision from the +predicate whose docblock claims to drive it. `computeLookupExpand` now calls +`isLookupType`, which gains its first production consumer, and the module no +longer imports the shared family or the retirement gate at all. + +**No behaviour changes**, and that is measured rather than assumed: + +- The two bodies were identical, so every boolean answer — `tree` is expanded, + `reference` is not, ordinary relations are — is the same before and after. +- The retired-spelling warning is not emitted a different number of times. + `reportRetiredFieldType` dedupes per SPELLING in one module-level set inside + `@object-ui/core`, which both bodies already shared, so routing two callers + through one function cannot change the count. + +Nothing published moves: `isLookupType` is not re-exported from +`@object-ui/plugin-dashboard`'s entry, so this is internal shape only. + +Because a refactor with no observable delta cannot be pinned by a behavioural +test — a byte-identical local copy satisfies every assertion you can write +about `$expand` — the pin is identity, in +`__tests__/expandableFamily.identity-5692.test.ts`: `computeLookupExpand` is +observed CALLING `isLookupType`, and `ObjectDataTable.tsx` is read at source +level to confirm no second body survives for it to call instead. diff --git a/packages/plugin-dashboard/src/ObjectDataTable.tsx b/packages/plugin-dashboard/src/ObjectDataTable.tsx index ac36072848..02b03eb1f3 100644 --- a/packages/plugin-dashboard/src/ObjectDataTable.tsx +++ b/packages/plugin-dashboard/src/ObjectDataTable.tsx @@ -13,13 +13,6 @@ import { isDrillEnabled, columnIdentity, columnHeader, - // The retirement gate (objectui#4914, ruling B) — `@object-ui/fields` - // re-exports the same function object; read here from its home. - isRetiredFieldType, - reportRetiredFieldType, - // The reference-bearing field family (objectui#5692). Read, never copied — - // see the convergence note on `computeLookupExpand`. - EXPANDABLE_FIELD_TYPES, } from '@object-ui/core'; import type { DrillDownConfig } from '@object-ui/types'; import { Skeleton, RefreshIndicator, cn } from '@object-ui/components'; @@ -30,6 +23,9 @@ import { renderFieldValue, isNumericFieldMeta, isSystemField, + // The package's single relation predicate (objectui#5876). The retirement + // gate and the family read live in ITS body — never restated here. + isLookupType, } from './recordFields'; import { RecordDetailDrawer } from './RecordDetailDrawer'; @@ -176,8 +172,10 @@ export function normalizeColumns(columns: (string | Record)[]): Nor /** * Compute the list of lookup-typed accessors that should be expanded when * fetching rows. Returns column accessors whose object schema field type is - * a relation. Which types those are is NOT restated here: it is - * {@link EXPANDABLE_FIELD_TYPES}, the family `@object-ui/core` publishes. Used + * a relation. Neither the type family nor the test itself is restated here: + * this delegates to {@link isLookupType} in `recordFields.tsx`, the package's + * single relation predicate, which reads `EXPANDABLE_FIELD_TYPES` — the family + * `@object-ui/core` publishes. Used * by the dashboard table widget to ask the data adapter to populate referenced * records (e.g. `account: { id, name }`) so cells don't show raw FK ids. * @@ -211,6 +209,19 @@ export function normalizeColumns(columns: (string | Record)[]): Nor * data: the spelling is absent from `@objectstack/spec`'s closed `FieldType` * and refused by `FieldSchema.safeParse`, so no object schema can declare a * field whose stored type is `reference`. + * + * ## One predicate, not two that agree by coincidence (objectui#5876) + * + * This function used to carry its own `isLookup`, byte-identical to + * `isLookupType` once objectui#5692 had pointed both at the same set — two + * bodies that agreed because one sweep aligned them, with nothing keeping them + * aligned afterwards. The test IS `isLookupType` now, so this module no longer + * IMPORTS the shared family or the retirement gate and no longer CALLS either + * (they are named in this prose and nowhere else in the file). That absence is + * the assertion: a BEHAVIOURAL test cannot see this change, because a + * byte-identical local copy satisfies every boolean claim you can make about + * `$expand`. The pin that can see it is the identity pin in + * `__tests__/expandableFamily.identity-5692.test.ts`. */ export function computeLookupExpand( schema: { columns?: any[]; objectName?: string }, @@ -223,17 +234,6 @@ export function computeLookupExpand( } else { for (const [name, def] of Object.entries(objectSchema.fields)) fieldsByName[name] = { name, ...(def as any) }; } - const isLookup = (t: unknown) => { - if (typeof t === 'string' && isRetiredFieldType(t)) { - reportRetiredFieldType(t); - return false; - } - // Never `new Set([...EXPANDABLE_FIELD_TYPES, …])` and never a re-listing of - // its members: a copy re-forks the table, which is the defect this removed, - // and the identity pin fails on it by design. - return EXPANDABLE_FIELD_TYPES.has(t as string); - }; - const cols = Array.isArray(schema.columns) ? schema.columns : []; const out = new Set(); @@ -251,14 +251,14 @@ export function computeLookupExpand( .filter(Boolean); for (const acc of accessors) { const def = fieldsByName[acc]; - if (def && isLookup(def.type)) out.add(acc); + if (def && isLookupType(def.type)) out.add(acc); } } else { // No columns whitelist (auto-derive mode, e.g. drill-down drawer): // expand every lookup-type field known from the schema so cells show // the related record's display name instead of a bare FK id. for (const [name, def] of Object.entries(fieldsByName)) { - if (isLookup((def as any)?.type)) out.add(name); + if (isLookupType((def as any)?.type)) out.add(name); } } return Array.from(out); diff --git a/packages/plugin-dashboard/src/__tests__/expandableFamily.identity-5692.test.ts b/packages/plugin-dashboard/src/__tests__/expandableFamily.identity-5692.test.ts index 78c0da40e3..08334294dc 100644 --- a/packages/plugin-dashboard/src/__tests__/expandableFamily.identity-5692.test.ts +++ b/packages/plugin-dashboard/src/__tests__/expandableFamily.identity-5692.test.ts @@ -45,7 +45,29 @@ * pin goes red too and its `reference` pin flips; the ordinary-relation * regression controls stay GREEN in both directions, which is what makes them * controls rather than duplicates of the pins. + * + * ## objectui#5876 — one predicate, and why NO behavioural test can pin it + * + * #5692 left the package with TWO byte-identical bodies: `isLookupType` here + * and a local `isLookup` inside `computeLookupExpand`. Collapsing the second + * into the first is observationally FREE — every boolean claim about `$expand` + * (`tree` expands, `reference` does not, ordinary relations do) answers the + * same before and after, so every assertion in the three describes above would + * stay GREEN on a revert. They are controls for this change, not pins of it. + * The `reportRetiredFieldType` count does not move either: its dedupe key is + * the SPELLING, in one module-level Set inside `@object-ui/core`, which both + * bodies already shared (`retired-field-types.ts` — "the dedupe is per + * SPELLING, not per face"). + * + * So the pin below is IDENTITY at the call level, in two halves that fail for + * different reasons: `computeLookupExpand` must be observed CALLING + * `isLookupType`, and `ObjectDataTable.tsx` must hold no second body for it to + * call instead. Predicted ablation: restore the local `isLookup` and BOTH go + * RED while everything above stays green. */ +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import { describe, it, expect, vi } from 'vitest'; import { EXPANDABLE_FIELD_TYPES } from '@object-ui/core'; import { FieldType } from '@objectstack/spec/data'; @@ -115,6 +137,72 @@ describe("the dashboard's relation rule is core's object, not a copy (objectui#5 }); }); +describe('one relation predicate, not two that agree by coincidence (objectui#5876)', () => { + it('`computeLookupExpand` CALLS `isLookupType` — in BOTH column modes', () => { + // Surgical by construction: `vi.doMock` is not hoisted, so it binds only + // the dynamic import below and leaves every other test in this file on the + // real module. The double delegates to the real implementation, so the + // exercise still answers correctly — what is being read is WHO answered. + return (async () => { + const actual = await import('../recordFields'); + const double = vi.fn(actual.isLookupType); + vi.resetModules(); + vi.doMock('../recordFields', () => ({ ...actual, isLookupType: double })); + try { + const { computeLookupExpand: subject } = await import('../ObjectDataTable'); + const modes: [string, () => string[]][] = [ + ['explicit whitelist', () => subject({ columns: ALL_COLUMNS }, objectSchema())], + ['auto-derive', () => subject({}, objectSchema())], + ]; + for (const [label, exercise] of modes) { + double.mockClear(); + // Control: the mode still answers, so an assertion about WHO was + // asked cannot pass on a subject that did nothing at all. + expect(exercise(), `${label} expanded nothing`).toContain('account'); + expect( + double.mock.calls.map(([t]) => t), + `${label} answered the relation question without asking isLookupType`, + ).toContain('lookup'); + } + } finally { + vi.doUnmock('../recordFields'); + vi.resetModules(); + } + })(); + }); + + it('`ObjectDataTable.tsx` holds no second predicate body', () => { + // The call pin above can be satisfied while a dead copy still sits in the + // file; this half is what makes "one predicate" true of the SOURCE. Read + // with comments stripped, so the prose that NAMES these symbols in the + // convergence note cannot fake a hit. + const code = readFileSync( + path.join(path.dirname(fileURLToPath(import.meta.url)), '..', 'ObjectDataTable.tsx'), + 'utf8', + ) + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/\/\/[^\n]*/g, ''); + + // Controls — chosen to be INVARIANT under the ablation this pin guards + // against, so that a mis-resolved path or an over-eager stripper fails + // HERE, as a broken probe, while a genuine revert fails on the subject + // lines below with an honest message. (Measured: an earlier draft used the + // `isLookupType` call count as its control, and a real revert then reported + // itself as "probe stripped the code away" — the two failures were + // indistinguishable, which is the whole thing a control exists to prevent.) + expect(code, 'probe read the wrong file').toContain('export function computeLookupExpand('); + expect(code, 'probe stripped the code away').toContain('out.add(acc)'); + + // Subject — each of these moves if the local copy comes back. + expect( + code.match(/isLookupType\(/g) ?? [], + 'computeLookupExpand stopped calling the shared predicate', + ).toHaveLength(2); + expect(code, 'a second family read lives here').not.toContain('EXPANDABLE_FIELD_TYPES'); + expect(code, 'a second retirement gate lives here').not.toContain('RetiredFieldType'); + }); +}); + describe('the ordinary relations are untouched — regression control', () => { // These must stay green through BOTH ablation legs. If they move, the // convergence took the whole whitelist with it and the pins above are