From 70b24ec57eaf24bb2ebf66df40cdac4bc4baa254 Mon Sep 17 00:00:00 2001 From: SandroMaglione Date: Sat, 7 Nov 2026 15:19:32 +0900 Subject: [PATCH] Name the fix in compile errors for targets, sources, owners, and final states Report a misspelled target, source, or owner by listing what is declared, where these used to surface as `never`. Run validation before the handler shape so its message wins, and read the whole diagnostic chain in a test, as an agent does. Reject a target below a state that is certainly inactive and has a value, which used to fail while planning. This found a devtools example that would have thrown. Co-Authored-By: Claude Opus 5.5 --- .changeset/actionable-type-errors.md | 17 +++ package.json | 2 +- .../browser/hierarchy-routing-example.ts | 8 +- .../test/internal/browser/StaticChart.test.ts | 2 +- packages/effect-machine/src/Machine.ts | 135 +++++++++++++++--- .../typetest/machine/Transitions.tst.ts | 17 ++- scripts/type-errors.test.mjs | 130 +++++++++++++++++ 7 files changed, 277 insertions(+), 34 deletions(-) create mode 100644 .changeset/actionable-type-errors.md create mode 100644 scripts/type-errors.test.mjs diff --git a/.changeset/actionable-type-errors.md b/.changeset/actionable-type-errors.md new file mode 100644 index 00000000..3d780aad --- /dev/null +++ b/.changeset/actionable-type-errors.md @@ -0,0 +1,17 @@ +--- +"@typeonce/effect-machine": minor +--- + +Compile errors for common machine mistakes now name the fix instead of reporting `never`: + +```ts +Pay: { target: "Archivd" } +// Unknown target "Archivd". Declared: Editing, Checkout, Archived + +invoke: { src: "loadd", onDone: { stay: true } } +// Unknown source. Registered: load +``` + +`Machine.handle` reports a misspelled target, source, or owner, a missing `data`, the root or a history state used as a target, and an `on`, `always`, `onDone`, or `invoke` on a final or choice state. + +A transition that targets a state below an inactive valued state, such as `{ target: "Checkout.Paid" }` from outside `Checkout`, used to fail while planning. It is now a compile error: target `"Checkout"` with a `branches` entry and construct the child with `select.target("Checkout", { data, states: { Paid: {} } })`. diff --git a/package.json b/package.json index 41955e16..643ea37e 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "visualizer": "pnpm devtools", "visualizer:build": "pnpm --dir packages/devtools build", "check:architecture": "node --test scripts/check-architecture.test.mjs && node scripts/check-architecture.mjs packages/effect-machine && node scripts/check-devtools-architecture.mjs packages/devtools", - "check:ci": "node --test scripts/ci-changes.test.mjs scripts/effect-compatibility.test.mjs scripts/invoke-autocomplete.test.mjs scripts/release-contract.test.mjs scripts/runtime-performance-compatibility.test.mjs scripts/runtime-performance-regression.test.mjs", + "check:ci": "node --test scripts/ci-changes.test.mjs scripts/effect-compatibility.test.mjs scripts/invoke-autocomplete.test.mjs scripts/type-errors.test.mjs scripts/release-contract.test.mjs scripts/runtime-performance-compatibility.test.mjs scripts/runtime-performance-regression.test.mjs", "typecheck": "pnpm --recursive --filter \"./packages/**\" run check && tsc -p tsconfig.tests.json && tsc -p ai-docs/tsconfig.json", "typecheck:ts7": "pnpm --dir tooling/typescript-7 run typecheck", "perf:types": "pnpm build && node scripts/type-performance.mjs", diff --git a/packages/devtools/src/internal/browser/hierarchy-routing-example.ts b/packages/devtools/src/internal/browser/hierarchy-routing-example.ts index e4bce13e..f200f0c4 100644 --- a/packages/devtools/src/internal/browser/hierarchy-routing-example.ts +++ b/packages/devtools/src/internal/browser/hierarchy-routing-example.ts @@ -68,8 +68,12 @@ export const hierarchyRoutingMachine = Machine.make({ id: "save-review", onDone: { target: "Workflow.Publishing" }, onFailure: { - target: "Workflow.Review.Failed", - data: () => ({ message: "The review could not be saved." }) + branches: [{ target: "Workflow.Review" }], + resolve: ({ select }) => + select.target("Workflow.Review", { + data: { title: "A deterministic chart" }, + states: { Failed: { data: { message: "The review could not be saved." } } } + }) } } }, diff --git a/packages/devtools/test/internal/browser/StaticChart.test.ts b/packages/devtools/test/internal/browser/StaticChart.test.ts index 19c084ad..91840c00 100644 --- a/packages/devtools/test/internal/browser/StaticChart.test.ts +++ b/packages/devtools/test/internal/browser/StaticChart.test.ts @@ -784,7 +784,7 @@ describe("Static chart", () => { assert.isAtLeast(parentTransition.points.length, 2) assert.isAtLeast(externalFailure.points.length, 2) assert.strictEqual(parentTransition.points[0]?.y, source.y + source.headerHeight) - assert.isAtLeast(Math.min(...externalFailure.points.map(({ y }) => y)), target.y) + assert.isAtLeast(Math.min(...externalFailure.points.map(({ y }) => y)), source.y) const directDistance = Math.abs(externalFailure.points[0]!.x - externalFailure.points.at(-1)!.x) + Math.abs(externalFailure.points[0]!.y - externalFailure.points.at(-1)!.y) assert.isAtMost(chartRouteLength(externalFailure.points), directDistance + 80) diff --git a/packages/effect-machine/src/Machine.ts b/packages/effect-machine/src/Machine.ts index ee1b3871..ad212027 100644 --- a/packages/effect-machine/src/Machine.ts +++ b/packages/effect-machine/src/Machine.ts @@ -6519,6 +6519,17 @@ export declare namespace Machine { } & InlineDestinationConstruction }[DestinationPath] + // A misspelled target matches this member, so validation can name the declared paths instead of `never`. + type InlineUnknownTarget = { + /** Selects a declared descendant, constructing the values it requires. */ + readonly target: ReferenceUnion> | (string & {}) + readonly update?: never + readonly initialize?: never + readonly history?: never + readonly stay?: never + readonly branches?: never + readonly resolve?: never + } type InlineUpdate, C> = [UpdatableOwner] extends [never] ? never : { /** Replaces the value of one retained source or ancestor without replacing its active children. */ @@ -6650,6 +6661,7 @@ export declare namespace Machine { } & DeclarationData< | InlineDestination + | InlineUnknownTarget | InlineUpdate | InlineRoot | { @@ -7042,6 +7054,8 @@ export declare namespace Machine { readonly exit?: never /** Constructs the decoded output declared by the state schema. */ readonly output?: never } + type StateFieldError = + `${Kind} state "${PathName}" cannot declare ${Field}. ${Fix}` /** * Handlers for a node of the declared root tree. * @@ -7078,13 +7092,13 @@ export declare namespace Machine { /** Required total transition for a transient choice state. */ readonly choice: Transition, Ev, Em, R> /** Event handlers keyed by the public or internal event tag. */ - readonly on?: never + readonly on?: StateFieldError<"Choice", Src, "on", "Declare one transition in choice"> /** Eventless transition evaluated during stabilization. */ - readonly always?: never + readonly always?: StateFieldError<"Choice", Src, "always", "Declare one transition in choice"> /** Handles successful invocation or state completion with its exact output type. */ - readonly onDone?: never + readonly onDone?: StateFieldError<"Choice", Src, "onDone", "Declare one transition in choice"> /** One registered invocation or an array with distinct lifecycle identities. */ - readonly invoke?: never + readonly invoke?: StateFieldError<"Choice", Src, "invoke", "Declare one transition in choice"> /** Child handlers nested according to the declared root topology. */ readonly states?: never /** Restores the declared history reference, using its fallback on first entry. */ @@ -7092,15 +7106,15 @@ export declare namespace Machine { } : Src extends StateIdentifier ? Node extends { readonly type: "final" } ? { /** Event handlers keyed by the public or internal event tag. */ - readonly on?: never + readonly on?: StateFieldError<"Final", Src, "on", "A final state has no outgoing work"> /** Eventless transition evaluated during stabilization. */ - readonly always?: never + readonly always?: StateFieldError<"Final", Src, "always", "A final state has no outgoing work"> /** Handles successful invocation or state completion with its exact output type. */ - readonly onDone?: never + readonly onDone?: StateFieldError<"Final", Src, "onDone", "A final state has no outgoing work"> /** Required total transition for a transient choice state. */ readonly choice?: never /** One registered invocation or an array with distinct lifecycle identities. */ - readonly invoke?: never + readonly invoke?: StateFieldError<"Final", Src, "invoke", "A final state has no outgoing work"> /** Child handlers nested according to the declared root topology. */ readonly states?: never /** Restores the declared history reference, using its fallback on first entry. */ @@ -7253,6 +7267,67 @@ export declare namespace Machine { [Result] extends [void | undefined] ? unknown : { readonly commands: "commands must not return a value. Enqueue work with its second argument" } : unknown + type DestinationNames = ReferenceUnion, HistoryIdentifier>> + type UnknownTargetMessage

= P extends "root" + ? `Cannot target "root". Reconstruct it with initialize, or update it with update: { root: ... }` + : P extends ReferenceUnion> ? + `"${P}" is a history state. Restore it with: { history: "${P}" }` + : `Unknown target "${P}". Declared: ${JoinUnion>}` + // A property-level message would collapse with the declared literal to `never`, so these replace the whole value. + type ValidateTarget = T extends { + readonly target: infer P extends string + } ? string extends P ? unknown + : P extends DestinationNames ? ValidateTargetAncestors & ValidateTargetData + : UnknownTargetMessage + : unknown + // A known target also matches the misspelled-target member, which cannot require its values. + type ValidateTargetData = "data" extends keyof T ? unknown + : PathOf

extends infer Id extends StateIdentifier ? + [NodeSchema>] extends [never] ? unknown + : {} extends NodeMakeInput> ? unknown + : `Target "${P}" requires data` + : unknown + // Entering below an inactive valued state has no value for it, so that state must be the target. Anything below the + // source or a parallel state may already be active, so only a state that is certainly inactive is reported here. + type HasParallelAncestor = true extends { + [A in Extract, StateIdentifier>]: NodeByIdentifier extends + { readonly type: "parallel" } ? true : false + }[Extract, StateIdentifier>] ? true : + false + type InactiveValuedAncestor = { + [ + A in Exclude< + Extract>, ValuedStateIdentifier>, + Src | ParentStateIdentifier | JoinPath + > + ]: HasParallelAncestor extends true ? never : A + }[ + Exclude< + Extract>, ValuedStateIdentifier>, + Src | ParentStateIdentifier | JoinPath + > + ] + type ValidateTargetAncestors = + [InactiveValuedAncestor] extends [never] ? unknown : + `Target "${P}" enters "${JoinUnion< + PathName> + >}", which needs a value. Target that state and construct "${P}" with states` + type ValidateBranchEntries = T extends { + readonly branches: infer B extends ReadonlyArray + } ? { + readonly branches: { + readonly [K in keyof B]: + & (B[K] extends { readonly target: infer P extends string } ? + P extends DestinationNames ? ValidateTargetAncestors : UnknownTargetMessage + : unknown) + & (B[K] extends { readonly update: infer O extends string } ? O extends UpdatableNames ? unknown + : `Cannot update "${O}". ${[UpdatableNames] extends [never] ? + "No retained state can be updated here" + : `Updatable here: ${JoinUnion>}`}` : + unknown) + } + } + : unknown type ValidateDestination = Extract< keyof T, "target" | "update" | "initialize" | "history" | "stay" | "branches" | "resolve" @@ -7285,7 +7360,9 @@ export declare namespace Machine { ]: never } & ValidateDestination + & ValidateTarget & ValidateBranches + & ValidateBranchEntries & ValidateUpdateOwners & ValidateCommands type AllowedInvokeChannels = @@ -7323,17 +7400,18 @@ export declare namespace Machine { I extends { /** Name of the source registered in make. */ readonly src: infer Src extends PropertyKey - } ? { - readonly [K in keyof I]: [Duplicate] extends [never] ? - ValidateInvokeField, Root, S, Path> - : K extends ("id" extends keyof I ? "id" : "src") ? InvocationDefinitionError< - `Invocation id '${Duplicate}' is declared more than once in this state`, - Extract, - K - > - : ValidateInvokeField, Root, S, Path> - } : - never + } ? Src extends SourceKeys ? { + readonly [K in keyof I]: [Duplicate] extends [never] ? + ValidateInvokeField, Root, S, Path> + : K extends ("id" extends keyof I ? "id" : "src") ? InvocationDefinitionError< + `Invocation id '${Duplicate}' is declared more than once in this state`, + Extract, + K + > + : ValidateInvokeField, Root, S, Path> + } + : UnknownSourceMessage + : never type ValidateInvokeField< I, K extends keyof I, @@ -7346,6 +7424,9 @@ export declare namespace Machine { ValidateObjectTransition & ValidateChoiceUpdate : K extends "src" | "id" | "address" | "input" ? unknown : InvokeFieldError + type UnknownSourceMessage = [Extract, string>] extends [never] + ? "Unknown source. Register it in make with effects, streams, timers, logic, or children" + : `Unknown source. Registered: ${JoinUnion, string>>}` type ValidateObjectInvoke = I extends ReadonlyArray ? number extends I["length"] ? { readonly [K in keyof I]: ValidateInvokeEntry } @@ -7459,13 +7540,19 @@ export declare namespace Machine { } // Reverse-map authored fields to avoid expanding every possible transition. // Retain missing fields from H for required handlers and editor completion. - type HandlerShape = + // Intersecting a misspelled `src` with its declared literals is `never`, so that invocation takes the message instead. + type InvokeShape = I extends ReadonlyArray<{ readonly src: infer Src }> ? + [Exclude>] extends [never] ? H : UnknownSourceMessage + : I extends { readonly src: infer Src } ? [Src] extends [SourceKeys] ? H : UnknownSourceMessage + : H + type HandlerShape = & { readonly [K in keyof C]: K extends keyof H ? K extends "states" ? { readonly [P in keyof C[K]]: P extends keyof NonNullable - ? HandlerShape[P]>> + ? HandlerShape[P]>, R> : never } + : K extends "invoke" ? InvokeShape : H[K] : never } @@ -7493,13 +7580,15 @@ export declare namespace Machine { config: & C & (C extends (...args: never[]) => unknown ? never : unknown) + // Checked first, so an invalid declaration reports the message that names its fix. + & ValidateObjectConfig, R, S> & HandlerShape< C, & StateHandler>, In, Pa, R & { readonly "~input": I }> - & RootConstruction + & RootConstruction, + R > & RootConstruction - & ValidateObjectConfig, R, S> & RootHandlerValidation< S, Ev, diff --git a/packages/effect-machine/typetest/machine/Transitions.tst.ts b/packages/effect-machine/typetest/machine/Transitions.tst.ts index e62f9400..9950e442 100644 --- a/packages/effect-machine/typetest/machine/Transitions.tst.ts +++ b/packages/effect-machine/typetest/machine/Transitions.tst.ts @@ -21,7 +21,7 @@ const Events = Machine.events({ Bump: { by: Schema.Number }, Pay: { amount: Sche const Emits = Machine.emittedEvents({ Cancelled: {} }) const definition = Machine.make({ root: Root, events: Events, emittedEvents: Emits }) const handleEditing = { root: { count: 0 }, initial: { target: "Editing", data: { complete: false } } } as const -declare const widened: "Archived" | "Checkout.Paid" +declare const widened: "Archived" | "Editing" describe("transition declarations", () => { it("types every plain transition and every select call from the local declaration", () => { @@ -34,8 +34,8 @@ describe("transition declarations", () => { Bump: { stay: true, commands: (_, enqueue) => enqueue.emit(Emits.Cancelled()) }, Pay: { branches: [ - { target: "Checkout.Charging" }, - { target: "Checkout.Paid" }, + { target: "Checkout" }, + { target: "Archived" }, { update: "Editing" }, { stay: true }, { decline: true } @@ -46,8 +46,11 @@ describe("transition declarations", () => { if (event.amount === 1) return select.update({ Editing: { complete: true } }) if (event.amount === 2) return select.stay() return event.amount === 0 - ? select.target("Checkout.Paid") - : select.target("Checkout.Charging", { data: { amount: event.amount } }) + ? select.target("Archived") + : select.target("Checkout", { + data: { total: 0 }, + states: { Charging: { data: { amount: event.amount } } } + }) } } } @@ -154,7 +157,7 @@ describe("transition declarations", () => { Editing: { on: { Pay: { - branches: [{ target: "Archived" }, { target: "Checkout.Paid" }], + branches: [{ target: "Archived" }, { target: "Editing" }], resolve: ({ select }) => select.target(widened) } } @@ -162,7 +165,7 @@ describe("transition declarations", () => { } })).type.toRaiseError( "", - "select.target takes one path, got: Archived, Checkout.Paid. Call it once per path" + "select.target takes one path, got: " ) }) diff --git a/scripts/type-errors.test.mjs b/scripts/type-errors.test.mjs new file mode 100644 index 00000000..7f9bf638 --- /dev/null +++ b/scripts/type-errors.test.mjs @@ -0,0 +1,130 @@ +import { strict as assert } from "node:assert" +import path from "node:path" +import { test } from "node:test" +import ts from "typescript" + +// Compiles invalid definitions and reads the whole diagnostic chain, as an agent reading `tsc` output does. +const projectRoot = path.resolve(import.meta.dirname, "../packages/effect-machine") +const virtualFile = path.join(projectRoot, "type-errors.fixture.ts") +const prelude = ` +import { Effect, Schema } from "effect" +import { Machine } from "./src/index.js" +const Root = Machine.state({ + fields: { count: Schema.Number }, + states: { + Editing: { fields: { complete: Schema.Boolean } }, + Checkout: { + fields: { total: Schema.Number }, + states: { + Charging: { fields: { amount: Schema.Number } }, + Route: { type: "choice" }, + Paid: {}, + recent: { type: "history" } + } + }, + Archived: { type: "final" } + } +}) +const Events = Machine.events({ Pay: { amount: Schema.Number } }) +const def = Machine.make({ root: Root, events: Events, effects: { load: Effect.succeed(1) } }) +const bare = Machine.make({ root: Root, events: Events }) +const base = { root: { count: 0 }, initial: { target: "Editing", data: { complete: false } } } as const +` +const cases = { + "a misspelled target": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { target: "Archivd" } } } } })`, + `Unknown target "Archivd". Declared: ` + ], + "a misspelled target in branches": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { branches: [{ target: "Archivd" }], resolve: ({ select }) => select.stay() } } } } })`, + `Unknown target "Archivd". Declared:` + ], + "the root as a target": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { target: "root" } } } } })`, + `Cannot target "root". Reconstruct it with initialize, or update it with update: { root: ... }` + ], + "a history state as a target": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { target: "Checkout.recent" } } } } })`, + `"Checkout.recent" is a history state. Restore it with: { history: "Checkout.recent" }` + ], + "a target below an inactive valued state": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { target: "Checkout.Paid" } } } } })`, + `Target "Checkout.Paid" enters "Checkout", which needs a value. Target that state and construct "Checkout.Paid" with states` + ], + "a target without its required data": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { target: "Checkout" } } } } })`, + `Target "Checkout" requires data` + ], + "a misspelled source": [ + `def.handle({ ...base, states: { Editing: { invoke: { src: "loadd", onDone: { stay: true } } } } })`, + `Unknown source. Registered: load` + ], + "a source when none is registered": [ + `bare.handle({ ...base, states: { Editing: { invoke: { src: "load", onDone: { stay: true } } } } })`, + `Unknown source. Register it in make with effects, streams, timers, logic, or children` + ], + "a misspelled source in a list": [ + `def.handle({ ...base, states: { Editing: { invoke: [{ src: "load", onDone: { stay: true } }, { src: "loadd", id: "b", onDone: { stay: true } }] } } })`, + `Unknown source. Registered: load` + ], + "an unknown owner": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { update: { Typo: { count: 1 } } } } } } })`, + `Cannot update "Typo". Updatable here:` + ], + "an unknown owner in branches": [ + `def.handle({ ...base, states: { Checkout: { initial: { target: "Checkout.Charging", data: { amount: 1 } }, states: { Charging: { on: { Pay: { branches: [{ target: "Checkout.Paid", update: "Typo" }], resolve: ({ select }) => select.stay() } } } } } } })`, + `Cannot update "Typo". Updatable here:` + ], + "two owners": [ + `def.handle({ ...base, on: { Pay: { update: { root: { count: 1 }, Editing: { complete: true } } } } })`, + `update takes one owner, got: ` + ], + "a transition on a final state": [ + `def.handle({ ...base, states: { Archived: { on: { Pay: { stay: true } } } } })`, + `Final state "Archived" cannot declare on. A final state has no outgoing work` + ], + "an event handler on a choice": [ + `def.handle({ ...base, states: { Checkout: { initial: { target: "Checkout.Charging", data: { amount: 1 } }, states: { Route: { choice: { target: "Checkout.Paid" }, on: { Pay: { stay: true } } } } } } })`, + `Choice state "Checkout.Route" cannot declare on. Declare one transition in choice` + ], + "a declared branch that is never selected": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { branches: [{ target: "Archived" }, { stay: true }], resolve: ({ select }) => select.stay() } } } } })`, + `Declared branch { target: "Archived" } is never selected by resolve` + ], + "a selected branch that is not declared": [ + `def.handle({ ...base, states: { Editing: { on: { Pay: { branches: [{ target: "Archived" }], resolve: ({ select }) => select.stay() } } } } })`, + `resolve selects undeclared { stay: true }. Add it to branches` + ] +} + +const names = Object.keys(cases) +const source = prelude + names.map((name, index) => `export const case${index} = ${cases[name][0]}\n`).join("") +const config = ts.readConfigFile(path.join(projectRoot, "tsconfig.json"), ts.sys.readFile) +const parsed = ts.parseJsonConfigFileContent(config.config, ts.sys, projectRoot) +const host = { + ...ts.createCompilerHost(parsed.options), + fileExists: (file) => file === virtualFile || ts.sys.fileExists(file), + readFile: (file) => file === virtualFile ? source : ts.sys.readFile(file), + getSourceFile: (file, languageVersion) => { + const text = file === virtualFile ? source : ts.sys.readFile(file) + return text === undefined ? undefined : ts.createSourceFile(file, text, languageVersion, true) + } +} +const program = ts.createProgram([virtualFile], { ...parsed.options, noEmit: true, composite: false, incremental: false }, host) +const unescape = (text) => text.replaceAll("\\\"", "\"") +const lines = source.split("\n") +const messagesByCase = new Map(names.map((name) => [name, []])) +for (const diagnostic of program.getSemanticDiagnostics(program.getSourceFile(virtualFile))) { + const line = lines[diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start).line] + const index = line.match(/^export const case(\d+) =/)?.[1] + if (index !== undefined) { + messagesByCase.get(names[Number(index)]).push(unescape(ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n"))) + } +} + +for (const name of names) { + test(`names the fix for ${name}`, () => { + const messages = messagesByCase.get(name).join("\n") + assert.ok(messages.includes(cases[name][1]), `expected "${cases[name][1]}" in:\n${messages.slice(0, 600)}`) + }) +}