Skip to content
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

feat(framework): Add InferWorkflowId and InferWorkflowPayload inference utility types #6812

Closed
wants to merge 3 commits into from

Conversation

rifont
Copy link
Collaborator

@rifont rifont commented Oct 31, 2024

What changed? Why was the change needed?

  • Make workflowId a generic type on DiscoverWorkflowOutput and the workflow resource to allow the identifier to be inferred
  • Add InferWorkflowId and InferWorkflowPayload inference utils
    • These inference utility types enable developers in user-land to take their defined workflows and do things like creating bulk trigger utilities and pass typings between their workflow declarations whilst maintaining strong type-safety with triggers.

Related Docs PR: novuhq/docs#726

Screenshots

See the Stackblitz example for live type-safety
https://stackblitz.com/edit/novu-type-safe-bulk-trigger?file=api%2Fnovu%2Froute.tsx&view=editor

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

Copy link

netlify bot commented Oct 31, 2024

Deploy Preview for novu-stg-vite-dashboard-poc failed. Why did it fail? →

Name Link
🔨 Latest commit 8891f87
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/6723f13b13c83700088c3d8d

Copy link

pkg-pr-new bot commented Oct 31, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/novuhq/novu/@novu/framework@6812

commit: 8891f87


export type InferWorkflowId<T_Workflow extends Workflow> = T_Workflow extends Workflow<infer Id, never> ? Id : never;

export type InferWorkflowPayload<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lovely DX improvement, but I feel we need to add the BulkEventParams and typedBulkTrigger utilities to the framework to maximize its DX benefits.

The instructions we give Novu users feel more convoluted than hardcoding a workflowId in a constant and reusing it in their code.

Alternatively, how about adding a triggerBulk function on the workflow object?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest we close this PR for the time being until the isomorphic @novu/api package is available and ready for close integration with @novu/framework. Then, everything can be bundled together in a much simpler way for user-land consumption.

@rifont
Copy link
Collaborator Author

rifont commented Dec 3, 2024

Closing for now, see #6812 (comment)

@rifont rifont closed this Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants