Skip to content

Commit b39f4b3

Browse files
os-zhuangclaude
andauthored
fix(runner): type-check the package at all, fix the hidden DataSource violation (#2917) (#2922)
@object-ui/runner was the worst-covered package in the repo: `vite build` (transpile only), no type-check script, and uniquely no tsconfig.json at all. Nothing had ever type-checked it, despite it being published. It was NOT broken at runtime — #2917's own risk assessment was wrong and is corrected there. The two bad imports were `import type`, so they were erased before they could fail; the one value import (emulateBatchTransaction) does exist; and MockDataSource is unreferenced anywhere in the repo. What the missing check hid: DataSource and BatchTransactionOperation were imported from @object-ui/core, which does not export them (they live in @object-ui/types). Because that import never resolved, `implements DataSource` was silently a no-op, and three commits maintained the class as if it were verified (62b9ab5, 09d9669, 5527388). A real contract violation survived all three: async find(resource: string, params?: any): Promise<any[]> { return []; } DataSource.find returns a QueryResult envelope, not a bare array. The file's doc comment invites using it as the starting point for a real adapter, so anyone who did would hand every consumer an array whose .data/.total are undefined. - tsconfig.json added, mirroring apps/console (a Vite app) rather than a library: bundler resolution, allowImportingTsExtensions for ./App.tsx, and types: ["vite/client"] for import.meta.glob and the ./index.css import. Standalone, so the root `paths` are never inherited and TS6059 cannot arise. - find() now returns Promise<QueryResult> as { data: [], total: 0 }. - 6 unused params prefixed with _; unused Circle icon dropped from LayoutRenderer. - type-check script added; DEBT entry deleted. Coverage 35 -> 36 of 45, outstanding errors 46 -> 32. With a tsconfig the real count is 10, not 14 — four were artifacts of having none. Proven the gate covers it rather than trusting the green: injecting an error into runner/src/App.tsx yields `Failed: @object-ui/runner#type-check`, which was impossible before. Closes #2917 Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 210c5c0 commit b39f4b3

6 files changed

Lines changed: 102 additions & 8 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@object-ui/runner": patch
3+
---
4+
5+
fix(runner): type-check the package at all, and fix the `DataSource` contract violation that hid behind a broken import (#2917)
6+
7+
`@object-ui/runner` was the worst-covered package in the repo: `build` is
8+
`vite build` (transpile only), it had no `type-check` script, and — uniquely —
9+
**no `tsconfig.json` at all**. Nothing had ever type-checked it, despite it being
10+
a published package.
11+
12+
**It was not broken at runtime.** The two bad imports were `import type`, so they
13+
were erased before they could fail, and the one value import
14+
(`emulateBatchTransaction`) does exist. `MockDataSource` is also unreferenced
15+
anywhere in the repo. So this is a correctness and reference-quality fix, not an
16+
outage.
17+
18+
**What the missing check actually hid.** `DataSource` and
19+
`BatchTransactionOperation` were imported from `@object-ui/core`, which does not
20+
export them — they live in `@object-ui/types`. Because that import never
21+
resolved, `class MockDataSource implements DataSource` was silently a no-op, and
22+
three separate commits maintained the class *as if* it were being verified
23+
(`62b9ab510` added `batchTransaction`, `09d9669c7` made `getObjectSchema`
24+
required, `5527388b0` added input validation). With the `implements` clause
25+
inert, a real contract violation survived all three:
26+
27+
```ts
28+
async find(resource: string, params?: any): Promise<any[]> { return []; }
29+
```
30+
31+
`DataSource.find` returns a `QueryResult` envelope, not a bare array. Anyone
32+
copying this mock as the starting point for their own adapter — which is exactly
33+
what its doc comment invites — would hand every consumer an array where `.data`
34+
and `.total` are `undefined`. Now typed as `Promise<QueryResult>` and returning
35+
`{ data: [], total: 0 }`.
36+
37+
Also in this change:
38+
39+
- `packages/runner/tsconfig.json` added, mirroring `apps/console` rather than the
40+
library packages: `runner` is a Vite app, so it wants `bundler` resolution,
41+
`allowImportingTsExtensions` (for `./App.tsx`) and `types: ["vite/client"]`
42+
(for `import.meta.glob` in `MetadataLoader` and the `./index.css` side-effect
43+
import). Keeping it standalone instead of extending the root config also means
44+
it never inherits the root `paths`, so workspace deps resolve through built
45+
`.d.ts` and the TS6059 `rootDir` class of error cannot appear.
46+
- unused parameters prefixed with `_` (6x in `mockDataSource`), and an unused
47+
`Circle` icon import dropped from `LayoutRenderer`.
48+
- `"type-check": "tsc --noEmit"` added, and the package's `DEBT` entry deleted
49+
from `scripts/check-type-check-coverage.mjs`. Coverage goes 35 -> 36 of 45 and
50+
outstanding errors 46 -> 32.
51+
52+
Verified the gate genuinely covers the package now, rather than trusting the
53+
green: injecting a type error into `runner/src/App.tsx` makes `pnpm type-check`
54+
fail with `Failed: @object-ui/runner#type-check`, which was impossible before
55+
this change.

packages/runner/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"scripts": {
1717
"dev": "vite",
1818
"build": "vite build",
19+
"type-check": "tsc --noEmit",
1920
"preview": "vite preview",
2021
"test": "vitest run"
2122
},

packages/runner/src/LayoutRenderer.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
Bell,
1313
Box,
1414
ChevronDown,
15-
Circle,
1615
Menu,
1716
Moon,
1817
Search,

packages/runner/src/lib/mockDataSource.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,28 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9-
import type { DataSource, BatchTransactionOperation } from '@object-ui/core';
9+
import type {
10+
DataSource,
11+
BatchTransactionOperation,
12+
QueryParams,
13+
QueryResult,
14+
} from '@object-ui/types';
1015
import { emulateBatchTransaction } from '@object-ui/core';
1116

1217
/**
1318
* 模拟数据源 (Mock Adapter)
1419
* 在真实项目中,你会在这里使用 fetch/axios 调用你的 API。
1520
*/
1621
export class MockDataSource implements DataSource {
17-
async find(resource: string, params?: any): Promise<any[]> {
22+
async find(resource: string, params?: QueryParams): Promise<QueryResult> {
1823
console.log(`[DataSource] Querying ${resource}`, params);
19-
return [];
24+
// `find` returns an envelope, not a bare array — consumers read `.data`
25+
// and `.total` (see QueryResult). Returning `[]` here would leave every
26+
// caller with `undefined` data.
27+
return { data: [], total: 0 };
2028
}
2129

22-
async findOne(resource: string, id: string): Promise<any> {
30+
async findOne(_resource: string, _id: string): Promise<any> {
2331
return null;
2432
}
2533

@@ -33,11 +41,11 @@ export class MockDataSource implements DataSource {
3341
return { id: Math.random().toString(), ...data };
3442
}
3543

36-
async update(resource: string, id: string, data: any): Promise<any> {
44+
async update(_resource: string, _id: string, data: any): Promise<any> {
3745
return data;
3846
}
3947

40-
async delete(resource: string, id: string): Promise<any> {
48+
async delete(_resource: string, _id: string): Promise<any> {
4149
return true;
4250
}
4351

packages/runner/tsconfig.json

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
// Standalone rather than extending the root config: `runner` is a Vite app,
3+
// so it wants the same shape as apps/console — and not inheriting the root
4+
// `paths` keeps `@object-ui/*` resolving through each dependency's built
5+
// `.d.ts` instead of pulling sibling sources in as program inputs.
6+
"compilerOptions": {
7+
"target": "ES2020",
8+
"useDefineForClassFields": true,
9+
"lib": ["ES2020", "DOM", "DOM.Iterable"],
10+
"module": "ESNext",
11+
"skipLibCheck": true,
12+
13+
/* Bundler mode */
14+
"moduleResolution": "bundler",
15+
"allowImportingTsExtensions": true,
16+
"resolveJsonModule": true,
17+
"isolatedModules": true,
18+
"noEmit": true,
19+
"jsx": "react-jsx",
20+
21+
/* Linting */
22+
"strict": true,
23+
"noUnusedLocals": true,
24+
"noUnusedParameters": true,
25+
"noFallthroughCasesInSwitch": true,
26+
27+
/* `import.meta.glob` in MetadataLoader, and CSS side-effect imports. */
28+
"types": ["vite/client"]
29+
},
30+
"include": ["src"],
31+
"exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"]
32+
}

scripts/check-type-check-coverage.mjs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ const root = resolve(dirname(fileURLToPath(import.meta.url)), "..");
2929
// #2911 sweep (bare `tsc --noEmit` with the `paths` override its type-checked
3030
// peers already carry, so the TS6059 rootDir noise is excluded).
3131
const DEBT = {
32-
"@object-ui/runner": { errors: 14, issue: 2917, note: "no tsconfig.json at all; also imports two exports @object-ui/core does not have" },
3332
"@object-ui/plugin-form": { errors: 10, issue: 2919, note: "6x t() fallback-signature mismatch, 2x undefined index, 2x string|number" },
3433
"@object-ui/site": { errors: 7, issue: 2919, note: "TS2304 on Next's generated LayoutProps/PageProps; needs .next/types from a prior next build" },
3534
"@object-ui/plugin-grid": { errors: 4, issue: 2919, note: "2x t() call signature + 2x TS2367 that are closure-mutation narrowing artifacts, NOT a logic bug" },

0 commit comments

Comments
 (0)