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

Fix: change WorkflowStep into an abstract class #2916

Merged

Conversation

LuisDuarte1
Copy link
Contributor

For some reason, Typescript resolves types in a different away compared to abstract classes. As a consequence, it would not apply the overloading properly and always treats WorkflowStep.do as a 3 argument function

For some reason, Typescript resolves types in a different away compared
to abstract classes. As a consequence, it would not apply the
overloading properly and always treats WorkflowStep.do as a 3 argument function
@LuisDuarte1 LuisDuarte1 requested review from a team as code owners October 14, 2024 10:12
Copy link
Member

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

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

LGTM assuming this works. Have you tried generating the types with this and examined them or is it more of a guess-and-check situation?

@sidharthachatterjee
Copy link
Collaborator

LGTM assuming this works. Have you tried generating the types with this and examined them or is it more of a guess-and-check situation?

Good question. To avoid thrashing, let's generate these and test locally on smoke tests @LuisDuarte1

@LuisDuarte1
Copy link
Contributor Author

LGTM assuming this works. Have you tried generating the types with this and examined them or is it more of a guess-and-check situation?

I have overridden my local worker-types to see if it works, so no worries here

@a-robinson
Copy link
Member

Then no worries here either, but it looks like it can't be merged yet because "This branch has conflicts that must be resolved"

@LuisDuarte1 LuisDuarte1 force-pushed the lduarte/fix-workflow-step-types branch from 68dae44 to b46edf2 Compare October 14, 2024 16:32
@a-robinson a-robinson merged commit 03b071f into cloudflare:main Oct 14, 2024
10 checks passed
@LuisDuarte1 LuisDuarte1 deleted the lduarte/fix-workflow-step-types branch October 14, 2024 18:12
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.

4 participants