Skip to content

Commit 60e4b6c

Browse files
TS strict (3/3): runtime model/form/service changes + enable strict mode (#3688)
* TS strict (2/4): strictNullChecks null-safety guards Mechanical null-safety: nullish-coalescing defaults, optional chaining, {#if} render guards, and generics-driven refactors. Behavior-preserving. Rebased onto main after the base PR (#3686) merged. * TS strict (3/4): runtime-affecting model, form, and service changes WorkflowExecution optional fields (with their snapshot tests), schedule-form zod default seeding, standalone-activity retry fields as strings coerced at the boundary, duration-input defaults, and requestFromAPI's Promise<T | undefined>. * TS strict: enable strict mode and enforce it in CI Fold the strict: true flip + Danger warn->fail into this PR. The standalone-activity form's superForm types only resolve correctly under strictNullChecks, so the behavior changes can't type-check on a strict:false main — the flag must land with them. Supersedes the separate flag PR (#3640).
1 parent ee6348b commit 60e4b6c

43 files changed

Lines changed: 1061 additions & 745 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

dangerfile.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,15 @@ async function checkStrictModeErrors() {
104104
const relevantErrors: Record<string, StrictError[]> = {};
105105
let relevantErrorCount = 0;
106106

107+
// svelte-check and Danger both report repo-relative POSIX paths; compare
108+
// by normalized equality (substring matching both missed regressions and
109+
// false-blocked unrelated files that share a path suffix).
110+
const normalizePath = (p: string) => p.replace(/^\.?\//, '');
111+
107112
for (const [filename, errors] of Object.entries(result.errorsByFile)) {
113+
const normalizedFilename = normalizePath(filename);
108114
const isChanged = Array.from(changedFiles).some(
109-
(changedFile) =>
110-
changedFile.includes(filename) || filename.includes(changedFile),
115+
(changedFile) => normalizePath(changedFile) === normalizedFilename,
111116
);
112117

113118
if (isChanged) {
@@ -120,25 +125,23 @@ async function checkStrictModeErrors() {
120125
return;
121126
}
122127

123-
const percentageInPR = (
124-
(relevantErrorCount / result.totalErrors) *
125-
100
126-
).toFixed(1);
128+
const fileCount = Object.keys(relevantErrors).length;
127129

128-
// Post summary warning with all errors
129-
let warningMessage = `📊 **Strict Mode**: ${relevantErrorCount} error${relevantErrorCount > 1 ? 's' : ''} in ${Object.keys(relevantErrors).length} file${Object.keys(relevantErrors).length > 1 ? 's' : ''} (${percentageInPR}% of ${result.totalErrors} total)\n\n`;
130+
// The codebase is strict-clean; any strict error in a changed file is a
131+
// regression and must block the PR.
132+
let failureMessage = `❌ **Strict Mode**: ${relevantErrorCount} TypeScript error${relevantErrorCount > 1 ? 's' : ''} in ${fileCount} changed file${fileCount > 1 ? 's' : ''}. Strict mode is enforced — please fix before merging.\n\n`;
130133

131134
for (const [filename, errors] of Object.entries(relevantErrors)) {
132-
warningMessage += `<details>\n<summary><strong>${filename}</strong> (${errors.length})</summary>\n\n`;
135+
failureMessage += `<details>\n<summary><strong>${filename}</strong> (${errors.length})</summary>\n\n`;
133136

134137
for (const error of errors) {
135-
warningMessage += `- L${error.start.line}:${error.start.character}: ${error.message}\n`;
138+
failureMessage += `- L${error.start.line}:${error.start.character}: ${error.message}\n`;
136139
}
137140

138-
warningMessage += '\n</details>\n\n';
141+
failureMessage += '\n</details>\n\n';
139142
}
140143

141-
warn(warningMessage);
144+
fail(failureMessage);
142145

143146
// Post individual warnings for errors on lines in the diff
144147
for (const [filename, errors] of Object.entries(relevantErrors)) {
@@ -183,13 +186,13 @@ async function checkStrictModeErrors() {
183186
);
184187
}
185188

186-
// Post warnings for errors on lines that are in the diff
189+
// Post inline failures for errors on lines that are in the diff
187190
for (const error of errors) {
188191
if (linesInDiff.has(error.start.line)) {
189192
if (DEBUG) {
190-
console.log(`Posting warning for ${filename}:${error.start.line}`);
193+
console.log(`Posting failure for ${filename}:${error.start.line}`);
191194
}
192-
warn(error.message, filename, error.start.line);
195+
fail(error.message, filename, error.start.line);
193196
}
194197
}
195198
}

scripts/count-strict-errors.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,13 @@ try {
6060
if (data.type === 'ERROR') {
6161
const error: StrictError = {
6262
filename: data.filename,
63-
start: data.start,
63+
// svelte-check's machine-verbose output uses 0-indexed LSP
64+
// positions; convert to 1-indexed to match editors and GitHub
65+
// inline-comment line numbers (Danger's `fail(msg, file, line)`).
66+
start: {
67+
line: data.start.line + 1,
68+
character: data.start.character + 1,
69+
},
6470
message: data.message.split('\n')[0], // Only first line
6571
};
6672

src/lib/components/activity/activity-options-update-drawer.svelte

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@
3030
};
3131
3232
let { open = $bindable(), namespace, execution, activity }: Props = $props();
33-
let { activityId: id, activityType: type } = $derived(activity);
33+
let { activityId: id } = $derived(activity);
3434
3535
// Form fields are seeded from activity.activityOptions once when the drawer
3636
// opens. They must NOT reset reactively if the activity prop changes while
3737
// the user is mid-edit — untrack() captures the initial value intentionally.
3838
let taskQueue = $state(
39-
untrack(() => activity.activityOptions?.taskQueue?.name),
39+
untrack(() => activity.activityOptions?.taskQueue?.name ?? ''),
4040
);
4141
let scheduleToCloseTimeout = $state(
4242
untrack(() =>
@@ -65,10 +65,14 @@
6565
),
6666
);
6767
let maximumAttempts = $state(
68-
untrack(() => activity?.activityOptions?.retryPolicy?.maximumAttempts),
68+
untrack(() => activity?.activityOptions?.retryPolicy?.maximumAttempts ?? 0),
6969
);
70+
// Temporal rejects backoffCoefficient < 1; default to the server default (2)
71+
// when unset rather than submitting an invalid 0 via the full field mask.
7072
let backoffCoefficient = $state(
71-
untrack(() => activity?.activityOptions?.retryPolicy?.backoffCoefficient),
73+
untrack(
74+
() => activity?.activityOptions?.retryPolicy?.backoffCoefficient ?? 2,
75+
),
7276
);
7377
let initialInterval = $state(
7478
untrack(() =>
@@ -125,7 +129,7 @@
125129
console.error('Error updating activity options:', error);
126130
toaster.push({
127131
variant: 'error',
128-
message: `Options for Activity ${id} have been failed to update: ${error?.message}`,
132+
message: `Options for Activity ${id} have been failed to update: ${(error as Error)?.message}`,
129133
duration: 5000,
130134
});
131135
} finally {

src/lib/components/schedule/schedule-form/schedule-policies-drawer.svelte

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@
225225
initialUnit={getFirstWholeNumberUnit(
226226
$policies.catchupWindow,
227227
durationUnits,
228+
'second(s)',
228229
)}
229230
units={durationUnits}
230231
error={!!$errors.catchupWindow?.[0]}
@@ -263,6 +264,7 @@
263264
initialUnit={getFirstWholeNumberUnit(
264265
$policies.taskTimeout,
265266
durationUnits,
267+
'second(s)',
266268
)}
267269
units={durationUnits}
268270
error={!!$errors.taskTimeout?.[0]}
@@ -282,6 +284,7 @@
282284
initialUnit={getFirstWholeNumberUnit(
283285
$policies.runTimeout,
284286
durationUnits,
287+
'second(s)',
285288
)}
286289
units={durationUnits}
287290
error={!!$errors.runTimeout?.[0]}
@@ -300,6 +303,7 @@
300303
initialUnit={getFirstWholeNumberUnit(
301304
$policies.executionTimeout,
302305
durationUnits,
306+
'second(s)',
303307
)}
304308
units={durationUnits}
305309
error={!!$errors.executionTimeout?.[0]}

src/lib/components/schedule/schedule-form/schedule-spec-item.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
}
5151
5252
if (specKind === 'frozen') {
53-
return spec.calendar ? 'Calendar' : 'Interval';
53+
return spec.interval?.interval ? 'Interval' : 'Calendar';
5454
}
5555
5656
return translate('schedules.spec-type-none');

src/lib/components/schedule/schema/form.ts

Lines changed: 67 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -78,78 +78,81 @@ const formSpecKind = z.enum([
7878
]);
7979
export type FormSpecKind = z.infer<typeof formSpecKind>;
8080

81-
export const formSpecSchema = z
82-
.object({
83-
kind: formSpecKind,
84-
cronString: z.string().trim().default(''),
85-
interval: z
86-
.object({
87-
interval: durationString().optional(),
88-
phase: durationString().optional(),
89-
})
90-
.default({}),
91-
calendar: structuredCalendarSchema(),
92-
})
93-
.superRefine((spec, ctx) => {
94-
switch (spec.kind) {
95-
case 'none': {
81+
// The bare object schema, without the cross-field validation below. Parsing a
82+
// partial seed through this injects the field defaults without tripping the
83+
// refinements (e.g. "cron string is required"), yielding a complete spec.
84+
export const formSpecObject = z.object({
85+
kind: formSpecKind,
86+
cronString: z.string().trim().default(''),
87+
interval: z
88+
.object({
89+
interval: durationString().optional(),
90+
phase: durationString().optional(),
91+
})
92+
.default({}),
93+
calendar: structuredCalendarSchema(),
94+
});
95+
96+
export const formSpecSchema = formSpecObject.superRefine((spec, ctx) => {
97+
switch (spec.kind) {
98+
case 'none': {
99+
ctx.addIssue({
100+
code: z.ZodIssueCode.custom,
101+
message: 'Type is required ',
102+
path: ['kind'],
103+
});
104+
return;
105+
}
106+
107+
case 'cron': {
108+
if (!spec.cronString) {
96109
ctx.addIssue({
97110
code: z.ZodIssueCode.custom,
98-
message: 'Type is required ',
99-
path: ['kind'],
111+
message: 'Cron expression is required',
112+
path: ['cronString'],
100113
});
101-
return;
102114
}
103115

104-
case 'cron': {
105-
if (!spec.cronString) {
106-
ctx.addIssue({
107-
code: z.ZodIssueCode.custom,
108-
message: 'Cron expression is required',
109-
path: ['cronString'],
110-
});
111-
}
112-
113-
if (!isValidCronString(spec.cronString)) {
114-
ctx.addIssue({
115-
code: z.ZodIssueCode.custom,
116-
message:
117-
'Cron string format invalid. Format: minute (0-59) hour (0-23) day-of-month (1-31) month (1-12) day-of-week (0-6) separated by a space',
118-
path: ['cronString'],
119-
});
120-
}
121-
122-
return;
116+
if (!isValidCronString(spec.cronString)) {
117+
ctx.addIssue({
118+
code: z.ZodIssueCode.custom,
119+
message:
120+
'Cron string format invalid. Format: minute (0-59) hour (0-23) day-of-month (1-31) month (1-12) day-of-week (0-6) separated by a space',
121+
path: ['cronString'],
122+
});
123123
}
124124

125-
case 'frozen': {
126-
return;
127-
}
125+
return;
126+
}
128127

129-
case 'interval': {
130-
if (
131-
!spec.interval?.interval ||
132-
!Number(parseDuration(spec.interval.interval))
133-
) {
134-
ctx.addIssue({
135-
code: z.ZodIssueCode.custom,
136-
message: 'Interval is required',
137-
path: ['interval.interval'],
138-
});
139-
}
140-
141-
return;
142-
}
128+
case 'frozen': {
129+
return;
130+
}
143131

144-
case 'month': {
145-
return;
132+
case 'interval': {
133+
if (
134+
!spec.interval?.interval ||
135+
!Number(parseDuration(spec.interval.interval))
136+
) {
137+
ctx.addIssue({
138+
code: z.ZodIssueCode.custom,
139+
message: 'Interval is required',
140+
path: ['interval.interval'],
141+
});
146142
}
147143

148-
case 'week': {
149-
return;
150-
}
144+
return;
151145
}
152-
});
146+
147+
case 'month': {
148+
return;
149+
}
150+
151+
case 'week': {
152+
return;
153+
}
154+
}
155+
});
153156

154157
export type FormSpecSchema = z.infer<typeof formSpecSchema>;
155158
const formScheduleTimingSchema = z.object({
@@ -209,7 +212,10 @@ export const formScheduleSchema = z
209212
});
210213
}
211214

212-
if (schedule.endAfterOccurrences <= 0) {
215+
if (
216+
typeof schedule.endAfterOccurrences === 'number' &&
217+
schedule.endAfterOccurrences <= 0
218+
) {
213219
ctx.addIssue({
214220
code: z.ZodIssueCode.custom,
215221
message: 'Number of occurrences must be greater than 0',

src/lib/components/schedule/utilities/get-form-schedule-defaults.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ describe('getFormScheduleDefaults', () => {
5353
});
5454

5555
it('seeds a single cron spec', () => {
56-
expect(defaults.specs).toEqual([{ kind: 'cron', cronString: '' }]);
56+
expect(defaults.specs).toHaveLength(1);
57+
expect(defaults.specs[0].kind).toBe('cron');
58+
expect(defaults.specs[0].cronString).toBe('');
5759
});
5860

5961
it('uses the default policies and timeouts', () => {
@@ -127,9 +129,12 @@ describe('getFormScheduleDefaults', () => {
127129
});
128130

129131
it('loads the interval spec', () => {
130-
expect(defaults.specs).toEqual([
131-
{ kind: 'frozen', interval: { interval: '3600s', phase: '0s' } },
132-
]);
132+
expect(defaults.specs).toHaveLength(1);
133+
expect(defaults.specs[0].kind).toBe('frozen');
134+
expect(defaults.specs[0].interval).toEqual({
135+
interval: '3600s',
136+
phase: '0s',
137+
});
133138
});
134139

135140
it('parses the overlap policy and timeouts', () => {

src/lib/components/schedule/utilities/get-form-spec-initial-data.test.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import { describe, expect, it } from 'vitest';
33
import { getFormSpecInitialData } from './get-form-spec-initial-data';
44

55
describe('getFormSpecInitialData', () => {
6-
it('returns an empty cron spec', () => {
7-
expect(getFormSpecInitialData('cron')).toEqual({
8-
kind: 'cron',
9-
cronString: '',
10-
});
6+
it('returns a cron spec with defaults injected', () => {
7+
const spec = getFormSpecInitialData('cron');
8+
9+
expect(spec.kind).toBe('cron');
10+
expect(spec.cronString).toBe('');
11+
expect(spec.interval).toEqual({});
12+
expect(spec.calendar).toBeDefined();
1113
});
1214

1315
it('seeds a week spec with the current day of the week', () => {
@@ -29,13 +31,11 @@ describe('getFormSpecInitialData', () => {
2931
expect(spec.calendar?.month).toEqual([{ start: now.getMonth() + 1 }]);
3032
});
3133

32-
it('returns an empty interval spec', () => {
33-
expect(getFormSpecInitialData('interval')).toEqual({
34-
kind: 'interval',
35-
interval: {
36-
interval: undefined,
37-
phase: undefined,
38-
},
39-
});
34+
it('returns an interval spec with an empty interval', () => {
35+
const spec = getFormSpecInitialData('interval');
36+
37+
expect(spec.kind).toBe('interval');
38+
expect(spec.interval).toEqual({});
39+
expect(spec.calendar).toBeDefined();
4040
});
4141
});

0 commit comments

Comments
 (0)