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

Add TypeScript types #585

Closed
wants to merge 15 commits into from

Conversation

zirkelc
Copy link
Contributor

@zirkelc zirkelc commented Sep 7, 2023

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:

  • Add /// <reference types="serverless-step-functions" /> at the top of your serverless.ts
/// <reference types="serverless-step-functions" />
import type { AWS as Serverless } from '@serverless/typescript';
const serverlessConfiguration: Serverless = {
  // ...
};
  • Add "types": ["serverless-step-functions"] to your tsconfig.json
{
  "compilerOptions": {
    // ...
    "types": [
      "serverless-step-functions"
    ]
  },
  "include": [
    "src/**/*.ts",
    "serverless.ts",
  ],
}

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.

@zirkelc zirkelc mentioned this pull request Sep 7, 2023
Copy link

@ebisbe ebisbe left a 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.

lib/index.d.ts Outdated Show resolved Hide resolved
lib/index.d.ts Outdated Show resolved Hide resolved
lib/index.d.ts Show resolved Hide resolved
lib/index.d.ts Outdated Show resolved Hide resolved
lib/index.d.ts Show resolved Hide resolved
@ebisbe
Copy link

ebisbe commented Sep 7, 2023

Also when I try to export the state machine to a single file it fails with:

Cannot load "serverless.ts": Initialization error: TSError: ⨯ Unable to compile TypeScript:
serverless.ts:650:7 - error TS2322: Type '{ events: { schedule: { rate: string; enabled: boolean; }; }[]; dependsOn: string[]; definition: { StartAt: string; States: { "Add empty LastEvaluatedKey": { Type: string; Next: string; Result: { LastEvaluatedKey: string; }; ResultPath: string; }; ... 4 more ...; "Add new LastEvaluatedKey": { ...; }; }; }; }' is not assignable to type '{ id?: string; name?: string; events?: any; dependsOn?: any; definition: Definition; tracingConfig?: TracingConfig; }'.
  The types of 'definition.States' are incompatible between these types.
    Type '{ "Add empty LastEvaluatedKey": { Type: string; Next: string; Result: { LastEvaluatedKey: string; }; ResultPath: string; }; Scan: { Type: string; Next: string; Parameters: { TableName: { Ref: string; }; FilterExpression: string; ExpressionAttributeNames: { ...; }; ExpressionAttributeValues: { ...; }; "ExclusiveStart...' is not assignable to type 'States'.
      Property '"Add empty LastEvaluatedKey"' is incompatible with index signature.
        Type '{ Type: string; Next: string; Result: { LastEvaluatedKey: string; }; ResultPath: string; }' is not assignable to type 'Choice | Fail | Map | Task | Parallel | Pass | Wait | Succeed'.
          Type '{ Type: string; Next: string; Result: { LastEvaluatedKey: string; }; ResultPath: string; }' is not assignable to type 'Pass'.
            Types of property 'Type' are incompatible.
              Type 'string' is not assignable to type '"Pass"'.

650       addCredits,
          ~~~~~~~~~~

  node_modules/serverless-step-functions/lib/index.d.ts:13:3
     13   [stateMachine: string]: {
          ~~~~~~~~~~~~~~~~~~~~~~~~~
     14     id?: string;
        ~~~~~~~~~~~~~~~~
    ... 
     19     tracingConfig?: TracingConfig;
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     20   };
        ~~~~
    The expected type comes from this index signature.
    ```

lib/index.d.ts Outdated Show resolved Hide resolved
zirkelc and others added 5 commits September 7, 2023 17:40
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
@zirkelc
Copy link
Contributor Author

zirkelc commented Sep 7, 2023

Also when I try to export the state machine to a single file it fails with:

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:

declare module '@serverless/typescript' {
interface AWS {
stepFunctions?: {
stateMachines: StateMachines;
validate?: boolean;
};
}
}

If you want to use the types as stand-alone, we should probably export the stepFunctions type separately.

Is that what you mean?

@ebisbe
Copy link

ebisbe commented Sep 7, 2023

Also when I try to export the state machine to a single file it fails with:

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:

declare module '@serverless/typescript' {
interface AWS {
stepFunctions?: {
stateMachines: StateMachines;
validate?: boolean;
};
}
}

If you want to use the types as stand-alone, we should probably export the stepFunctions type separately.

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 type?: 'STANDARD' | 'EXPRESS'; and loggingConfig?: any; ( #281 )

@zirkelc
Copy link
Contributor Author

zirkelc commented Sep 7, 2023

Also when I try to export the state machine to a single file it fails with:

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:

declare module '@serverless/typescript' {
interface AWS {
stepFunctions?: {
stateMachines: StateMachines;
validate?: boolean;
};
}
}

If you want to use the types as stand-alone, we should probably export the stepFunctions type separately.
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.

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:

Types of property 'Type' are incompatible.
Type 'string' is not assignable to type '"Pass"'.

The state machine expects type to be a string literal of Pass or Task etc. But when you define your state machine outside of the serverless type, TypeScript widens the type from literal Pass to string. That causes the type error I guess.

@ebisbe
Copy link

ebisbe commented Sep 7, 2023

I assume the issue is related to type widening by TypeScript. So the last error message as an example:

Types of property 'Type' are incompatible.
Type 'string' is not assignable to type '"Pass"'.

The state machine expects type to be a string literal of Pass or Task etc. But when you define your state machine outside of the serverless type, TypeScript widens the type from literal Pass to string. That causes the type error I guess.

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 StateMachine

lib/index.d.ts Outdated Show resolved Hide resolved
lib/index.d.ts Show resolved Hide resolved
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
@ebisbe
Copy link

ebisbe commented Sep 8, 2023

Everything is working from my side. Not sure if we are missing anything.

Edit: A few updates.

lib/index.d.ts Outdated Show resolved Hide resolved
lib/index.d.ts Outdated Show resolved Hide resolved
lib/index.d.ts Show resolved Hide resolved
zirkelc and others added 3 commits September 10, 2023 09:34
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
lib/index.d.ts Show resolved Hide resolved
lib/index.d.ts Show resolved Hide resolved
lib/index.d.ts Show resolved Hide resolved
lib/index.d.ts Show resolved Hide resolved
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
zirkelc and others added 3 commits September 11, 2023 11:44
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
Co-authored-by: Enric Bisbe Gil <enricu@gmail.com>
@zirkelc
Copy link
Contributor Author

zirkelc commented Sep 11, 2023

@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?: {
Copy link

Choose a reason for hiding this comment

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

Suggested change
Result?: {
Result?: string | {
Suggested change
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.

Copy link
Contributor Author

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> {}
Suggested change
Result?: {
Result?: JSONValue;

@horike37
Copy link
Collaborator

@zirkelc
Thank you for this PR! Adding the type system should be really important. But right now we can't regularly maintain this repo, so it would be great if you can move this PR to the DefinitelyTyped repo.

@zirkelc
Copy link
Contributor Author

zirkelc commented Sep 13, 2023

@horike37 thanks for you reply! Alright, I'll submit a PR to DefinitelyTyped in the next few days.

@zirkelc
Copy link
Contributor Author

zirkelc commented Sep 13, 2023

@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 serverless.ts file directly:

import type { AWS } from '@serverless/typescript';
import type { StepFunctions } from 'serverless-step-functions';

declare module '@serverless/typescript' {
   interface AWS {
     stepFunctions?: StepFunctions;
  }
}

@ebisbe
Copy link

ebisbe commented Sep 14, 2023

we have to use module augmentation

I don't know what's that it. I will try to help you on the other PR.

@zirkelc
Copy link
Contributor Author

zirkelc commented Sep 14, 2023

It means add types from package serverless-step-functions into the base types of @serverless/typescript. However, I figured out that type packages can install other type packages. So we get it to work without add this part at the top:

import type { AWS } from '@serverless/typescript';
import type { StepFunctions } from 'serverless-step-functions';

declare module '@serverless/typescript' {
   interface AWS {
     stepFunctions?: StepFunctions;
  }
}

We have to wait for the approval on the DT repo.

@ebisbe
Copy link

ebisbe commented Sep 14, 2023

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 )

@zirkelc
Copy link
Contributor Author

zirkelc commented Sep 14, 2023

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 could be possible to generate TS types from these files, but I didn't check if it's complete.

It might be possible to generate the types with this package https://www.npmjs.com/package/json-schema-to-typescript

@ebisbe
Copy link

ebisbe commented Sep 14, 2023

@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.

@horike37 horike37 closed this Oct 1, 2023
@zirkelc zirkelc deleted the typescript-types branch October 2, 2023 06:51
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.

None yet

3 participants