Skip to content

Commit 7a9134a

Browse files
committed
fix step/array/map overloads to use TOpts intersection pattern for skippability detection
1 parent bdabd07 commit 7a9134a

8 files changed

Lines changed: 144 additions & 34 deletions

pkgs/core/supabase/tests/condition_evaluation/dependent_step_condition_met.test.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ select pgflow.add_step(
1616
'{first}', -- depends on first
1717
null, null, null, null, -- default options
1818
'single', -- step_type
19-
'{"first": {"success": true}}'::jsonb, -- condition: first.success must be true
19+
'{"first": {"success": true}}'::jsonb, -- if: first.success must be true
2020
'skip' -- when_unmet
2121
);
2222

pkgs/core/supabase/tests/condition_evaluation/dependent_step_condition_unmet_skip.test.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ select pgflow.add_step(
1616
'{first}', -- depends on first
1717
null, null, null, null, -- default options
1818
'single', -- step_type
19-
'{"first": {"success": true}}'::jsonb, -- condition: first.success must be true
19+
'{"first": {"success": true}}'::jsonb, -- if: first.success must be true
2020
'skip' -- when_unmet
2121
);
2222

pkgs/core/supabase/tests/condition_evaluation/plain_skip_iterates_until_convergence.test.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ select pgflow.add_step(
2525
'{}', -- root step
2626
null, null, null, null,
2727
'single',
28-
'{"enabled": true}'::jsonb, -- condition: requires enabled=true
28+
'{"enabled": true}'::jsonb, -- if: requires enabled=true
2929
'skip' -- plain skip
3030
);
3131
select pgflow.add_step(
@@ -34,7 +34,7 @@ select pgflow.add_step(
3434
'{step_a}', -- depends on a
3535
null, null, null, null,
3636
'single',
37-
'{"step_a": {"success": true}}'::jsonb, -- condition: a.success must be true
37+
'{"step_a": {"success": true}}'::jsonb, -- if: a.success must be true
3838
'skip' -- plain skip (won't be met since a was skipped)
3939
);
4040
select pgflow.add_step(

pkgs/core/supabase/tests/condition_evaluation/plain_skip_propagates_to_map.test.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ select pgflow.add_step(
2121
'{}', -- root step
2222
null, null, null, null, -- default options
2323
'single', -- step_type
24-
'{"enabled": true}'::jsonb, -- condition: requires enabled=true
24+
'{"enabled": true}'::jsonb, -- if: requires enabled=true
2525
'skip' -- when_unmet - plain skip (not skip-cascade)
2626
);
2727
-- Map consumer: no condition, just depends on producer

pkgs/core/supabase/tests/condition_evaluation/skipped_deps_excluded_from_input.test.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ select pgflow.add_step(
2727
'{}', -- root step
2828
null, null, null, null,
2929
'single',
30-
'{"enabled": true}'::jsonb, -- condition: requires enabled=true
30+
'{"enabled": true}'::jsonb, -- if: requires enabled=true
3131
'skip' -- plain skip
3232
);
3333
select pgflow.add_step(
Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
import { Flow, type ExtractFlowSteps } from '../../src/index.js';
1+
import { Flow, type ExtractFlowSteps, type StepOutput } from '../../src/index.js';
22
import { describe, it, expectTypeOf } from 'vitest';
33

4+
// ExtractFlowSteps returns step slugs as keys
5+
// Use StepOutput<> to get the output type from a step
46
describe('ExtractFlowSteps utility type', () => {
5-
it('should correctly extract steps from a flow with defined input', () => {
7+
it('should correctly extract step slugs from a flow', () => {
68
const flow = new Flow<{ userId: number }>({ slug: 'user_flow' })
79
.step({ slug: 'fetchUser' }, () => ({ name: 'John', age: 30 }))
810
.step({ slug: 'fetchPosts', dependsOn: ['fetchUser'] }, () => [
@@ -12,15 +14,17 @@ describe('ExtractFlowSteps utility type', () => {
1214

1315
type Steps = ExtractFlowSteps<typeof flow>;
1416

15-
expectTypeOf<Steps>().toMatchTypeOf<{
16-
fetchUser: { name: string; age: number };
17-
fetchPosts: Array<{ id: number; title: string }>;
18-
}>();
17+
// Keys are step slugs
18+
expectTypeOf<keyof Steps>().toEqualTypeOf<'fetchUser' | 'fetchPosts'>();
1919

20-
// ensure it doesn't extract non-existent fields
21-
expectTypeOf<Steps>().not.toMatchTypeOf<{
22-
nonExistentStep: number;
20+
// Use StepOutput to get output types (public API)
21+
expectTypeOf<StepOutput<typeof flow, 'fetchUser'>>().toMatchTypeOf<{
22+
name: string;
23+
age: number;
2324
}>();
25+
expectTypeOf<StepOutput<typeof flow, 'fetchPosts'>>().toMatchTypeOf<
26+
Array<{ id: number; title: string }>
27+
>();
2428
});
2529

2630
it('should work with AnyFlow to extract steps from a generic flow', () => {
@@ -31,15 +35,14 @@ describe('ExtractFlowSteps utility type', () => {
3135

3236
type Steps = ExtractFlowSteps<typeof anyFlow>;
3337

34-
expectTypeOf<Steps>().toMatchTypeOf<{
35-
step1: number;
36-
step2: string;
37-
step3: { complex: { nested: boolean } };
38-
}>();
38+
// Keys are step slugs
39+
expectTypeOf<keyof Steps>().toEqualTypeOf<'step1' | 'step2' | 'step3'>();
3940

40-
// ensure it doesn't extract non-existent fields
41-
expectTypeOf<Steps>().not.toMatchTypeOf<{
42-
nonExistentStep: number;
41+
// Use StepOutput to verify output types
42+
expectTypeOf<StepOutput<typeof anyFlow, 'step1'>>().toEqualTypeOf<number>();
43+
expectTypeOf<StepOutput<typeof anyFlow, 'step2'>>().toEqualTypeOf<string>();
44+
expectTypeOf<StepOutput<typeof anyFlow, 'step3'>>().toMatchTypeOf<{
45+
complex: { nested: boolean };
4346
}>();
4447
});
4548

@@ -59,16 +62,15 @@ describe('ExtractFlowSteps utility type', () => {
5962

6063
type Steps = ExtractFlowSteps<typeof primitiveFlow>;
6164

62-
expectTypeOf<Steps>().toMatchTypeOf<{
63-
numberStep: number;
64-
stringStep: string;
65-
booleanStep: boolean;
66-
nullStep: null;
67-
}>();
65+
// Keys are step slugs
66+
expectTypeOf<keyof Steps>().toEqualTypeOf<
67+
'numberStep' | 'stringStep' | 'booleanStep' | 'nullStep'
68+
>();
6869

69-
// ensure it doesn't extract non-existent fields
70-
expectTypeOf<Steps>().not.toMatchTypeOf<{
71-
nonExistentStep: number;
72-
}>();
70+
// Use StepOutput to verify output types
71+
expectTypeOf<StepOutput<typeof primitiveFlow, 'numberStep'>>().toEqualTypeOf<number>();
72+
expectTypeOf<StepOutput<typeof primitiveFlow, 'stringStep'>>().toEqualTypeOf<string>();
73+
expectTypeOf<StepOutput<typeof primitiveFlow, 'booleanStep'>>().toEqualTypeOf<boolean>();
74+
expectTypeOf<StepOutput<typeof primitiveFlow, 'nullStep'>>().toEqualTypeOf<null>();
7375
});
7476
});

pkgs/dsl/__tests__/types/skippable-deps.test-d.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,3 +438,111 @@ describe('skippable deps type safety', () => {
438438
});
439439
});
440440
});
441+
442+
/**
443+
* Compile-time error tests for skippable deps
444+
*
445+
* These tests use @ts-expect-error to verify that TypeScript correctly
446+
* rejects invalid patterns when accessing skippable dependencies.
447+
*/
448+
describe('skippable deps compile-time errors', () => {
449+
describe('direct property access on optional deps', () => {
450+
it('should reject direct property access on skippable dep without null check', () => {
451+
new Flow<{ value: number }>({ slug: 'test' })
452+
.step({ slug: 'maybeSkipped', retriesExhausted: 'skip' }, () => ({
453+
data: 'result',
454+
}))
455+
.step({ slug: 'consumer', dependsOn: ['maybeSkipped'] }, (deps) => {
456+
// @ts-expect-error - deps.maybeSkipped is optional, cannot access .data directly
457+
const result: string = deps.maybeSkipped.data;
458+
return { result };
459+
});
460+
});
461+
462+
it('should reject direct property access with else: skip', () => {
463+
new Flow<{ value: number }>({ slug: 'test' })
464+
.step({ slug: 'conditional', if: { value: 42 }, else: 'skip' }, () => ({
465+
processed: true,
466+
}))
467+
.step({ slug: 'next', dependsOn: ['conditional'] }, (deps) => {
468+
// @ts-expect-error - deps.conditional is optional due to else: 'skip'
469+
const flag: boolean = deps.conditional.processed;
470+
return { flag };
471+
});
472+
});
473+
474+
it('should reject direct property access with else: skip-cascade', () => {
475+
new Flow<{ value: number }>({ slug: 'test' })
476+
.step(
477+
{ slug: 'cascading', if: { value: 42 }, else: 'skip-cascade' },
478+
() => ({ count: 10 })
479+
)
480+
.step({ slug: 'next', dependsOn: ['cascading'] }, (deps) => {
481+
// @ts-expect-error - deps.cascading is optional due to else: 'skip-cascade'
482+
const num: number = deps.cascading.count;
483+
return { num };
484+
});
485+
});
486+
487+
it('should reject direct property access with retriesExhausted: skip-cascade', () => {
488+
new Flow<{ value: number }>({ slug: 'test' })
489+
.step({ slug: 'risky', retriesExhausted: 'skip-cascade' }, () => ({
490+
status: 'ok',
491+
}))
492+
.step({ slug: 'next', dependsOn: ['risky'] }, (deps) => {
493+
// @ts-expect-error - deps.risky is optional due to retriesExhausted: skip-cascade
494+
const s: string = deps.risky.status;
495+
return { s };
496+
});
497+
});
498+
});
499+
500+
describe('mixed deps - optional and required', () => {
501+
it('should allow direct access on required dep but reject on optional', () => {
502+
new Flow<{ value: number }>({ slug: 'test' })
503+
.step({ slug: 'required' }, () => ({ reqData: 'always' }))
504+
.step({ slug: 'optional', retriesExhausted: 'skip' }, () => ({
505+
optData: 'maybe',
506+
}))
507+
.step(
508+
{ slug: 'consumer', dependsOn: ['required', 'optional'] },
509+
(deps) => {
510+
// This is fine - required dep is always present
511+
const req: string = deps.required.reqData;
512+
513+
// @ts-expect-error - deps.optional is optional, cannot access .optData directly
514+
const opt: string = deps.optional.optData;
515+
516+
return { req, opt };
517+
}
518+
);
519+
});
520+
});
521+
522+
describe('array and map steps with skip modes', () => {
523+
it('should reject direct access on skippable array step output', () => {
524+
new Flow<{ items: string[] }>({ slug: 'test' })
525+
.array({ slug: 'processed', retriesExhausted: 'skip' }, (input) =>
526+
input.items.map((s) => s.toUpperCase())
527+
)
528+
.step({ slug: 'consumer', dependsOn: ['processed'] }, (deps) => {
529+
// @ts-expect-error - deps.processed is optional, cannot access .length directly
530+
const len: number = deps.processed.length;
531+
return { len };
532+
});
533+
});
534+
535+
it('should reject direct access on skippable map step output', () => {
536+
new Flow<string[]>({ slug: 'test' })
537+
.map(
538+
{ slug: 'doubled', retriesExhausted: 'skip' },
539+
(item) => item + item
540+
)
541+
.step({ slug: 'consumer', dependsOn: ['doubled'] }, (deps) => {
542+
// @ts-expect-error - deps.doubled is optional, cannot access [0] directly
543+
const first: string = deps.doubled[0];
544+
return { first };
545+
});
546+
});
547+
});
548+
});

pkgs/dsl/src/dsl.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ export interface StepRuntimeOptions extends RuntimeOptions {
494494
/**
495495
* What to do when the 'if' pattern doesn't match the input
496496
*
497-
* @default 'fail'
497+
* @default 'skip'
498498
*
499499
* @example
500500
* { else: 'fail' } // Pattern doesn't match -> step fails -> run fails

0 commit comments

Comments
 (0)