diff --git a/.changeset/rotten-plums-smash.md b/.changeset/rotten-plums-smash.md new file mode 100644 index 000000000..d8c958253 --- /dev/null +++ b/.changeset/rotten-plums-smash.md @@ -0,0 +1,6 @@ +--- +--- + +`@object-ui/plugin-list` now type-checks its 31 test files: `tsconfig.test.json` is chained from its `type-check` script and its `TEST_DEBT` entry is gone (#4040). + +Test-side and build-config only — no package source changed, so nothing is released by this. diff --git a/packages/plugin-list/package.json b/packages/plugin-list/package.json index fdcf00d85..20302fe60 100644 --- a/packages/plugin-list/package.json +++ b/packages/plugin-list/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit && tsc -p tsconfig.typetests.json", + "type-check": "tsc --noEmit && tsc -p tsconfig.typetests.json && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/plugin-list/src/__tests__/ListView.emptyStateFilteredCopy.test.tsx b/packages/plugin-list/src/__tests__/ListView.emptyStateFilteredCopy.test.tsx index 6ef3ea2ac..3f0e6c77a 100644 --- a/packages/plugin-list/src/__tests__/ListView.emptyStateFilteredCopy.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.emptyStateFilteredCopy.test.tsx @@ -64,13 +64,38 @@ const BASE: ListViewSchema = { fields: ['name'], }; +/** + * A `list-view` node as an AUTHOR writes it, before `normalizeListViewSchema` + * folds the legacy vocabulary onto the spec's (#2890). + * + * `ListViewSchema` describes the CANONICAL surface, so it types `filter` as the + * spec's `ViewFilterRule[]` (`{ field, operator, value }` objects). What + * objectui actually stores and passes to `$filter` is an ObjectQL FilterNode + * array — `[['status', 'not_in', ['archived']]]`, the form used below and the + * form the issue's repro carries. That divergence is stated outright in the + * normalizer's own contract note ("the spec types `filter` as `ViewFilterRule[]` + * … so objectui's field is typed from the spec but used as something else … + * That mismatch is real and out of scope here"), and it is the reason these + * fixtures cannot be re-spelled into the declared shape: re-spelling them would + * stop testing the filter form the renderer actually receives. + * + * So the assertion is deliberately made about pre-normalization input, and the + * cast says exactly that rather than widening `ListViewSchema` to admit both — + * widening is what would re-fork the vocabulary #2890 unified. The legacy input + * vocabulary has no declared type of its own today; filed as #4337. + */ +const authored = (node: Record): ListViewSchema => + node as unknown as ListViewSchema; + describe('ListView empty state — a filtered view says it is filtered (#4155)', () => { it('a view filtered to empty says "no matching records", not "nothing here yet"', async () => { - const panel = await emptyState({ - ...BASE, - // The source-declared filter from the issue's repro. - filter: [['status', 'not_in', ['archived', 'deleted']]], - } as ListViewSchema); + const panel = await emptyState( + authored({ + ...BASE, + // The source-declared filter from the issue's repro. + filter: [['status', 'not_in', ['archived', 'deleted']]], + }), + ); expect(panel.textContent).toMatch(/No matching records/i); expect(panel.textContent).not.toMatch(/Nothing here yet/i); @@ -91,11 +116,13 @@ describe('ListView empty state — a filtered view says it is filtered (#4155)', }); it("the author's own emptyState copy still wins over both", async () => { - const panel = await emptyState({ - ...BASE, - filter: [['status', 'not_in', ['archived']]], - emptyState: { title: 'No open work orders', message: 'Dispatch one to get started.' }, - } as ListViewSchema); + const panel = await emptyState( + authored({ + ...BASE, + filter: [['status', 'not_in', ['archived']]], + emptyState: { title: 'No open work orders', message: 'Dispatch one to get started.' }, + }), + ); expect(panel.textContent).toMatch(/No open work orders/); expect(panel.textContent).not.toMatch(/No matching records/i); diff --git a/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx b/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx index 0a515ed40..a71762bf3 100644 --- a/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx @@ -29,8 +29,12 @@ import type { ObjectPermissionConfig, RoleDefinition } from '@object-ui/types'; * register table renderers), but the $select contract is invariant. */ +// `permissions: []` is accurate and required: a role's DIRECT object grants live +// on `RoleDefinition.permissions`, and this role has none — every grant it uses +// comes from the `ObjectPermissionConfig` below. (That the field is required and +// read by nothing is the dormancy filed as #4288.) const roles: RoleDefinition[] = [ - { name: 'restricted', description: 'denies one field' }, + { name: 'restricted', label: 'Restricted', description: 'denies one field', permissions: [] }, ]; function makeRestrictedConfig(deniedField: string): ObjectPermissionConfig { @@ -38,8 +42,19 @@ function makeRestrictedConfig(deniedField: string): ObjectPermissionConfig { object: 'account', roles: { restricted: { - roleName: 'restricted', - objectPermissions: { read: true, create: false, update: false, delete: false }, + // `actions` is the declared channel for a role's object-level grants and + // the only one `evaluatePermission` reads. This entry used to spell them + // `objectPermissions: { read: true, … }` beside a `roleName` echo of its + // own map key — neither key exists on + // `ObjectPermissionConfig['roles'][string]`, so both were inert and the + // role granted nothing at all. The rewrite says what the old shape + // meant: read allowed, no writes. + // + // No assertion moves. The field gate these cases exercise runs through + // `checkField`, which reads `fieldPermissions` directly and never + // consults `actions`, so they were testing what they claim even while + // the object grant was empty. + actions: ['read'], fieldPermissions: [{ field: deniedField, read: false, write: false }], }, }, @@ -146,11 +161,15 @@ function makeObjectPermissions(allowDelete: boolean): ObjectPermissionConfig { object: 'account', roles: { restricted: { - roleName: 'restricted', // `evaluatePermission` reads the role's `actions` list, so the grant - // has to live there — `objectPermissions` drives the field-level gate. + // has to live there. The `roleName` echo of the map key and the + // `objectPermissions` block that used to sit beside it are gone: neither + // is declared on `ObjectPermissionConfig['roles'][string]`, so neither + // was ever read. The comment they carried — that `objectPermissions` + // "drives the field-level gate" — was wrong twice over: that gate is + // `checkField`, it reads `fieldPermissions`, and this fixture declares + // none, so the block decided nothing here at all. actions: allowDelete ? ['read', 'delete'] : ['read'], - objectPermissions: { read: true, create: false, update: false, delete: allowDelete }, }, }, }; diff --git a/packages/plugin-list/src/__tests__/ListView.test.tsx b/packages/plugin-list/src/__tests__/ListView.test.tsx index 49492a11b..1c906d87f 100644 --- a/packages/plugin-list/src/__tests__/ListView.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.test.tsx @@ -40,6 +40,27 @@ const renderWithProvider = (component: React.ReactNode) => { ); }; +/** + * A `list-view` node as an AUTHOR writes it, before `normalizeListViewSchema` + * folds the LEGACY vocabulary onto the spec's (#2890). + * + * `ListViewSchema` is the canonical surface — `aria: { ariaLabel }`, `sharing: + * { type }` — and `ListView` reads only that, because the first thing it does + * is `normalizeListViewSchema(propSchema)`. Stored view metadata in user + * databases still carries the legacy spellings, which is exactly why that fold + * exists and why it cannot be dropped; the two cases below are its coverage. + * + * They therefore have to be typed as PRE-fold input, and this says so. The two + * wrong ways out are worth naming, because both look like the smaller change: + * re-spelling the fixtures canonically deletes the only test that the fold + * still happens, and widening `ListViewSchema` to accept both spellings + * re-forks the vocabulary #2890 unified. The legacy input vocabulary has no + * declared type of its own today — filed as #4337 — so a cast is currently the + * only honest way to name it. + */ +const authored = (node: Record): ListViewSchema => + node as unknown as ListViewSchema; + /** * Reveal the visualization options regardless of which form the switcher * takes. With 2–4 visualizations the switcher renders an inline segmented @@ -552,7 +573,9 @@ describe('ListView', () => { }); it('should apply aria attributes to root container', () => { - const schema: ListViewSchema = { + // Legacy `aria.label`; the fold renames it to the spec's `ariaLabel`, which is + // the only spelling ListView's root container reads. + const schema = authored({ type: 'list-view', objectName: 'contacts', viewType: 'grid', @@ -561,7 +584,7 @@ describe('ListView', () => { label: 'Contacts List', live: 'polite', }, - }; + }); renderWithProvider(); const region = screen.getByRole('region', { name: 'Contacts List' }); @@ -570,7 +593,9 @@ describe('ListView', () => { }); it('should render share button when sharing is enabled', () => { - const schema: ListViewSchema = { + // Legacy `sharing: { enabled, visibility }`; the fold collapses it onto the + // spec's `{ type }`, which is what the badge below asserts. + const schema = authored({ type: 'list-view', objectName: 'contacts', viewType: 'grid', @@ -579,7 +604,7 @@ describe('ListView', () => { enabled: true, visibility: 'team', }, - }; + }); renderWithProvider(); const shareButton = screen.getByTestId('share-button'); @@ -1810,7 +1835,11 @@ describe('ListView', () => { objectName: 'contacts', viewType: 'grid', fields: ['name', 'email'], - gantt: { startDateField: 'start', endDateField: 'end' }, + // `titleField` is required by the spec's gantt config — the fixture simply + // omitted it. Supplied rather than cast: this case is about the SPEC + // config winning over the legacy `options` block, so it should be a + // spec-valid config. + gantt: { startDateField: 'start', endDateField: 'end', titleField: 'name' }, }; renderWithProvider(); diff --git a/packages/plugin-list/src/__tests__/UserFilters.tabPresetOperators.test.tsx b/packages/plugin-list/src/__tests__/UserFilters.tabPresetOperators.test.tsx index ad9bd1ac2..69438d8f1 100644 --- a/packages/plugin-list/src/__tests__/UserFilters.tabPresetOperators.test.tsx +++ b/packages/plugin-list/src/__tests__/UserFilters.tabPresetOperators.test.tsx @@ -51,13 +51,32 @@ import { UserFilters } from '../UserFilters'; * a click — the same path a user takes when a view opens on its default tab, * and the path that shipped the 400. */ -function emitFor(rule: Record): unknown[] { +/** + * The authored rule, typed as authored metadata rather than as an untyped bag. + * + * `operator` is a plain `string`, not the spec's operator enum, because feeding + * the legacy and misspelled spellings (`nin`, `notin`, `notIn`) is the point of + * these cases — and it is OPTIONAL because one case deliberately authors a rule + * with no operator at all, to pin that the deleted `case undefined: return '='` + * branch does not come back. `Record` expressed neither fact; + * it merely hid both. + */ +type AuthoredRule = { field: string; operator?: string; value?: unknown }; + +function emitFor(rule: AuthoredRule): unknown[] { const onFilterChange = vi.fn(); render( > & { value?: unknown }] }], }} data={[]} onFilterChange={onFilterChange} diff --git a/packages/plugin-list/tsconfig.test.json b/packages/plugin-list/tsconfig.test.json new file mode 100644 index 000000000..74dbedc54 --- /dev/null +++ b/packages/plugin-list/tsconfig.test.json @@ -0,0 +1,33 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + "declaration": false, + // One notch above the root's ES2020, and no further. 14 of this package's + // 23 raw errors were a single missing built-in: `Array.prototype.at`, which + // these tests use to read the last emitted call (`calls.at(-1)`). The + // compiler named the remedy itself ("Try changing the 'lib' compiler option + // to 'es2022' or later"). Raised HERE and not in `tsconfig.json`, so the + // package SOURCE keeps compiling against the ES2020 baseline it ships to. + "lib": ["ES2022", "DOM", "DOM.Iterable"], + // `spec-symbol-batch6.test.tsx` reads sibling sources off disk to prove the + // spec-derived symbols are not hand-copied. Naming `types` at all switches + // off automatic `@types/*` inclusion; the `toBeInTheDocument` matchers do + // not need naming here because the files that use them `import + // '@testing-library/jest-dom'` explicitly, and a global augmentation + // reached by an import applies to the whole program. + "types": ["node"], + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 27069013c..17bb68b13 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -120,7 +120,6 @@ export const TEST_DEBT = { "@object-ui/components": { errors: 31, issue: 4118, note: "TS7006x12, TS7031x12 — untyped test callback params" }, "@object-ui/react": { errors: 27, issue: 4118, note: "TS2769x9 — overload mismatch on render helpers" }, "@object-ui/plugin-dashboard": { errors: 6, issue: 4118 }, - "@object-ui/plugin-list": { errors: 6, issue: 4118, note: "TS2353x3 — dialect keys" }, }; // ── Collect workspace packages ───────────────────────────────────────────────