diff --git a/.changeset/plugin-dashboard-test-typecheck.md b/.changeset/plugin-dashboard-test-typecheck.md new file mode 100644 index 000000000..6fab659b9 --- /dev/null +++ b/.changeset/plugin-dashboard-test-typecheck.md @@ -0,0 +1,11 @@ +--- +--- + +Test-only change to `@object-ui/plugin-dashboard`: its 47 test files are now +type-checked by a `tsconfig.test.json` chained off `type-check`, and the type +errors that surfaced were fixed in the tests (mock call signatures, one +`vi.importActual` cast, two filter fixtures re-spelled to the spec's option pair +form). No published behaviour changes — no source file was touched. + +This also removes the last `TEST_DEBT` row from +`scripts/check-type-check-coverage.mjs`, closing the objectui#4040 program. diff --git a/packages/plugin-dashboard/package.json b/packages/plugin-dashboard/package.json index 4fcf51d21..965d0682a 100644 --- a/packages/plugin-dashboard/package.json +++ b/packages/plugin-dashboard/package.json @@ -16,7 +16,7 @@ }, "scripts": { "build": "vite build", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "test": "vitest run", "lint": "eslint ." }, diff --git a/packages/plugin-dashboard/src/__tests__/DashboardRenderer.domProps.test.tsx b/packages/plugin-dashboard/src/__tests__/DashboardRenderer.domProps.test.tsx index 8797faa78..fb3a2b917 100644 --- a/packages/plugin-dashboard/src/__tests__/DashboardRenderer.domProps.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DashboardRenderer.domProps.test.tsx @@ -237,7 +237,15 @@ describe("the grid's click channel has one carrier (objectui#4432)", () => { selections.push(id)} + // `id` is annotated rather than inferred, and that is not a style + // choice: `DashboardRendererProps` carries `[key: string]: any`, which + // makes `'ref' extends keyof Props` true, so React's `PropsWithoutRef` + // resolves to `Pick` and every NAMED prop — + // `onWidgetClick` included — collapses to the index signature at the + // JSX call site. The annotation restates the type the interface itself + // still declares, `(widgetId: string | null) => void`, so this callback + // is checked even though the element around it is not (objectui#4040). + onWidgetClick={(id: string | null) => selections.push(id)} onClick={() => hostClicks.push('host')} />, ); @@ -263,7 +271,8 @@ describe("the grid's click channel has one carrier (objectui#4432)", () => { selections.push(id)} + // Annotated for the same reason as the case above. + onWidgetClick={(id: string | null) => selections.push(id)} onClick={'navigate' as never} />, ); diff --git a/packages/plugin-dashboard/src/__tests__/DashboardRenderer.filters.test.tsx b/packages/plugin-dashboard/src/__tests__/DashboardRenderer.filters.test.tsx index 1ed042237..894b45a02 100644 --- a/packages/plugin-dashboard/src/__tests__/DashboardRenderer.filters.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DashboardRenderer.filters.test.tsx @@ -25,8 +25,19 @@ import { DashboardRenderer } from '../DashboardRenderer'; afterEach(cleanup); -/** A dataset data source that records every `queryDataset` call. */ -const makeQueryDataset = () => vi.fn(async () => ({ rows: [{ count: 1 }], fields: [] })); +/** + * A dataset data source that records every `queryDataset` call. + * + * The two parameters are DECLARED, not decorative: `DatasetWidget` calls this + * adapter as `queryDataset(dataset, selection)` (see `DatasetCapableSource` in + * `../DatasetWidget`), and a bare `vi.fn(async () => …)` types `mock.calls` as + * an array of EMPTY tuples — so `c[0]` / `c[1]` below read out of bounds and + * `lastRuntimeFilter` was comparing and casting `undefined` while every + * assertion still passed. Declaring the call shape here is what lets `tsc` + * check the two reads this whole file is built on (objectui#4040). + */ +const makeQueryDataset = () => + vi.fn(async (_dataset: string, _selection: unknown) => ({ rows: [{ count: 1 }], fields: [] })); /** The `runtimeFilter` of the LAST `queryDataset` call for a given dataset. */ const lastRuntimeFilter = ( @@ -81,7 +92,22 @@ describe('DashboardRenderer dashboard-level filters', () => { const schema: DashboardComponentSchema = { type: 'dashboard', globalFilters: [ - { name: 'region', field: 'region', type: 'select', options: ['EMEA', 'APAC'], defaultValue: 'EMEA' }, + // Options in @objectstack/spec's `{ value, label }` pair form. The + // bare-string shorthand these used to spell is a UI-side authoring + // convenience that `GlobalFilterSchema` does not declare; its own + // coverage is `packages/core/src/utils/__tests__/dashboard-filters.test.ts`, + // which pins `normalizeFilterOptions` lifting it. Nothing here reads the + // list — the broadcast under test comes from `defaultValue`. + { + name: 'region', + field: 'region', + type: 'select', + options: [ + { value: 'EMEA', label: 'EMEA' }, + { value: 'APAC', label: 'APAC' }, + ], + defaultValue: 'EMEA', + }, ], widgets: [ { id: 'w1', type: 'bar', dataset: 'invoices', values: ['count'], filter: { status: 'paid' } }, @@ -138,7 +164,8 @@ describe('DashboardRenderer dashboard-level filters', () => { const schema: DashboardComponentSchema = { type: 'dashboard', globalFilters: [ - { name: 'region', field: 'region', type: 'select', options: ['EMEA'], defaultValue: 'EMEA' }, + // Pair form, as above — the shorthand's own pin lives in `@object-ui/core`. + { name: 'region', field: 'region', type: 'select', options: [{ value: 'EMEA', label: 'EMEA' }], defaultValue: 'EMEA' }, ], widgets: [ { id: 'w1', type: 'metric', dataset: 'sales', values: ['revenue'], filter: { stage: 'won' } } as any, diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.queryOptions.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.queryOptions.test.tsx index ebd01bf6b..ad1a17efb 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.queryOptions.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.queryOptions.test.tsx @@ -19,9 +19,17 @@ afterEach(cleanup); * the payload was missing the options the author wrote. */ describe('DatasetWidget — query-affecting widget options (#3588)', () => { - const makeSource = () => ({ queryDataset: vi.fn(async () => ({ rows: [], fields: [] })) }); + // The two parameters are DECLARED because this suite reads them back: + // `DatasetWidget` calls the adapter as `queryDataset(dataset, selection)` + // (`DatasetCapableSource` in `../DatasetWidget`), and a bare + // `vi.fn(async () => …)` types `mock.calls` as an array of EMPTY tuples, so + // every `[1]` read below is out of bounds and resolves to `undefined` — + // silently, because the reads are cast (objectui#4040). + const makeSource = () => ({ + queryDataset: vi.fn(async (_dataset: string, _selection: unknown) => ({ rows: [], fields: [] })), + }); - const selectionOf = (src: { queryDataset: ReturnType }) => + const selectionOf = (src: ReturnType) => src.queryDataset.mock.calls[0]?.[1] as Record; it('lowers options.dateGranularity into the selection', async () => { diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx index fe80ba539..a72fed470 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx @@ -546,7 +546,11 @@ describe('DatasetWidget', () => { { region: 'new_york', quarter: 'q1' }, ], })), - find: vi.fn(async () => ({ data: [] })), + // Parameters DECLARED because the drill assertion below reads + // `find.mock.calls[0][1]`: a bare `vi.fn(async () => …)` types `mock.calls` + // as an array of EMPTY tuples, so that read is out of bounds and resolves + // to `undefined` — invisibly, since it is cast (objectui#4040). + find: vi.fn(async (_object: string, _params: unknown) => ({ data: [] })), getObjectSchema: vi.fn(async () => ({ fields: { region: { type: 'text', label: 'Region' } } })), }; render(); @@ -591,7 +595,11 @@ describe('DatasetWidget', () => { { region: 'literal_emptyset', quarter: 'q1' }, ], })), - find: vi.fn(async () => ({ data: [] })), + // Parameters DECLARED because the drill assertion below reads + // `find.mock.calls[0][1]`: a bare `vi.fn(async () => …)` types `mock.calls` + // as an array of EMPTY tuples, so that read is out of bounds and resolves + // to `undefined` — invisibly, since it is cast (objectui#4040). + find: vi.fn(async (_object: string, _params: unknown) => ({ data: [] })), getObjectSchema: vi.fn(async () => ({ fields: { region: { type: 'text', label: 'Region' } } })), }; render(); diff --git a/packages/plugin-dashboard/src/__tests__/ObjectDataTable.cells.test.tsx b/packages/plugin-dashboard/src/__tests__/ObjectDataTable.cells.test.tsx index 73bd690b5..c92447521 100644 --- a/packages/plugin-dashboard/src/__tests__/ObjectDataTable.cells.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/ObjectDataTable.cells.test.tsx @@ -42,7 +42,12 @@ vi.mock('@object-ui/react', async () => { ); }, useDataScope: () => undefined, - SchemaRendererContext: actual.SchemaRendererContext || (await vi.importActual('react')).createContext({}), + // `vi.importActual` answers `unknown` unless the module's type is named, so + // the `.createContext` read was unchecked — it would have survived the + // export being renamed or removed (objectui#4040). + SchemaRendererContext: + actual.SchemaRendererContext || + (await vi.importActual('react')).createContext({}), }; }); diff --git a/packages/plugin-dashboard/tsconfig.test.json b/packages/plugin-dashboard/tsconfig.test.json new file mode 100644 index 000000000..4e606b64d --- /dev/null +++ b/packages/plugin-dashboard/tsconfig.test.json @@ -0,0 +1,81 @@ +{ + // 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. Here it + // hid two reads that could never have worked: `DashboardRenderer.filters` and + // `DatasetWidget` both declared their `queryDataset` / `find` mocks with NO + // parameters, so `mock.calls` typed as empty tuples and every `calls[0][1]` + // the drill and broadcast assertions are built on read out of bounds — cast, + // so invisible at runtime too (objectui#4040). + // + // Chained from this package's `type-check` script, which is what the CI + // `Type Check` job runs; scripts/check-type-check-coverage.mjs enforces the + // chaining — a config nothing runs is the objectui#3009 failure itself. This + // package was the last TEST_DEBT entry, so that table is now empty. + "extends": "../../tsconfig.json", + "compilerOptions": { + // A checking project, never an emitting one. + "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, + "jsx": "react-jsx", + // Deliberately NOT raised, unlike plugin-list / plugin-grid / app-shell, + // whose suites needed ES2022 for `Array.prototype.at`. Measured: this + // package's 47 test files compile clean against the root's ES2020 baseline, + // so the tests are held to the same lib the shipped SOURCE targets. + "lib": ["ES2020", "DOM", "DOM.Iterable"], + // Naming `types` at all switches off automatic `@types/*` inclusion. + // `node` is load-bearing and measured: dropping it turns five suites red + // with TS2304 `Cannot find name 'global'` — they all install a `global.fetch` + // double. `@testing-library/jest-dom` is deliberately NOT + // listed even though 18 suites use its matchers — measured too: removing it + // changes nothing, because the seven files that reach for the matchers + // `import '@testing-library/jest-dom'` explicitly and a global augmentation + // reached by an import applies to the whole program (plugin-list's lesson). + "types": ["node"], + // + // `paths` drops the root tsconfig's source-tree mappings so `@object-ui/*` + // and `@objectstack/spec` resolve through each workspace dependency's built + // `.d.ts` rather than pulling sibling sources in as program inputs (TS6059) + // — with ONE deliberate exception, which is not a workaround but a + // restatement of what the runtime already does: + // + // Two suites import `@object-ui/plugin-charts` SUBPATHS, and the repo's + // vitest config (`vitest.config.mts`) aliases the whole package to its + // source, which is the only reason they resolve when the tests RUN: + // - `DatasetWidget.chartConfig.dom.test.tsx` side-effect imports + // `.../AdvancedChartImpl` at module scope to pre-load the chunk + // `ChartRenderer` reaches through `React.lazy`, so the unbounded first + // import is paid in the import phase instead of inside RTL's 1000 ms + // `waitFor` (AGENTS.md §测试纪律). + // - `DatasetWidget.comboPresentation.test.tsx` imports + // `.../normalizeChartSchema` as a VALUE and runs the emitted schema + // through it, so what it pins is what `AdvancedChartImpl` receives + // rather than a restatement of it. + // Neither subpath is published: `@object-ui/plugin-charts`' `exports` map + // declares `"."` alone, and neither name is on the barrel either, so both + // specifiers exist only inside this repo — the same packaging gap + // objectui#4325 found behind `@object-ui/fields/widgets/*`. Filed as + // objectui#4529 rather than fixed here: what this package publishes is a + // product call, and #4325's own answer ("drop the import") does not + // transfer to `normalizeChartSchema`, which is a VALUE the assertions run + // the emitted schema through. The mapping below points `tsc` at the + // very files vitest loads, so the compiler checks the real modules; it is + // the honest description of today's state, and it should be DELETED the + // moment either the subpaths are published or the imports are rebuilt on + // a witness that does not need them. Values are relative to the inherited + // `baseUrl` (the repo root), not to this file. + "paths": { + "@object-ui/plugin-charts/*": ["packages/plugin-charts/src/*"] + } + }, + // `src/**/*.d.ts` is pulled in explicitly: the build program gets ambient + // declarations for free from `"include": ["src"]`, but an ambient declaration + // file is only a program input when a pattern NAMES it — being imported is not + // enough, because nothing imports it (plugin-map's lesson, #4270). This + // package keeps one, `src/global.d.ts`, declaring the `*.css` module. + "include": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/*.d.ts"] +} diff --git a/scripts/__tests__/check-type-check-coverage.test.ts b/scripts/__tests__/check-type-check-coverage.test.ts index 96a046800..f7f748381 100644 --- a/scripts/__tests__/check-type-check-coverage.test.ts +++ b/scripts/__tests__/check-type-check-coverage.test.ts @@ -8,6 +8,7 @@ import { fileURLToPath } from 'node:url'; // `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here — // re-adding one is now itself an error (TS2578). See objectui#3494. import { + TEST_DEBT, auditPackages, collect, listTestFiles, @@ -50,10 +51,18 @@ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../ /** All tables empty, so a fixture tree is judged on its own merits. */ const NO_TABLES = { debt: {}, notCompiled: [], checkedByOwnBuild: {}, testDebt: {} }; -/** Builds a throwaway workspace and runs the REAL collector + audit over it. */ +/** + * Builds a throwaway workspace and runs the REAL collector + audit over it. + * + * `tables` overrides the empty defaults for the one block that needs a DECLARED + * entry to judge — what the tables do is itself part of the gate's behaviour, + * and the `TEST_DEBT` block at the bottom of this file asks that question now + * that the real table is empty (objectui#4040). + */ function withWorkspace( build: (write: (rel: string, contents: string) => void) => void, run: (verdict: { dir: string; errors: string[]; packages: ReturnType }) => void, + tables: Partial = {}, ): void { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-type-check-coverage-')); const write = (rel: string, contents: string) => { @@ -64,7 +73,7 @@ function withWorkspace( try { build(write); const packages = collect(dir); - run({ dir, errors: auditPackages(packages, { ...NO_TABLES, root: dir }), packages }); + run({ dir, errors: auditPackages(packages, { ...NO_TABLES, ...tables, root: dir }), packages }); } finally { fs.rmSync(dir, { recursive: true, force: true }); } @@ -787,3 +796,94 @@ describe('examples/schema-catalog is wired up, in this repository', () => { } }); }); + +describe('TEST_DEBT is empty, and what an empty table does (objectui#4040)', () => { + const packages = collect(repoRoot); + // Same shape the blocks above use: the build reads `src/`, the tests live in + // `test/`, so the build program never reaches them. + const buildConfig = JSON.stringify({ include: ['src/**/*'], exclude: ['node_modules', 'dist', 'test'] }, null, 2); + + it('is empty — every package that has tests compiles them', () => { + // The terminal state of the objectui#4040 program. `@object-ui/plugin-dashboard` + // was the last entry (declared 6, measured 14 — wrong in the same direction + // the whole table was wrong in); wiring its `tsconfig.test.json` took the + // list to zero. + // + // Stated as the table AND as the property the table is about, because the + // first alone would be satisfied by deleting an entry without paying it: + // section 6 catches a paid-off entry that lingers, and the second assertion + // here catches the opposite — a package that stops compiling its tests while + // the table stays empty, which is section 5c's job in the gate. + expect(Object.keys(TEST_DEBT)).toEqual([]); + + const uncovered = packages + .filter((p) => p.hasScript && p.testFiles > 0 && !testsCovered(p)) + .map((p) => p.name) + .sort(); + expect(uncovered).toEqual([]); + }); + + it('does NOT close the population — a declared entry still suppresses 5c', () => { + // Worth pinning precisely because "empty table" reads like "no new rows + // allowed", and that is not what this gate does. It is declared, reasoned + // and shrink-only: a package whose tests nothing reads may still DECLARE the + // gap instead of fixing it, exactly as the thirteen graduated packages did. + // What the empty table changes is that such a row can now only arrive as a + // deliberate edit to a table that has been at zero — never unnoticed. + withWorkspace( + (write) => { + write('packages/demo/package.json', manifest('@fixture/demo', 'tsc --noEmit')); + write('packages/demo/tsconfig.json', buildConfig); + write('packages/demo/src/index.ts', 'export const demo = 1;\n'); + write('packages/demo/test/a.test.ts', A_TEST); + }, + ({ errors }) => { + expect(errors).toEqual([]); + }, + { testDebt: { '@fixture/demo': { errors: 3, issue: 4118 } } }, + ); + }); + + it('makes a SILENT gap impossible — undeclared and unread is red', () => { + // The other half of the same fixture, with the entry taken away: this is the + // direction the empty table now leaves nothing between. Identical tree, one + // table difference, opposite verdict. + withWorkspace( + (write) => { + write('packages/demo/package.json', manifest('@fixture/demo', 'tsc --noEmit')); + write('packages/demo/tsconfig.json', buildConfig); + write('packages/demo/src/index.ts', 'export const demo = 1;\n'); + write('packages/demo/test/a.test.ts', A_TEST); + }, + ({ errors }) => { + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('that no `tsc`'); + }, + ); + }); + + it('keeps the ratchet: an entry whose package graduated has to leave', () => { + // Section 6, which is what stops the table from drifting back up by + // accretion — and is the rule this PR obeyed in deleting the last row. + withWorkspace( + (write) => { + write( + 'packages/demo/package.json', + manifest('@fixture/demo', 'tsc --noEmit && tsc -p tsconfig.test.json'), + ); + write('packages/demo/tsconfig.json', buildConfig); + write( + 'packages/demo/tsconfig.test.json', + JSON.stringify({ compilerOptions: { noEmit: true }, include: ['test/**/*.test.ts'] }, null, 2), + ); + write('packages/demo/src/index.ts', 'export const demo = 1;\n'); + write('packages/demo/test/a.test.ts', A_TEST); + }, + ({ errors }) => { + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('type-checks its tests now — delete its TEST_DEBT entry'); + }, + { testDebt: { '@fixture/demo': { errors: 3, issue: 4118 } } }, + ); + }); +}); diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 8a15549f8..ec29a4e00 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -126,9 +126,24 @@ export const CHECKED_BY_OWN_BUILD = { // react declared 27 and measured 43), so remeasure before planning against any // number here. The tranche-4 remeasurement of `core` (56) and `app-shell` (62) // held exactly at tranche 5, which is what a measured number is supposed to do. -export const TEST_DEBT = { - "@object-ui/plugin-dashboard": { errors: 6, issue: 4118 }, -}; +// The last entry, `@object-ui/plugin-dashboard`, was declared 6 and measured 14 +// — wrong in the same direction, to the end. +// +// EMPTY, and that is the objectui#4040 program's terminal state: all 41 packages +// that have tests now compile them. Note what an empty table does and does not +// do. It does NOT close the population: section 5c below still accepts a fresh +// TEST_DEBT entry as an alternative to a `tsconfig.test.json`, so a package that +// stops reading its tests can still DECLARE the gap rather than fix it — the +// gate's whole design is "declared, reasoned, shrink-only", not "forbidden". +// What is now structurally impossible is a SILENT one: a package whose tests no +// program reads and which is not listed here fails 5c, an entry that has been +// paid off fails section 6, and a `tsconfig.test.json` that exists but is +// chained by nothing, emits, misses a test file, or is chained while missing +// fails 5a/5b/5·—. So a new row can only appear as a deliberate, reviewable +// addition to a table that has been at zero — which is exactly the ratchet +// objectui#4291/#4347 tightened. Adding one back is a decision, never an +// accident; keep it that way. +export const TEST_DEBT = {}; // ── Collect workspace packages ─────────────────────────────────────────────── export const GROUPS = ["packages", "apps", "examples"];