-
Notifications
You must be signed in to change notification settings - Fork 397
add types for event-subscriptions #3814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
All contributors have signed the CLA ✍️ ✅ |
this looks pretty solid to me! |
I have read the CLA Document and I hereby sign the CLA |
recheck |
Service extends string, | ||
Type extends string, | ||
Source, | ||
Payload = {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should probably default to Record<string, never>
instead? (assuming this is meant to be an empty object)
type SubscriptionEventSourceWorkflows = { | ||
workflowId: string; | ||
workflowName: string; | ||
versionId: string; | ||
instanceId: string; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit weird that workflows has a helper for this, but superSlurper doesn't. Since this type will be exposed to users, I wonder if we should inline it?
Event Subscriptions let you receive events about other CF products as messages on to a Queue. I'm unsure what other context you all need to be able to review this, but I'm happy to provide it.
WIP Docs change: cloudflare/cloudflare-docs#21197