(feat): add support for default query handler #1639
Conversation
packages/workflow/src/workflow.ts
Outdated
| const activator = assertInWorkflowContext( | ||
| 'Workflow.setDefaultQueryHandler(...) may only be used from a Workflow Execution.' | ||
| ); | ||
| if (typeof handler === 'function') { |
There was a problem hiding this comment.
nit: You can combine the if and else if branches here.
packages/workflow/src/internals.ts
Outdated
| let fn = this.queryHandlers.get(queryName)?.handler; | ||
| // Fallback to default query handler if no handler is registered. | ||
| if (fn === undefined) { | ||
| fn = this.defaultQueryHandler; |
There was a problem hiding this comment.
The default query handler will need an extra argument, the first one, that indicates the name of the query being called. See signalWorkflowNextHandler().
There was a problem hiding this comment.
Fixed, improved the test to catch this case. Thanks
…andler properly, improve test
packages/workflow/src/interfaces.ts
Outdated
| /** | ||
| * A handler function accepting query calls for non-registered query names. | ||
| */ | ||
| export type DefaultQueryHandler = (queryName: string, ...args: any[]) => any; |
There was a problem hiding this comment.
nit: That should be unknown rather than any (both occurrences).
packages/workflow/src/internals.ts
Outdated
| // Fallback to default query handler if no handler is registered. | ||
| if (fn === undefined && this.defaultQueryHandler !== undefined) { | ||
| // Use non-null assertion because Typescript can't tell that defaultQueryHandler cannot be undefined here. | ||
| fn = (...args) => this.defaultQueryHandler!(queryName, ...args); |
There was a problem hiding this comment.
TS is technically right: even though you just checked that this.defaultQueryHandler is not undefined, you are then using that field inside a function call; there is no guarantee that this.defaultQueryHandler will still be valid by the time fn gets called. Well, we know it will, but the compiler can't figure this out.
You can avoid the TS error without a non-null assertion by copying this.defaultQueryHandler to a temporary variable inside the if block, or by using this.defaultQueryHandler.bind(...) instead:
| fn = (...args) => this.defaultQueryHandler!(queryName, ...args); | |
| fn = this.defaultQueryHandler.bind(this, queryName); |
THardy98
changed the title
Added support for to register a default query handler - a handler for queries that do not have registered query names/handlers.
Part of [Feature Request] Support dynamic workflows, activities, signals, queries, and updates #1015
How was this tested:
Added small integration tests.
Any docs updates needed?
Maybe