Infer Workflow Result Type from WorkflowRunOperationHandler#1968
Infer Workflow Result Type from WorkflowRunOperationHandler#1968VegetarianOrc wants to merge 5 commits intomainfrom
Conversation
…from typed workflows startWorkflow now returns WorkflowHandle<WorkflowResultType<T>> instead of WorkflowHandle<T>, so the handler's output type is inferred as the workflow's return type (e.g. string) rather than the workflow function type. A new workflowResultType brand on WorkflowHandle makes WorkflowHandle<X> structurally distinct from WorkflowHandle<Y>, enabling TypeScript to catch type mismatches when explicit type params contradict the actual workflow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| /** | ||
| * A Nexus Operation implementation that is backed by a Workflow run. | ||
| * | ||
| * The type parameter `O` represents the operation's output type. When the handler uses |
| test('WorkflowRunOperationHandler infers correct output type from typed workflow function', async (t) => { | ||
| // When constructing WorkflowRunOperationHandler without explicit type parameters using a typed | ||
| // workflow function, the operation output type should be inferred as the workflow's return type | ||
| // (e.g. string), not the workflow function type (e.g. (input: string) => Promise<string>). |
There was a problem hiding this comment.
The last part of this comment will be unhelpful to future readers. It puts emphasis on what will then be a former bug, actually creating confusion.
Not totally convinced of the pertinence of the first two lines, but there's not doing any harm.
| // (e.g. string), not the workflow function type (e.g. (input: string) => Promise<string>). |
mjameswh
left a comment
There was a problem hiding this comment.
Minor comments. Please fix before merging,.
|
Should there be unit tests for these? Or are they tested in some other way? |
| * | ||
| * @experimental Nexus support in Temporal SDK is experimental. | ||
| */ | ||
| readonly [workflowResultType]: T; |
There was a problem hiding this comment.
Seems like a better structure IMHO.
| readonly [workflowResultType]: T; | |
| readonly [workflowResultType]: WorkflowResult<T>; |
There was a problem hiding this comment.
I explored this option, but it actually conflicts with explicit typing. Including that restricts T to T extends Workflow which has knock on effects that cause any explicitly provided type parameters to also extend Workflow. For example:
// Explicit type parameters should also work correctly.
const _explicitStringOp: nexus.OperationHandler<string, string> = new temporalnexus.WorkflowRunOperationHandler<
string,
string
>(async (ctx, input) => {
return await temporalnexus.startWorkflow(ctx, echoWorkflow, {
args: [input],
workflowId: 'test',
});
});Instead would need to be
// Explicit type parameters should also work correctly.
const _explicitStringOp: nexus.OperationHandler<string, string> = new temporalnexus.WorkflowRunOperationHandler<
string,
typeof echoWorkflow
>(async (ctx, input) => {
return await temporalnexus.startWorkflow(ctx, echoWorkflow, {
args: [input],
workflowId: 'test',
});
});
There are some existing tests for the functionality of the |
What was changed
startWorkflownow returns WorkflowHandle<WorkflowResultType> instead of WorkflowHandle, so the handler's output type is inferred as the workflow's return type (e.g. string) rather than the workflow function type. A new workflowResultType brand on WorkflowHandle makes WorkflowHandle structurally distinct from WorkflowHandle, enabling TypeScript to catch type mismatches when explicit type params contradict the actual workflow.Why?
Previously the
WorkflowRunOperationHandlerwas inferring the return type to be the workflow instead of the workflow's output type. This required you to explicitly type theWorkflowRunOperationHandlerin order to fulfill Nexus service contracts.Checklist
A test with the different combinations of typing has been added to assert compilation errors and correct inferrence.