-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add TypeScript types #585
Add TypeScript types #585
Conversation
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.
That's what I've found missing with my current step function I'm building.
Also when I try to export the state machine to a single file it fails with:
|
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
What do you mean with exporting it to a single file? The types use module augmentation to merge the types into the official AWS type: serverless-step-functions/lib/index.d.ts Lines 3 to 10 in c464dcd
If you want to use the types as stand-alone, we should probably export the Is that what you mean? |
Nope. What I wanted to have in a different file is the stateMachine implementation. Not the types. So I can have a file for each StateMachine and split things so I don't have a huge serverless.ts file. I've also found that we are missing |
Yes, I think the issue wouldn't appear if you could type your stateMachine like this: export const stateMachine1: StateMachine = {
// ...
}; I assume the issue is related to type widening by TypeScript. So the last error message as an example:
The state machine expects |
Ok, I didn't know the term or what was really happening but that's exactly the issue. Then what we could do is export the single type |
Co-authored-by: Enric Bisbe Gil <[email protected]>
Everything is working from my side. Not sure if we are missing anything. Edit: A few updates. |
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
Co-authored-by: Enric Bisbe Gil <[email protected]>
@horike37 could you please take a look if you have the chance. If you prefer not to include TS types in this package, we can move this PR to the DefinitelyTyped repo. |
Parameters?: { | ||
[key: string]: string | Array<unknown> | { [key: string]: string }; | ||
}; | ||
Result?: { |
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.
Result?: { | |
Result?: string | { |
Result?: { | |
Result?: string | [] | { |
I'm using Result: "${env:something}"
to add an array. Not sure what's the best approach here. The string value resolves into an array. The tip from the Ui console says: Must be valid JSON.
Array and object are the only ones valid.
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.
Should we try something like this: microsoft/TypeScript#1897 (comment)
export type JSONPrimitive = string | number | boolean | null;
export type JSONValue = JSONPrimitive | JSONObject | JSONArray;
export type JSONObject = { [member: string]: JSONValue };
export interface JSONArray extends Array<JSONValue> {}
Result?: { | |
Result?: JSONValue; |
@zirkelc |
@horike37 thanks for you reply! Alright, I'll submit a PR to DefinitelyTyped in the next few days. |
@ebisbe I created a new PR in the DefinetlyTyped repo: DefinitelyTyped/DefinitelyTyped#66693 I didn't try it yet, but I think to get the types merged from this package, we have to use module augmentation in our
|
I don't know what's that it. I will try to help you on the other PR. |
It means add types from package
We have to wait for the approval on the DT repo. |
Ok. What I don't like on this PR is that we have been doing it manually... It would be perfect if we could extract the full state machine definition and convert it to types so it can be automated. Here it's for CF resources https://github.com/awboost/cfntypes ( only sadly ) |
Yes, I totally agree! Unfortunately, there is no official JSON schema that we can rely on. The official spec doesn't publish any specification: https://states-language.net/spec.html There is this package that provides validation for ASL: https://github.com/ChristopheBougere/asl-validator It has some JSON schemas for the actual states: https://github.com/ChristopheBougere/asl-validator/tree/main/src/schemas It might be possible to generate the types with this package https://www.npmjs.com/package/json-schema-to-typescript |
@zirkelc this package has a validation option. Let me see what is using under the hood. EDIT: It's using "asl-validator": "^3.8.0", the package you mentioned. |
This PR adds TypeScript types as discussed in #370
First, you need to use the official TypeScript @serverless/typescript package. for your
serverless.ts
file.TypeScript merges the types from this package with the base types from @serverless/typescript with either of these two options:
/// <reference types="serverless-step-functions" />
at the top of yourserverless.ts
"types": ["serverless-step-functions"]
to yourtsconfig.json
I can't guarantee that the types are fully complete and correct, but I think there are a good starting point.
If you rather prefer to have the types provided as a separate
@types/serverless-step-functions
package, I'd be willing to submit a PR to the DefinetlyTypes package.