From 8648b9081a584e979c3a4a3d1ece57f48a87a371 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 18:39:16 +0000 Subject: [PATCH] refactor(components): bind SortDirection to @objectstack/spec (objectui#7265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DataTable renderer declared `SortDirection` module-locally, under the exact name `@objectstack/spec/shared` exports — the planted-premise class that `check:spec-symbols` exists to stop. The third member it carried, `null`, made the route question live: measured at the resolved pin @objectstack/spec@17.4.0, the spec's `SortDirection` is the two-member direction and nothing else. Reading the site settled it toward BIND rather than RENAME. `null` is not a third direction, it is the absence of one: written at exactly one place (the end of the client-side header cycle in `handleSort`) and read only by guards that guard `sortColumn` in the same breath — and `sortColumn`, the other half of the same state pair, already spelled its own empty case as `useState` at the slot rather than folding it into a type name. So the strongest BIND form applied: the type is imported outright, the local declaration is gone rather than derived, and the third state is confined to the one state slot that carries it with the reason written there. `applySort` held a second inline hand copy of the same two members and now takes the bound type. Type-only: an `import type`, a deleted alias and an erased `useState` type argument. The declaration was never exported, so nothing published moves. Ledger regenerated with `--ledger`; the block shrank by this package's group. Ratchets: the spec-side properties the binding rests on are appended to this package's existing spec-symbol parity file, and the site plus the block are pinned in scripts/__tests__/spec-symbol-ledger-components-7265.test.ts. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_011QreXiyMEqKLN4U5daMPVa --- .../7265-components-sort-direction-bound.md | 17 ++ .../share-filter-sort-spec-parity.test.ts | 81 ++++++- .../src/renderers/complex/data-table.tsx | 22 +- ...spec-symbol-ledger-components-7265.test.ts | 200 ++++++++++++++++++ scripts/check-spec-symbol-derivation.mjs | 27 ++- 5 files changed, 339 insertions(+), 8 deletions(-) create mode 100644 .changeset/7265-components-sort-direction-bound.md create mode 100644 scripts/__tests__/spec-symbol-ledger-components-7265.test.ts diff --git a/.changeset/7265-components-sort-direction-bound.md b/.changeset/7265-components-sort-direction-bound.md new file mode 100644 index 0000000000..74df1be44c --- /dev/null +++ b/.changeset/7265-components-sort-direction-bound.md @@ -0,0 +1,17 @@ +--- +--- + +Bind `@object-ui/components`' `SortDirection` to `@objectstack/spec` (objectui#7265, +the `@object-ui/components` slice of the `check:spec-symbols` DEBT burn-down). + +The DataTable renderer declared `SortDirection` module-locally, under the exact +name `@objectstack/spec/shared` exports — the planted-premise class that guard +exists to stop. It now imports the spec's own type. The `null` third state the +declaration carried is not a third direction but the absence of one, so it is +spelled `| null` at the single state slot that holds it, the way `sortColumn` +already spelled its own empty case, with the reason written there. + +Type-only. `import type`, a deleted type alias and an erased `useState` type +argument leave the emitted JavaScript byte-identical and the package's `.d.ts` +unmoved — the declaration was never exported, so nothing published moves and no +package is released by this change. diff --git a/packages/components/src/__tests__/share-filter-sort-spec-parity.test.ts b/packages/components/src/__tests__/share-filter-sort-spec-parity.test.ts index acb35c373b..43bb40836a 100644 --- a/packages/components/src/__tests__/share-filter-sort-spec-parity.test.ts +++ b/packages/components/src/__tests__/share-filter-sort-spec-parity.test.ts @@ -29,6 +29,19 @@ * a tripwire built on `Object.keys(await import(…))` would pass while proving * nothing. So this reads each subpath's `.d.ts` through the TypeScript checker, * exactly as `scripts/check-spec-symbol-derivation.mjs` does. + * + * ## Later arrivals are APPENDED under their own card + * + * Six / four / two is a measurement of the batch-5 burn-down at the time it was + * taken; re-counting it here whenever a new symbol arrives would make it + * unreproducible. `SortDirection` (objectui#7265) is the first such arrival — a + * module-local mirror in the DataTable renderer that rule 1 could not see until + * its export filter was dropped. It went the BIND route, and because the + * declaration is now an import rather than a derivation there is no local name + * left to reach for: what is pinned below is the SPEC-side property the binding + * rests on, the way the app-shell slice pinned its own module-local names. The + * absence of the old declaration is pinned where the scanner can execute it, in + * scripts/__tests__/spec-symbol-ledger-components-7265.test.ts. */ import { describe, it, expect } from 'vitest'; @@ -45,8 +58,9 @@ import type { ShareLinkAudience as SpecShareLinkAudience, ShareLinkPermission as SpecShareLinkPermission, } from '@objectstack/spec/contracts'; -import type { SortItem as SpecSortItem } from '@objectstack/spec/shared'; +import type { SortItem as SpecSortItem, SortDirection as SpecSortDirection } from '@objectstack/spec/shared'; import type { FilterCondition as SpecFilterCondition } from '@objectstack/spec/data'; +import type { TableSortItem } from '@object-ui/types'; /** Every name `@objectstack/spec` exports from any subpath — types AND values. */ function specExportNames(): Set { @@ -241,3 +255,68 @@ describe('FilterBuilderCondition is NOT the spec ObjectQL AST', () => { expect(true).toBe(true); }); }); + +/* -------------------------------------------------------------------------- */ +/* objectui#7265 — `SortDirection`, the DataTable renderer's module-local */ +/* mirror. BOUND: the declaration is gone and the renderer imports the spec's */ +/* own type, with the third state confined to the state slot that carries it. */ +/* -------------------------------------------------------------------------- */ + +describe('the spec still owns `SortDirection` — the collision this route answers is real', () => { + it('reads the name off the spec, not off a list written here', () => { + expect( + SPEC_NAMES.has('SortDirection'), + '@objectstack/spec no longer exports `SortDirection`. The DataTable ' + + 'renderer imports it, so this is a build break rather than a silent ' + + 'drift — but it also means the burn-down that removed the local ' + + 'declaration no longer has a spec symbol to bind to, and the route has ' + + 'to be taken again.', + ).toBe(true); + }); +}); + +describe('the DataTable sort state IS the spec direction, widened only by absence', () => { + it('is pinned at compile time', () => { + // Guard-header cases 1, 2 and 2b: an `any` or `unknown` on the SPEC side + // answers every assignability question affirmatively, so binding to it would + // be a type-safety regression wearing a burn-down's clothes. + type _SpecNotAny = Assert, false>>; + type _SpecNotUnknown = Assert, false>>; + + // The third state is genuinely OUTSIDE the spec's vocabulary. This is the + // assertion the confinement rests on: if `null` were already admissible + // there, the `| null` at the renderer's state slot would be a no-op and the + // comment explaining it would be false. + type _NullIsNotADirection = Assert, false>>; + + // …and the two real members are, so the widening is by absence and nothing + // else. Read as an assignability pair rather than by restating `'asc' | + // 'desc'` here: a member list typed out in a test is the same hand copy the + // renderer just stopped keeping. + type _DirectionsSurviveTheWidening = Assert>; + type _WideningIsStrict = Assert, false>>; + + // The seam the non-null half flows into. `activeSort` builds a + // `TableSortItem` out of `sortDirection` verbatim, so the day the spec grows + // a direction this reds on the hand-declared `order` in @object-ui/types + // rather than on the renderer — which is where the drift would actually be. + // (@object-ui/types takes no dependencies and declares the shape instead of + // importing it; that is a documented decision of that package, and this is + // the tripwire on it.) + type _OrderIsTheSpecDirection = Assert>; + + expect(true).toBe(true); + }); + + it('the three-state cycle the `| null` exists for is covered behaviourally', () => { + // Stated here rather than re-tested: the client-side header cycle that + // PRODUCES the unsorted state is pinned by the `leaves client-side sorting + // exactly as it was` case in data-table-manual-sorting.test.tsx, and the + // manual-sorting side is pinned there too by `never asks for "no sort" — the + // third click returns to ascending`. Duplicating them here would make two + // places to keep honest; naming them makes the type pins above and the + // behaviour one file, which is what a reader deciding to "simplify" the + // `| null` away needs to find. + expect(true).toBe(true); + }); +}); diff --git a/packages/components/src/renderers/complex/data-table.tsx b/packages/components/src/renderers/complex/data-table.tsx index 95f7fba6a2..d992df3234 100644 --- a/packages/components/src/renderers/complex/data-table.tsx +++ b/packages/components/src/renderers/complex/data-table.tsx @@ -14,6 +14,7 @@ import { useGridFieldAuthoring } from '../../context/gridFieldAuthoring'; import { describeIgnoredBind, describeNonArrayData } from './dataTableBindDiagnostic'; import { ComponentRegistry, compareSortValues, evalRowPredicate, formatDate, formatDateTime, getSortValue } from '@object-ui/core'; import type { DataTableSchema, TableSortItem, TableColumnType } from '@object-ui/types'; +import type { SortDirection } from '@objectstack/spec/shared'; import { SchemaRenderer, toRenderableSchema, useRowPredicate, usePredicateScope } from '@object-ui/react'; import { createSafeTranslation } from '@object-ui/i18n'; import { @@ -62,8 +63,6 @@ import { DropdownMenuTrigger, } from '../../ui/dropdown-menu'; -type SortDirection = 'asc' | 'desc' | null; - /** * Inline-edit helpers: convert a stored cell value to the string a native * `` / `` expects, and back. @@ -928,7 +927,22 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { // State management const [searchQuery, setSearchQuery] = useState(''); const [sortColumn, setSortColumn] = useState(null); - const [sortDirection, setSortDirection] = useState(null); + // The sort state's second half. `SortDirection` is `@objectstack/spec`'s own + // export, imported rather than re-declared: this module used to hand-write + // `'asc' | 'desc' | null` under that exact export name, which is the planted- + // premise class `check:spec-symbols` exists to stop (objectui#7265). + // + // `null` is the ONE divergence, and it lives HERE rather than in the name + // because it is not a third direction — it is the absence of one, the + // unsorted end of the client-side header cycle in `handleSort`. That is the + // same "this half is empty" that `sortColumn` above already spells at its own + // slot, which is why folding it into a type would have been the odd one out. + // No `null` can reach the protocol's vocabulary: the sort comparator is past + // the `!sortDirection` guard in `sortedData`, and `activeSort` emits a + // `TableSortItem` only when both halves are set. The cycle that produces the + // third state is pinned by the `leaves client-side sorting exactly as it was` + // case in data-table-manual-sorting.test.tsx. + const [sortDirection, setSortDirection] = useState(null); const [selectedRowIds, setSelectedRowIds] = useState>(new Set()); const [currentPage, setCurrentPage] = useState(1); const [pageSize, setPageSize] = useState(initialPageSize); @@ -1279,7 +1293,7 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { * directly, which under `manualSorting` would have written to state nothing * reads: a menu item that highlights, closes, and changes nothing. */ - const applySort = (columnKey: string, order: 'asc' | 'desc') => { + const applySort = (columnKey: string, order: SortDirection) => { if (manualSorting) { onSortChange?.([{ field: columnKey, order }]); return; diff --git a/scripts/__tests__/spec-symbol-ledger-components-7265.test.ts b/scripts/__tests__/spec-symbol-ledger-components-7265.test.ts new file mode 100644 index 0000000000..536141eddb --- /dev/null +++ b/scripts/__tests__/spec-symbol-ledger-components-7265.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { SortDirectionEnum } from '@objectstack/spec/shared'; + +// Plain-JS CI helper. Its types are INFERRED from the .mjs source by +// `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here. +import { scanFile } from '../check-spec-symbol-derivation.mjs'; + +/** + * objectui#7265, the `@object-ui/components` slice -- the one name this package + * held in rule 1's DEBT block, in one file. + * + * Sibling of `spec-symbol-ledger-core-7265.test.ts`, + * `spec-symbol-ledger-app-shell-7265.test.ts` and + * `spec-symbol-ledger-types-7265.test.ts`, same two-part shape, because a ledger + * needs both halves: + * + * 1. THE SITE. The real scanner, run over the real file. This is the half that + * reds if a local copy comes back -- deleting a name from a ledger is not a + * burn-down unless the declaration went with it. + * 2. THE BLOCK. Shrink-only is the card's own invariant, so it is asserted, + * not just respected. Stated as a ceiling rather than an equality so the + * next slice can shrink it further without touching this file. + * + * ⭐ What is specific to THIS slice: the route was decided against a third + * state, not against a member count. The renderer declared `SortDirection` as + * the spec's two members plus `null`, and `null` is the one thing the spec's + * vocabulary refuses -- so the rename question was live, and the answer came + * from what that member MEANS at the site. It is the ABSENCE of a direction, + * written once at the end of the client-side header cycle and read only by + * guards that guard `sortColumn` in the same breath, and `sortColumn` -- the + * other half of the same state pair -- already spelled its own empty case at its + * slot. So the concept is the spec's, the declaration is gone rather than + * re-derived, and the third state is confined to the state slot. The refusal + * that makes the confinement load-bearing is probed below in BOTH directions. + * + * ⚠️ What this file does NOT assert, on purpose: that the surviving DEBT rows + * are still there. A pin on those would fail on the next slice's success -- it + * would red on healthy progress, which is the opposite of a ratchet. + * + * ⚠️ The scanner is only evidence if it can fail, so the site assertion is + * paired with a fixture of the same kind that it MUST flag. A green scan with an + * empty `specNames` map, or over a path that does not exist, looks exactly like + * a green scan over a burned-down site. + */ + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const gateSource = path.join(repoRoot, 'scripts/check-spec-symbol-derivation.mjs'); + +/** The name that was BURNED DOWN by binding it to the spec. */ +const BOUND = 'SortDirection'; +/** The subpath that owns it. */ +const SUBPATH = '@objectstack/spec/shared'; +/** The single site it lived at. */ +const SITE = 'packages/components/src/renderers/complex/data-table.tsx'; + +/** + * The names rule 1 matches on, built for the one name this slice dealt with + * rather than typed out as a list -- a hand-written name list would keep + * asserting a collision after the spec stopped exporting the symbol. + */ +const specNamesForBound = (): Map> => new Map([[BOUND, new Set([SUBPATH])]]); + +/** Reads one `const NAME = { … };` block out of the gate's source text. */ +function ledgerBlock(name: string): string { + const text = fs.readFileSync(gateSource, 'utf8'); + const start = text.indexOf(`const ${name} = {\n`); + expect(start, `${name} block not found in the gate source`).toBeGreaterThan(-1); + const end = text.indexOf('\n};\n', start); + expect(end, `${name} block is not terminated`).toBeGreaterThan(start); + return text.slice(start, end + 3); +} + +const ledgerNames = (block: string) => [...block.matchAll(/^ {4}"([^"]+)",$/gm)].map((m) => m[1]); + +describe('the spec still owns the name this slice bound to', () => { + it(`\`${BOUND}\` is still a live \`${SUBPATH}\` vocabulary`, () => { + // The `import` above already fails the build if the enum disappears. This + // asserts the stronger thing the route rests on -- that the name still names + // a PARSER, not a type-only shim that would make the probes below vacuous. + // (`SortDirection` itself is `z.input` of this enum and so is type-only; it + // is read through the compiler in the package's own parity file.) + expect(typeof SortDirectionEnum.safeParse).toBe('function'); + expect(SortDirectionEnum.safeParse('asc').success).toBe(true); + expect(SortDirectionEnum.safeParse('desc').success).toBe(true); + }); + + it('…and it refuses the third state, which is why `null` had to be confined', () => { + // Both directions of the one divergence. If the spec ever admits `null` as + // a direction, the confinement at the state slot is spent and the comment + // explaining it has gone false -- this is the assertion that says so out + // loud instead of staying quiet. + expect(SortDirectionEnum.safeParse(null).success).toBe(false); + // Lit control: a refusal is only news if the parser accepts anything at all, + // and the two accepted values above are read back off the enum rather than + // restated, so a widened vocabulary shows up here rather than in prose. + const members = Object.keys( + (SortDirectionEnum as unknown as { _zod?: { def?: { entries?: Record } } }) + ._zod?.def?.entries ?? {}, + ); + expect(members.length).toBeGreaterThan(1); + expect(members).not.toContain('null'); + }); +}); + +describe('the site: the module-local mirror is gone', () => { + const found = () => scanFile(path.join(repoRoot, SITE), specNamesForBound()).map((f: { name: string }) => f.name); + + it(`rule 1 no longer sees \`${BOUND}\` -- the renderer imports it now`, () => { + expect(found()).toEqual([]); + }); + + it('the scanner can still see the shape it used to flag -- the control', () => { + // Same kind as the subject: the exact declaration this slice deleted. + // Written to a throwaway path so the control cannot be satisfied by + // anything already in the tree. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'spec-symbol-components-7265-')); + try { + const fixture = path.join(dir, 'relapse.ts'); + fs.writeFileSync( + fixture, + [ + "type SortDirection = 'asc' | 'desc' | null;", + '', + 'export const used: SortDirection = null;', + '', + ].join('\n'), + ); + expect(scanFile(fixture, specNamesForBound()).map((f: { name: string }) => f.name)).toEqual([BOUND]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('…and a DERIVED re-declaration is NOT flagged -- the other half of the control', () => { + // This proves the scan judges DERIVATION rather than counting names: a + // scanner that flagged this would make the first assertion above unpassable, + // so its green would mean nothing. It also records what the gate does and + // does not decide -- the union-extension spelling below is a legitimate BIND + // form too, and the gate is indifferent between it and the outright import. + // The site chose the import; the gate did not choose for it. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'spec-symbol-components-7265-ok-')); + try { + const fixture = path.join(dir, 'derived.ts'); + fs.writeFileSync( + fixture, + [ + "import type { SortDirection as SpecSortDirection } from '@objectstack/spec/shared';", + '', + 'type SortDirection = SpecSortDirection | null;', + '', + 'export const used: SortDirection = null;', + '', + ].join('\n'), + ); + expect(scanFile(fixture, specNamesForBound()).map((f: { name: string }) => f.name)).toEqual([]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('the block: shrink-only, and it shrank by exactly this package', () => { + it(`DEBT no longer lists \`${BOUND}\``, () => { + expect(ledgerBlock('DEBT')).not.toContain(`"${BOUND}"`); + }); + + it('the whole `@object-ui/components` group is gone', () => { + expect(ledgerBlock('DEBT')).not.toContain('"@object-ui/components"'); + }); + + it('DEBT has not grown -- 2 names is the ceiling this slice left', () => { + // The `@object-ui/types` slice left 3. This one took one of them. + // Any future measurement above this number is the ratchet failing, whatever + // reason is given for it. + expect(ledgerNames(ledgerBlock('DEBT')).length).toBeLessThanOrEqual(2); + }); + + it('CLAIM_DEBT did not grow either -- this slice removed no rule 2 claim', () => { + // The `@object-ui/core` slice was FORCED to regenerate that block too, + // because the const it deleted carried a `Mirrors …` comment. The + // declaration this slice deleted carried no spec-alignment claim, so + // `--claim-ledger` regenerates byte-identically. If a later edit here moves + // this number, that coupling is back. + expect(ledgerNames(ledgerBlock('CLAIM_DEBT')).length).toBeLessThanOrEqual(18); + }); + + it('ALLOW did NOT gain a waiver -- the route was BIND, and that is a fact worth pinning', () => { + // The sibling slice put one of its two names in ALLOW instead, so "a name + // left DEBT" does not say which route was taken. This one took none: there + // is nothing here that deliberately differs from the spec's direction + // vocabulary. A later hand that waives it instead has to delete this + // assertion, which is the visibility a silent re-fork would not have. + expect(fs.readFileSync(gateSource, 'utf8')).not.toContain(`"@object-ui/components:${BOUND}"`); + }); +}); diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs index 18975c7827..6e76953280 100644 --- a/scripts/check-spec-symbol-derivation.mjs +++ b/scripts/check-spec-symbol-derivation.mjs @@ -924,10 +924,31 @@ const DEBT_ISSUE = 7265; // default }` preset dialect that the spec's strict ViewTabSchema rejects, and // binding it would 422 metadata that renders today. Both are pinned, in both // directions, by packages/types/src/__tests__/spec-symbol-parity.test.ts. +// +// Then the `@object-ui/components` slice at objectui#7265, where the route was +// decided by what the extra member MEANS rather than by how many members there +// were. The DataTable renderer declared `SortDirection` as `'asc' | 'desc' | +// null`, and that third state made the RENAME question genuinely live: the +// spec's `SortDirection` -- re-measured at the RESOLVED pin 17.4.0, on the +// `@objectstack/spec/shared` subpath, because byte-identical is a statement +// about a version and so is two-member -- is the direction and nothing else. +// Reading the site settled it the other way. `null` is not a third DIRECTION, +// it is the ABSENCE of one: it is written at exactly one place (the end of the +// client-side header cycle in `handleSort`) and read only by guards that guard +// `sortColumn` in the same breath -- and `sortColumn`, the other half of the +// same state pair, already spelled its own empty case as `useState` at the slot instead of folding it into a type name. So the concept IS +// the spec's, and the strongest BIND form was available: the type is now +// IMPORTED outright, the local declaration is gone rather than derived, and the +// third state is confined to the one state slot that carries it with the reason +// written there. `applySort`, one screen down, was a second hand copy of the +// same two members inline in its signature and now takes the bound type too. +// The spec-side properties the binding rests on are pinned in this package's +// existing spec-symbol file, packages/components/src/__tests__/ +// share-filter-sort-spec-parity.test.ts (appended to, not duplicated); the site +// and the block are pinned in +// scripts/__tests__/spec-symbol-ledger-components-7265.test.ts. const DEBT = { - "@object-ui/components": [ - "SortDirection", - ], "@object-ui/data-objectstack": [ "normalizeFilterOperator", ],