Return value for `monday.api()` function in SDK

monday Apps & Developers Feature Requests - API and Apps Framework
1

Hi @rflynn,
great to hear that the sdk is now type safe!
However, when I try to execute a query to fetch boards, I’m getting a TS error while trying to grab the fetched data:
grafik

Is there anything I’m missing?
Thank you in advance!

Cheers
Jona

2

From what I understand, the Typescript support is currently limited to the SDK methods and not the results returned by monday. Those you will have to handle yourself

3

In the client-api.interface.ts (see Github) the type is written like this:

// ...
api(query: string, options?: APIOptions): Promise<{ data: object }>;
// ...

I think that Promise<{data: object}> needs to be changed in order to have correct types or at least to have no TS errors (maybe to something like Promise<{data: Record<string, any>}>). Else one would have to disable type checking for all lines where one is trying to access res.data from an api call.

1 Like
4

Why do you have to display type checking for res.data? Giving that the data returned by res.data is dynamic, you can create your own custom interface or class and you it like this res: CustomInterface

5

Maybe my explanation wasn’t that good. So let me try to summarize:

  • For me it is fine to don’t have type checking on the res object, since - as you stated - res.data is dynamic and type checking would not really have any benifit because it would be only type casting and you never could be sure that the types are really met.
  • My problem with the current types is, that I cannot access any values from res.data because it is of type object and this type does not have any properties that I could access (see error in my screenshot above).
  • So for every access on the res.data object throughout all api calls in my code, we would have to do type casting so that TS wouldn’t complain. However, this seems to be quite an overhead for us since this would be a fix on our side due to misleading types in the monday sdk. E.g.
    api(query: string, options?: APIOptions): Promise<{ data: any }>;
// or
api(query: string, options?: APIOptions): Promise<{ data: Record<string, any>}>;
    would allow any access on res.data so that we don’t have to type cast it on our end.
  • Doing res: CustomInterface does not work because you cannot just overwrite existing typings

So, all in all, my issue is not that there is no concrete type checking but that there is “wrong” type checking with data: object.

6

Hi @j-ittermann-decadis,

Thanks for your clarification! I can see how this would cost some additional effort.

I’ve submitted these suggestions to our development team so they consider implementing this fix.

2 Likes
8

Good morning @alessandra,
are there any updates, e.g. maybe a public github issue I could watch to get updates regarding the implementation progress of the fix? Or will you post updates here as soon as there are some?

9

Hello @j-ittermann-decadis,

We do not have an update yet.

Any changes like this one would be announced in our changelog. You can an eye open over there to check our updates!

Cheers,
Matias

1 Like
10

Hey @j-ittermann-decadis – I like the suggestion of using a record for this.

But I want your 2c: if we changed the return type of api to Record, would it be a breaking change?

Aka, will the following be a breaking change for people already using the types?

Old version –

api(query: string, options?: APIOptions): Promise<{ data: object }>;

New version –

api(query: string, options?: APIOptions): Promise<{ data: Record<string, any>}>;

BTW, I moved this to a new topic to keep the original announcement clean!

1 Like
11

Hi @dipro,
thank you for taking a look and creating the new topic!

As far as I can estimate it, this should not be a breaking change because:

  1. Record<string, any> would still be an object. So when doing for example typeof data for both the old and new version, it would return "object". So anybody out there who is doing type checking like so would not see any difference in their outcome.
  2. object is not a real TS type you could work upon as I stated in the beginning of the post, so I’m assuming that everybody who wants to have here some type safety or in general wants to work with data, is already casting it to some other type. If they didn’t want to have type safety, e.g. by casting to type any, it would not make any difference of which type data is anyways.
  3. If people are doing type casting (2), e.g. data: SomeCustomType or data as SomeCustomType that means that TS allows that casting from object to SomeCustomType
  4. If (3) holds - and it should do since the TS transpiler is checking it - and having (1) and (2), there should not be anything that would be a breaking change after this fix.

Let me know, if you can follow my train of thought and if it does also make sense for you :smiley:

Cheers!

PS: FYI there is also some PR open for changing the types in another place where using Record is being considered, too.

2 Likes
12

You’re the best, thanks for the explanation! Sounds like it won’t be a breaking change.

Anyway, I’ll test it out a bit when making a PR, to double check.

13

I vote that ideal solution is using generics with a default of whatever is decided. That way we can specify the type when we call the method monday.api<CustomResponseType>() and the response will be of a type like:

interface MondayResponse<T = Record<string, any>> {
     data?: T
     error_code?: string //ideally a union type of all the error_code values
     // rest of the error related keys
}

This way if we provide our own type for T, then data will be our custom type, but otherwise Record<string, any> if T is not specified.

I think the response needs to be a type that includes more than data, we need to be checking if there are errors too still, correct? (unless the SDK is now throwing exceptions… I haven’t tested since I starting using my own custom client for NodeJS.)