feat(workflows-sdk): Allow a step to not define an expected input

[adrien2p]

What
Allow a step to not define an expected input, previously even if no input was expected, an object was always expected to be passed to the stepFunction inside the workflow composition. Now

const stepWithoutArgs = createStep("step1", () => {
  return new StepResponse("string")
})

const stepWithoutExepectedInput = createStep("step2", (_: {}, context) => {
  console.log("input", _) // {}
  return new StepResponse("string")
})

const workflow = createWorkflow("workflow1", () => {
  stepWithoutArgs()
  return stepWithoutExepectedInput()
})

workflow()
  .run()
  .then((res) => {
    console.log(res.result) // string
  })

[changeset-bot]

🦋 Changeset detected

Latest commit: 9a0d54e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@medusajs/workflows-sdk	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
api-reference	⬜️ Ignored (Inspect)	Visit Preview		Dec 21, 2023 1:20pm
docs-ui	⬜️ Ignored (Inspect)	Visit Preview		Dec 21, 2023 1:20pm
medusa-docs	⬜️ Ignored (Inspect)	Visit Preview		Dec 21, 2023 1:20pm

[olivermrbl]

/snapshot-this

[github-actions]

🚀 A snapshot release has been made for this PR

Test the snapshots by updating your package.json with the newly published versions:

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add [email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

Latest commit: 428331a

[olivermrbl]

Two questions:

• How do you type a createStep that doesn't take an input?

• Have you tested this with an example of createStep that expects an input type? I am faced with the following error:
  

[olivermrbl]

Second question continued: I can see the type of my createStep export is wrong. It expects no arguments, even though it is typed to expect one:

[adrien2p]

for the first question, you dont type it or you put undefined, unknown etc. for the second I ve pushed the fix earlier

[carlos-r-l-rodrigues]

Can we have a function with no input, that has access to the context?

[adrien2p]

Yes you can, if you do (input, context) => something or (_, context) => something, the input will not be required and will be undefined and the context will be present.

On the other hand, if you do (input: YourInterface, context) then inout become required and you will have to provide it.

We have to keep in mind that since context is always present and after the input which can be optional, we are just playing tricks with the typings 😅

[carlos-r-l-rodrigues]

yeah, but with the additional parameter that won't be used.
but it is alright, as we are limiting the arguments to always be two. Doesn't feel natural, but I won't raise that argument again 😆

[olivermrbl]

/snapshot-this

[github-actions]

🚀 A snapshot release has been made for this PR

Test the snapshots by updating your package.json with the newly published versions:

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add [email protected]

yarn add [email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

yarn add @medusajs/[email protected]

Latest commit: 85cda7c

[olivermrbl]

Just reiterating the DX to ensure we are aligned:

const step = createStep("step-1", async (input, context) => {
  return new StepResponse("");
});

const workflow = createWorkflow<any, any>("hello-world", function (input) {

  step(); // no TS error
  step({}); // TS error
  
});

const step2 = createStep("step-2", async () => {
  return new StepResponse("");
});

const workflow2 = createWorkflow<any, any>("hello-world", function (input) {

  step2(); // no TS error
  step2({}); // TS error

});

const step3 = createStep("step-2", async (input: Record<string, unknown>, context) => {
  return new StepResponse("");
});

const workflow3 = createWorkflow<any, any>("hello-world", function (input) {

  step3(); // TS error
  step3({}); // no TS error

});

So essentially, an untyped input will be treated as if the step doesn't expect any arguments. However, the argument defaults to an empty object.

I am on board with this.

The only thing left to discuss would be if we should allow for the empty object to be passed without a TS error.

So this case:

const step3 = createStep("step-2", async (_) => {
  return new StepResponse("");
});

const workflow3 = createWorkflow<any, any>("hello-world", function (input) {

  step3(); // no TS error
  step3({}); // no TS error

});

I can see why it could make sense to allow for this, as the untyped argument would default to an empty object anyway. But then again, why would you pass an empty object if you are aware that the step doesn't take an input.

In my opinion, this doesn't have a significant impact on the DX, so I would be fine to go with either.

[adrien2p]

@olivermrbl I would just add for the following case

const workflow3 = createWorkflow<any, any>("hello-world", function (input) {

  step3(); // no TS error
  step3({}); // no TS error

});

That the ts type provided for the user is something along the line of step3: () => something if we allow for empty object to be authorized then they will have something like step3: (() => something) | ((input: {}) => something) in order to indicate that the input is not expected. Feels a bit weird in my opinion but just wanted to add those elements.

Also, this dx is only concerning the ts users since for js users they can do that

[olivermrbl]

Couldn't we create the type such that it would be: step3: ((input?: {}) => something)

[adrien2p]

Couldn't we create the type such that it would be: step3: ((input?: {}) => something)

Yes we can, but I thought the idea at the beginning was to display to the user that there is no expected argument. Now it feels like the argument can be passed and it will change something except that it won't. Up to you.

The type I provided you above was more to stress the fact that we are saying you can pass nothing, or you can pass something. But it gives the feeling that it will change something in the behaviour like you would expect for any method handling an optional params, except that here the intention is to say nothing needs to be provided.

[adrien2p]

My argument is not about the typing itself, but more about what information are we trying to provide the user with

[adrien2p]

It really sounds like for example, we take the retrieveDefaultCurrency that we have and do not expect any args, and suddenly we say to the user through the typings that he can still pass an empty object as the argument event though the method doesn't expect one, but it won't change the behavior. I just feel wrong about it. If you really have a strong opinion I ll go with it

[olivermrbl]

As I mentioned above, I don't think it matters much from a DX perspective, so I am fine to go with what we have now. Just wanted to put the suggestion out there to hear your thoughts on it. I think you raise a couple of good points 👍

[olivermrbl]

LGTM, pending @srindom's approval

[srindom]

LGTM

[olivermrbl]

@adrien2p, feel free to merge this one when conflicts are resolved :)

[adrien2p]

Perfect thank you man, ill do that 👌