Skip to content

Commit d7e07c8

Browse files
baozhoutaoos-zhuangclaude
authored
fix(client): compile the client test layer so its @ts-expect-error stops being a phantom check (#5546)
`packages/client/tsconfig.json` excludes `**/*.test.ts` and the package's `typecheck` script was a bare `tsc --noEmit` reading that same config, so no gate anywhere read a client test file with a type checker. The one `@ts-expect-error` in the package — `client.test.ts:1283` — evaluated never: on `origin/main`, deleting the directive line left `pnpm --filter @objectstack/client typecheck` just as green as leaving it there. Compiled for the first time, that directive reports TS2578 "unused". It never had anything to suppress: `project(environmentId: string)` accepts `''`, which is a perfectly good `string`, and the directive's own comment already said what the test proves — the empty id is rejected at RUNTIME. So the repair is to delete the directive, not to keep it: the reverse verification is the mirror of the usual one, and RESTORING the line is what now goes red. - `packages/client/tsconfig.test.json`: a sibling of the build config (which keeps its exclusion — ci.yml gates that no test file reaches the published artifact) with vitest's module semantics (`module: esnext`, `moduleResolution: bundler`, ES2022 lib) and `rootDir` widened to the workspace root, since four test files deep-import sibling packages' route ledgers. Strictness flags are inherited, untouched. - 13 errors surfaced; eight were the tests' own and are fixed here — two unused imports, an unused parameter, two possibly-undefined reads on an optional `routes` map, an `unknown` payload now asserted with `toMatchObject` instead of cast, a `reference_to` key the field schema never had (the lookup declared no target at all), and the phantom pin. Re-spelling that key uncovered one more of the remaining kind; the six that stay are one producer-side defect (#5543) held per file in `test-typecheck-debt.json`, EXACT and shrink-only. - `scripts/check-test-typecheck.mts`: PROMOTED from `packages/spec/scripts/`, parameterized with `--package`, so client onboards by wiring its `typecheck` script rather than by copying 300 lines. spec's ledger and its 79 files / 691 errors are unchanged, and the generated `_comment` is byte-identical. - Both graduations the gates force: `@objectstack/client` leaves TEST_DEBT (its stale entry measured 15 files / 19 errors, five of them the inherited-rootDir TS6059 that were the check's own misconfiguration) and its `PHANTOM_PIN_DEBT` seed — the entry #5478 left addressed to this issue — is deleted. Fixes #5449 Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh Co-authored-by: os-zhuang <jack@objectstack.ai> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 4fdf480 commit d7e07c8

11 files changed

Lines changed: 180 additions & 52 deletions

AGENTS.md

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,15 @@ never its own *strictness*: `strict` and friends are inherited, untouched.
4646
"tsc is the best sweeper" channel the spec-property-retirement playbook leans on: the
4747
directive is meant to go red the day a removed key comes back. Outside a program it
4848
evaluates never, and *deleting the directive leaves every gate just as green* — which is
49-
how spec's 17 retirement pins across 5 files were found (#5286). Before writing one,
50-
check the file is compiled. `packages/spec` additionally holds its test-layer residue in
51-
a per-file, exactly-measured, shrink-only ledger (`packages/spec/test-typecheck-debt.json`,
52-
`pnpm --filter @objectstack/spec gen:test-typecheck-debt`): a file not listed there may
53-
have no type errors at all.
49+
how spec's 17 retirement pins across 5 files were found (#5286), and the repo-wide sweep
50+
that followed found the eighteenth in `packages/client` (#5449). Before writing one,
51+
check the file is compiled. A package whose test layer still carries residue holds it in
52+
a per-file, exactly-measured, shrink-only ledger next to its `tsconfig.test.json`
53+
(`<package>/test-typecheck-debt.json`, regenerated with
54+
`pnpm --filter <package> gen:test-typecheck-debt`): a file not listed there may have no
55+
type errors at all. The gate behind both is one shared script,
56+
`scripts/check-test-typecheck.mts --package <dir>` — onboard a package by wiring its
57+
`typecheck` script to it, never by copying it.
5458

5559
One trap worth knowing before you read any of these counts: under `moduleResolution:
5660
NodeNext` a relative import missing its `.js` extension does not resolve, every symbol it

packages/client/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
"build": "tsup --config ../../tsup.config.ts",
1717
"test": "vitest run",
1818
"test:integration": "vitest run --config vitest.integration.config.ts",
19-
"typecheck": "tsc --noEmit"
19+
"check:test-typecheck": "tsx ../../scripts/check-test-typecheck.mts --self-test && tsx ../../scripts/check-test-typecheck.mts --package packages/client --project tsconfig.test.json",
20+
"gen:test-typecheck-debt": "tsx ../../scripts/check-test-typecheck.mts --update --package packages/client --project tsconfig.test.json",
21+
"typecheck": "tsc --noEmit && pnpm check:test-typecheck"
2022
},
2123
"dependencies": {
2224
"@objectstack/core": "workspace:*",
@@ -29,6 +31,7 @@
2931
"@objectstack/objectql": "workspace:*",
3032
"@objectstack/plugin-hono-server": "workspace:*",
3133
"@objectstack/runtime": "workspace:*",
34+
"tsx": "^4.23.1",
3235
"typescript": "^6.0.3",
3336
"vitest": "^4.1.10"
3437
},

packages/client/src/client.batch-transaction.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ describe('data.batchTransaction (live Hono, #1604)', () => {
9494
label: 'Task',
9595
fields: {
9696
title: { type: 'text', label: 'Title' },
97-
project: { type: 'lookup', reference_to: 'project', label: 'Project' },
97+
// `reference`, not `reference_to`: the latter is no key the field
98+
// schema knows, so this lookup declared no target at all until a
99+
// tsc program finally read the file (TS2561, #5449).
100+
project: { type: 'lookup', reference: 'project', label: 'Project' },
98101
},
99102
});
100103
// Objects registered AFTER bootstrap miss the boot-time schema sync, so

packages/client/src/client.hono.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('ObjectStackClient (with Hono Server)', () => {
4747
// --- BROKER SHIM START ---
4848
// HttpDispatcher requires a broker to function. We inject a simple shim.
4949
(kernel as any).broker = {
50-
call: async (action: string, params: any, opts: any) => {
50+
call: async (action: string, params: any, _opts: any) => {
5151
const parts = action.split('.');
5252
const service = parts[0];
5353
const method = parts[1];
@@ -159,9 +159,12 @@ describe('ObjectStackClient (with Hono Server)', () => {
159159

160160
// Discovery is REST's, computed from its registry (#4018 D12: declared
161161
// === enforced). Every route it advertises must actually answer.
162+
// `routes` is optional on the discovery payload, so it is reached
163+
// optionally and asserted — a missing map fails `toContain` rather than
164+
// being waved through by a `!` or a `?? {}` default (#5449).
162165
const endpoints = client['discoveryInfo']!.routes;
163-
expect(endpoints.data).toContain('/api/v1/data');
164-
expect(endpoints.metadata).toContain('/api/v1/meta');
166+
expect(endpoints?.data).toContain('/api/v1/data');
167+
expect(endpoints?.metadata).toContain('/api/v1/meta');
165168

166169
// Enforced, not just declared — the pairing #4018 exists to hold.
167170
expect((await fetch(`${baseUrl}/api/v1/meta/objects`)).status).not.toBe(404);

packages/client/src/client.test.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, it, expect, vi } from 'vitest';
2-
import { ObjectStackClient, QueryBuilder, FilterBuilder, createQuery, createFilter } from './index';
2+
// `QueryBuilder` / `FilterBuilder` are named only by the `describe` blocks below;
3+
// the suites build them through `createQuery` / `createFilter`, so importing the
4+
// classes themselves left two unused bindings (TS6133) the moment this file
5+
// entered a tsc program (#5449).
6+
import { ObjectStackClient, createQuery, createFilter } from './index';
37

48
/** Helper: create a client with mocked fetch that returns the given response body */
59
function createMockClient(body: any, status = 200) {
@@ -103,7 +107,11 @@ describe('ObjectStackClient', () => {
103107

104108
const result = await client.meta.getItem('object', 'customer');
105109
expect(fetchMock).toHaveBeenCalledWith('http://localhost:3000/api/v1/meta/object/customer', expect.any(Object));
106-
expect(result.name).toBe('customer');
110+
// `meta.getItem` has no declared return type (unlike the `getItems`
111+
// beside it — #5545), so its unwrapped payload is `unknown`. Asserted
112+
// structurally rather than cast: same assertion strength, without
113+
// pretending this surface is typed (#5449).
114+
expect(result).toMatchObject({ name: 'customer' });
107115
});
108116

109117
it('meta.getView speaks the path-param dialect both surfaces accept (#3611)', async () => {
@@ -1280,7 +1288,12 @@ describe('ScopedProjectClient', () => {
12801288

12811289
it('throws when environmentId is missing', () => {
12821290
const client = new ObjectStackClient({ baseUrl: 'http://localhost:3000' });
1283-
// @ts-expect-error — empty string rejected at runtime
1291+
// No `@ts-expect-error` here, and that is the finding of #5449 rather than
1292+
// an omission. `project(environmentId: string)` accepts `''` — it is a
1293+
// perfectly good `string` — so the directive that sat on this line
1294+
// suppressed nothing and reported TS2578 ("unused") the first time a tsc
1295+
// program read the file. Its own comment said what the test actually
1296+
// proves: the empty id is rejected at RUNTIME, by the guard below.
12841297
expect(() => client.project('')).toThrow(/environmentId is required/);
12851298
});
12861299

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"_comment": "Per-file tsc error debt of the @objectstack/client TEST layer (#5286). `tsconfig.test.json` compiles `src/**/*.test.ts` — which `tsconfig.json` excludes and therefore no gate ever read — and every file below still carries errors from before that gate existed, almost all of them fixture literals annotated with a schema OUTPUT type (`z.infer`) while holding an authored INPUT literal. EXACT ratchet, judged by re-running tsc: a file that gains errors is red, a file that loses them is red until its number is re-recorded, a file that reaches zero is red until its entry is deleted, and a file NOT listed here may have no errors at all. Regenerate with: pnpm --filter @objectstack/client gen:test-typecheck-debt",
3+
"entries": {
4+
"src/client.batch-transaction.test.ts": 3,
5+
"src/client.environment-scoping.test.ts": 1,
6+
"src/client.hono.test.ts": 2
7+
}
8+
}

packages/client/tsconfig.test.json

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// The TEST-layer type-check program (#5449, the mechanism #5286/PR #5478 set
2+
// for `packages/spec`). `tsconfig.json` above stays as it is: it is the BUILD
3+
// config, and its `**/*.test.ts` exclusion has a reason — ci.yml gates that no
4+
// test file reaches the published artifact. This sibling puts the excluded
5+
// layer back in front of tsc, and `package.json`'s `typecheck` script NAMES it
6+
// (`-p tsconfig.test.json`), because a config no script invokes is exactly the
7+
// phantom this whole change is about.
8+
//
9+
// What differs from the build config, and what deliberately does NOT:
10+
// - module semantics ONLY. The tests are written and executed as ESM by
11+
// vitest (esbuild/vite), while `client` has no `"type": "module"`, so the
12+
// build config's NodeNext compiles them as CJS and reports errors about the
13+
// CHECK rather than the code (TS2835 extensionless relative imports, TS1470
14+
// `import.meta`, TS2550 lib). Matching vitest is fidelity.
15+
// - `rootDir` widens to the workspace root. It steers emit layout only, and
16+
// this program emits nothing; inherited as `./src` it reported TS6059 for
17+
// the four route-ledger modules `client-url-conformance.test.ts` and the
18+
// three `*-route-ledger-coverage.test.ts` files deep-import from sibling
19+
// packages (`../../runtime/src/route-ledger`, …). Those five TS6059 are the
20+
// bulk of this package's stale TEST_DEBT entry — a measurement of the
21+
// misconfigured check, not of the tests.
22+
// - STRICTNESS IS UNTOUCHED. `strict`, `noUnusedLocals`, `noUnusedParameters`,
23+
// `noImplicitReturns` and the rest are inherited from the root config.
24+
// Nothing here may loosen a type rule; if a test does not compile, that is
25+
// the finding.
26+
//
27+
// `include` deliberately stops at `src`, matching the build config's root, and
28+
// none of the files it leaves out carries a `@ts-expect-error`, so no pin is
29+
// hiding there. `tests/integration/` — the suite `vitest.integration.config.ts`
30+
// runs against a live server — is in no tsconfig at all: a second,
31+
// differently-shaped hole (1 file / 3 errors, one of them a real API drift, the
32+
// suite reading a `client.discovery` property `ObjectStackClient` does not
33+
// have) that wants its own change rather than a rider on this one. Filed as
34+
// #5544.
35+
//
36+
// The per-file ledger beside this config (`test-typecheck-debt.json`) is small
37+
// on purpose. Under the repaired config the whole test layer came to 13 errors;
38+
// eight were the tests' own and are fixed in this same change (two unused
39+
// imports, an unused parameter, two possibly-undefined reads, an `unknown`
40+
// payload asserted structurally, a `reference_to` key the field schema never
41+
// had, and the phantom pin itself). Re-spelling that key uncovered one more of
42+
// the remaining kind, and all six that stay are ONE producer-side defect
43+
// wearing three files' clothes: objectql's `registerObject` takes the schema's
44+
// OUTPUT type (`z.infer`) where it should take the INPUT one, so a perfectly
45+
// good authored literal reads as missing nine defaulted keys (#5543). Holding
46+
// them EXACT and shrink-only means fixing #5543 turns the ledger red until the
47+
// entries are deleted, instead of letting it rot. Every file NOT listed there —
48+
// `client.test.ts`, the pin file, first among them — must have no errors at
49+
// all.
50+
{
51+
"extends": "./tsconfig.json",
52+
"compilerOptions": {
53+
"noEmit": true,
54+
"rootDir": "../..",
55+
"module": "esnext",
56+
"moduleResolution": "bundler",
57+
"lib": ["ES2022", "DOM", "DOM.Iterable"],
58+
"types": ["node"]
59+
},
60+
"include": ["src/**/*"],
61+
"exclude": ["node_modules", "dist"]
62+
}

packages/spec/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,8 @@
217217
"check:react-blocks": "tsx scripts/build-react-blocks-contract.ts --check",
218218
"check:react-declaration-parity": "tsx scripts/check-react-blocks-declaration-parity.ts",
219219
"check:skill-examples": "tsx scripts/check-skill-examples.ts",
220-
"check:test-typecheck": "tsx scripts/check-test-typecheck.mts --self-test && tsx scripts/check-test-typecheck.mts --project tsconfig.test.json",
221-
"gen:test-typecheck-debt": "tsx scripts/check-test-typecheck.mts --update --project tsconfig.test.json",
220+
"check:test-typecheck": "tsx ../../scripts/check-test-typecheck.mts --self-test && tsx ../../scripts/check-test-typecheck.mts --package packages/spec --project tsconfig.test.json",
221+
"gen:test-typecheck-debt": "tsx ../../scripts/check-test-typecheck.mts --update --package packages/spec --project tsconfig.test.json",
222222
"typecheck": "tsc --noEmit && pnpm check:test-typecheck"
223223
},
224224
"keywords": [

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)