Skip to content

Commit e21a308

Browse files
committed
Fix: DSL Type Inference for .array() and .map() Methods (#248)
## Problem TypeScript failed to automatically infer handler parameter types in `.array()` and `.map()` methods, forcing users to add manual type annotations: ``` // Required manual annotations (BAD) .array({ slug: 'fetch_users' }, async ({ run }: { run: { userIds: string[] } }) => { return run.userIds.map((id: string) => ({ id, name: `User_${id}` })); }) .map({ slug: 'add_timestamps', array: 'fetch_users' }, (user: { id: string; name: string }) => ({ ...user, createdAt: new Date().toISOString(), })) ``` ## Root Cause Complex conditional types on handler **parameters** prevented TypeScript from inferring the `THandler` type parameter: ``` // ❌ Prevented inference handler: ValidateHandlerReturn<THandler, readonly any[]> handler: TFlowInput extends readonly (infer Item)[] ? THandler & ((item: Item, ...) => ...) : never ``` ## Solution Moved all type constraints from parameter types to **type parameter constraints**: ``` // ✅ Enables inference THandler extends (...) => readonly any[] | Promise<readonly any[]> handler: THandler THandler extends TFlowInput extends readonly (infer Item)[] ? (item: Item, ...) => Json | Promise<Json> : never handler: THandler ``` ### Additional Fixes **Client exhaustiveness checks**: Replaced verbose unused variable pattern with cleaner `satisfies never`: ``` // Before const _exhaustivenessCheck: never = event; // After event satisfies never; ``` ## Results ✅ **Type inference works automatically** - no manual annotations needed ✅ **All 111 DSL type tests pass** ✅ **All package typechecks pass** (dsl, core, client, edge-worker) ✅ **Cleaner exhaustiveness checks** - no unused variable warnings ## Type System Changes - `.array()`: Handler constraint moved to type parameter - `.map()` (root): Handler constraint moved to type parameter - `.map()` (dependent): Handler constraint moved to type parameter - Removed unused `ValidateHandlerReturn` utility type These changes maintain the same runtime behavior and type safety while enabling proper type inference.
1 parent 026cb6c commit e21a308

File tree

14 files changed

+574
-127
lines changed

14 files changed

+574
-127
lines changed
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
name: Type Testing Health Check
2+
3+
# Run on every push/PR to ensure type testing infrastructure always works
4+
# Cost: ~1-2 seconds per run
5+
# Benefit: Never ship broken type validation
6+
on:
7+
pull_request:
8+
push:
9+
branches:
10+
- main
11+
- 'release/**'
12+
13+
# Allow manual trigger
14+
workflow_dispatch:
15+
16+
jobs:
17+
health-check:
18+
name: Verify Type Testing Infrastructure
19+
runs-on: ubuntu-latest
20+
21+
steps:
22+
- uses: actions/checkout@v4
23+
24+
- uses: pnpm/action-setup@v2
25+
with:
26+
version: 8
27+
28+
- uses: actions/setup-node@v4
29+
with:
30+
node-version: '20'
31+
cache: 'pnpm'
32+
33+
- name: Install dependencies
34+
run: pnpm install --frozen-lockfile
35+
36+
- name: Run Type Testing Health Check
37+
run: pnpm nx test:types:health dsl
38+
39+
- name: Report Results
40+
if: failure()
41+
run: |
42+
echo "::error::Type testing infrastructure is broken!"
43+
echo "This means type tests may not be catching bugs."
44+
echo "Check the health check output above for details."

pkgs/client/project.json

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,24 @@
187187
"parallel": false
188188
}
189189
},
190-
"test:types": {
190+
"test:types:vitest": {
191+
"executor": "nx:run-commands",
192+
"dependsOn": ["build"],
193+
"options": {
194+
"cwd": "{projectRoot}",
195+
"command": "pnpm vitest --typecheck.only --run"
196+
}
197+
},
198+
"test:types:strict": {
191199
"executor": "nx:run-commands",
192200
"options": {
193201
"cwd": "{projectRoot}",
194-
"command": "bash ../../scripts/typecheck-strict.sh"
202+
"command": "bash ../../scripts/typecheck-ts2578.sh"
195203
}
204+
},
205+
"test:types": {
206+
"executor": "nx:noop",
207+
"dependsOn": ["test:types:vitest", "test:types:strict"]
196208
}
197209
}
198210
}

pkgs/client/src/lib/FlowRun.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,8 @@ export class FlowRun<TFlow extends AnyFlow>
345345
break;
346346

347347
default: {
348-
// Exhaustiveness check - should never happen with proper types
349-
// @ts-expect-error Intentional exhaustiveness check
350-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
351-
const _exhaustivenessCheck: never = event;
348+
// Exhaustiveness check - ensures all event statuses are handled
349+
event satisfies never;
352350
return false;
353351
}
354352
}

pkgs/client/src/lib/FlowStep.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,8 @@ export class FlowStep<
235235
break;
236236

237237
default: {
238-
// Exhaustiveness check - should never happen with proper types
239-
// @ts-expect-error Intentional exhaustiveness check
240-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
241-
const _exhaustivenessCheck: never = event;
238+
// Exhaustiveness check - ensures all event statuses are handled
239+
event satisfies never;
242240
return false;
243241
}
244242
}

pkgs/core/project.json

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,12 +270,24 @@
270270
},
271271
"cache": true
272272
},
273-
"test:types": {
273+
"test:types:vitest": {
274274
"executor": "nx:run-commands",
275+
"dependsOn": ["build"],
275276
"options": {
276277
"cwd": "{projectRoot}",
277-
"command": "bash ../../scripts/typecheck-strict.sh"
278+
"command": "pnpm vitest --typecheck.only --run"
278279
}
280+
},
281+
"test:types:strict": {
282+
"executor": "nx:run-commands",
283+
"options": {
284+
"cwd": "{projectRoot}",
285+
"command": "bash ../../scripts/typecheck-ts2578.sh"
286+
}
287+
},
288+
"test:types": {
289+
"executor": "nx:noop",
290+
"dependsOn": ["test:types:vitest", "test:types:strict"]
279291
}
280292
}
281293
}

pkgs/dsl/__tests__/types/README.md

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# Type Testing Infrastructure
2+
3+
This directory contains TypeScript type tests using a **hybrid approach** that combines:
4+
1. **Vitest `expectTypeOf`** for positive type assertions (clear error messages)
5+
2. **`@ts-expect-error`** for negative assertions (enforces code doesn't compile)
6+
7+
## Running Type Tests
8+
9+
```bash
10+
# Run all type tests (vitest + TS2578 validation)
11+
pnpm nx test:types dsl
12+
13+
# Individual targets for faster iteration:
14+
pnpm nx test:types:vitest dsl # Vitest only (fast, ~300ms)
15+
pnpm nx test:types:strict dsl # TS2578 validation only
16+
17+
# Health check (verifies testing infrastructure works)
18+
pnpm nx test:types:health dsl
19+
```
20+
21+
## Development Workflow
22+
23+
**Fast iteration (during development):**
24+
```bash
25+
pnpm nx test:types:vitest dsl # Fast, nice output
26+
```
27+
28+
**Full validation (before commit):**
29+
```bash
30+
pnpm nx test:types dsl # Runs both vitest and TS2578 checks
31+
```
32+
33+
## Health Check System
34+
35+
### Purpose
36+
The health check ensures the type testing infrastructure itself is working correctly. Without this, type tests could silently break and fail to catch bugs.
37+
38+
### Components
39+
40+
#### 1. Canary File (`__canary__.test-d.ts`)
41+
- **Intentionally fails** type checking
42+
- Contains known type errors that MUST be detected
43+
- Excluded from normal test runs (see `tsconfig.typecheck.json`)
44+
- Tests both `expectTypeOf` and `@ts-expect-error` mechanisms
45+
46+
#### 2. Verification Script (`scripts/verify-type-test-health.sh`)
47+
Runs diagnostic tests to verify:
48+
- tsc detects type errors
49+
- `expectTypeOf` produces clear "Expected X, Actual Y" messages
50+
- TS2578 detection works (unused `@ts-expect-error`)
51+
- Expected error patterns appear (TS2344, TS2578)
52+
53+
#### 3. Nx Target (`test:types:health`)
54+
Run with: `pnpm nx test:types:health dsl`
55+
56+
### When to Run Health Check
57+
58+
- **After TypeScript version upgrades**
59+
- **After Vitest updates**
60+
- **After modifying tsconfig files**
61+
- **After changing typecheck-ts2578.sh script**
62+
- **In CI** (recommended: run weekly or on release branches)
63+
64+
### Expected Output
65+
66+
**Healthy system:**
67+
```
68+
✅ SUCCESS: Type testing infrastructure is healthy
69+
70+
All checks passed:
71+
• Pass 1 (project-wide) catches type errors
72+
• expectTypeOf produces clear error messages
73+
• Pass 2 (individual files) catches TS2578 errors
74+
• Canary file exhibits expected error patterns
75+
```
76+
77+
**Broken system:**
78+
```
79+
❌ FAILURE: Type testing infrastructure is BROKEN
80+
81+
This means type tests may not be catching bugs!
82+
```
83+
84+
## Hybrid Testing Approach
85+
86+
### Use `expectTypeOf` for Positive Assertions
87+
88+
```typescript
89+
it('should correctly infer types', () => {
90+
const flow = new Flow<{ userId: string }>({ slug: 'test' })
91+
.step({ slug: 's1' }, () => 42);
92+
93+
const step = flow.getStepDefinition('s1');
94+
95+
// Clear, explicit type assertion
96+
expectTypeOf(step.handler).returns.toEqualTypeOf<number>();
97+
});
98+
```
99+
100+
**Benefits:**
101+
- ✅ Clear error messages: `"Expected: number, Actual: string"`
102+
- ✅ Explicit about what types SHOULD be
103+
- ✅ Good for complex type testing
104+
105+
### Use `@ts-expect-error` for Negative Assertions
106+
107+
```typescript
108+
it('should reject invalid handlers', () => {
109+
new Flow({ slug: 'test' })
110+
// @ts-expect-error - should reject null return
111+
.array({ slug: 'bad' }, () => null)
112+
113+
// @ts-expect-error - should reject undefined return
114+
.array({ slug: 'bad2' }, () => undefined);
115+
});
116+
```
117+
118+
**Benefits:**
119+
- ✅ Actually enforces code doesn't compile
120+
- ✅ Detected by typecheck-ts2578.sh via TS2578
121+
- ✅ Tests real-world usage patterns
122+
123+
### Why Both?
124+
125+
- **`expectTypeOf`**: Tests type relationships and transformations
126+
- **`@ts-expect-error`**: Tests that invalid code is actually rejected
127+
128+
Using both provides comprehensive coverage and catches different classes of bugs.
129+
130+
## Troubleshooting
131+
132+
### Health Check Fails
133+
134+
1. Check TypeScript version: `pnpm tsc --version`
135+
2. Check Vitest version: `pnpm vitest --version`
136+
3. Review `tsconfig.typecheck.json`
137+
4. Check `typecheck-ts2578.sh` script
138+
5. Look at canary file errors: `pnpm tsc --noEmit __tests__/types/__canary__.test-d.ts`
139+
140+
### Type Tests Pass But Should Fail
141+
142+
This usually means:
143+
- Type signatures are too permissive
144+
- Missing `@ts-expect-error` directives
145+
- Run health check to verify infrastructure works
146+
147+
### TS2578 Not Detected
148+
149+
- Ensure `typecheck-ts2578.sh` runs correctly
150+
- Check that file is included in typecheck
151+
- Verify `@ts-expect-error` is on its own line above the code
152+
153+
## Adding New Type Tests
154+
155+
1. **Choose the right tool**:
156+
- Positive assertion (what type IS) → `expectTypeOf`
157+
- Negative assertion (what shouldn't compile) → `@ts-expect-error`
158+
159+
2. **Follow existing patterns** in the test files
160+
161+
3. **Run health check** after major changes:
162+
```bash
163+
pnpm nx test:types:health dsl
164+
```
165+
166+
4. **Test your tests**: Temporarily break the type signature and ensure the test fails
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* TYPE TESTING HEALTH CHECK FILE
3+
*
4+
* This file intentionally contains type errors to verify that our type testing
5+
* infrastructure is working correctly. It should NEVER pass type checking.
6+
*
7+
* ⚠️ DO NOT INCLUDE IN NORMAL TEST RUNS ⚠️
8+
* ⚠️ EXCLUDED FROM tsconfig.typecheck.json ⚠️
9+
*
10+
* Purpose:
11+
* - Verifies that expectTypeOf detects type mismatches
12+
* - Verifies that @ts-expect-error unused directive detection works
13+
* - Verifies that typecheck-ts2578.sh catches errors correctly
14+
*
15+
* Verified by: scripts/verify-type-test-health.sh
16+
* Run with: pnpm nx test:types:health dsl
17+
*/
18+
19+
import { describe, it, expectTypeOf } from 'vitest';
20+
import { Flow } from '../../src/index.js';
21+
22+
describe('Health: expectTypeOf detects type mismatches', () => {
23+
it('MUST FAIL: detects wrong return type with clear error message', () => {
24+
const flow = new Flow<{ count: number }>({ slug: 'health_test' })
25+
.array({ slug: 'items' }, () => [1, 2, 3]);
26+
27+
const arrayStep = flow.getStepDefinition('items');
28+
29+
// This MUST fail with: "Expected: string, Actual: number"
30+
expectTypeOf(arrayStep.handler).returns.toEqualTypeOf<string[]>();
31+
});
32+
33+
it('MUST FAIL: detects input type mismatch', () => {
34+
const flow = new Flow<{ userId: string }>({ slug: 'health_test2' })
35+
.step({ slug: 'step1' }, () => 42);
36+
37+
const step = flow.getStepDefinition('step1');
38+
39+
// This MUST fail - expecting number input but it's { run: { userId: string } }
40+
expectTypeOf(step.handler).parameter(0).toEqualTypeOf<{ run: { userId: number } }>();
41+
});
42+
});
43+
44+
describe('Health: @ts-expect-error unused directive detection', () => {
45+
it('MUST FAIL: detects unused @ts-expect-error (TS2578)', () => {
46+
const validArray = new Flow({ slug: 'health_test3' })
47+
.array({ slug: 'numbers' }, () => [1, 2, 3]);
48+
49+
// These @ts-expect-error directives are UNUSED because the types are actually valid
50+
// They MUST trigger TS2578 errors in Pass 2 of typecheck-ts2578.sh
51+
52+
// @ts-expect-error - HEALTH: This is actually valid, so TS2578 should fire
53+
const step1 = validArray.getStepDefinition('numbers');
54+
55+
// @ts-expect-error - HEALTH: This is actually valid, so TS2578 should fire
56+
const handler = step1.handler;
57+
58+
// Suppress unused variable warnings
59+
void handler;
60+
});
61+
});
62+
63+
describe('Health: Hybrid approach validation', () => {
64+
it('MUST FAIL: both expectTypeOf and @ts-expect-error work together', () => {
65+
const flow = new Flow<{ value: number }>({ slug: 'health_hybrid' })
66+
.step({ slug: 's1' }, () => 'result');
67+
68+
// expectTypeOf assertion that MUST fail
69+
expectTypeOf(flow.getStepDefinition('s1').handler).returns.toEqualTypeOf<number>();
70+
71+
// @ts-expect-error that is unused (no actual error) - MUST trigger TS2578
72+
// @ts-expect-error - HEALTH: This line is actually valid
73+
const validStep = flow.getStepDefinition('s1');
74+
void validStep;
75+
});
76+
});

0 commit comments

Comments
 (0)