MunifTanjim / node-bitbucket

Bitbucket API client for Browser and Node.js
https://bitbucketjs.netlify.app
MIT License
106 stars 28 forks source link

TypeScript: Many return values are optional #54

Closed robcresswell closed 4 years ago

robcresswell commented 4 years ago

Hello!

Had a general query about the library; it seems like almost every single return value is optional, which doesn't reflect Bitbuckets upstream APIs. What was the reasoning behind that? Are you open to PRs to update this?

Thanks for your help!

MunifTanjim commented 4 years ago

Can you give me an example please? Otherwise, I can't say why it happened or if it's intended or not...

robcresswell commented 4 years ago

Hi! Sorry for going AWOL, I was moved away from working with Bitbucket to some other tasks. Here's an example of the behaviour I'm seeing:

const client = new bitbucket.Bitbucket();

const res = await client.repositories.list({ workspace: 'robcresswell' });

const repos = res.data?.values; // values is optional

console.log(repos?.[0]?.full_name); // full_name is optional

I can't really understand why this is. From reading the docs at https://developer.atlassian.com/bitbucket/api/2/reference/resource/repositories/%7Bworkspace%7D it seems like values would always be defined (with a size of 0..∞ according to the docs).

In fact, according to the node-bitbucket types everything except type is optional, which is not what the API docs seems to specify? It makes working with the API in TS really difficult, because you have to abuse non-null assertions everywhere to make it behave correctly.

MunifTanjim commented 4 years ago

If you look at the Swagger Definition (https://api.bitbucket.org/swagger.json) for Bitbucket API, you'll see that this is the schema for the paginated repository.

https://github.com/MunifTanjim/node-bitbucket/blob/2c990c491ebfd287c1c7fd3b6c8295c25f95cf96/specification/definitions.json#L3056-L3095

The required key is missing here. And according to JSON Schema specification (https://json-schema.org/draft/2019-09/json-schema-validation.html#rfc.section.6.5.3) that means that all the keys of this object is optional.

And I guess they did the same thing for the whole Swagger Definition.

As the TypeScript types are generated directly from Bitbucket API's Swagger Definition, that is the reason behind this issue.

So, if Bitbucket corrects their API's Swagger Definition, this library will directly benefit from it.


Now, when generating TypeScript types, if we blindly mark everything as required then we might mark something "required" that is actually "optional" and the generated types will be wrong. And if we were to trust that wrong types, the program will face runtime error.


Now that you have the context, do you have any suggestions?

robcresswell commented 4 years ago

Ah, no, I don't. You've made the same decision I would have, to follow the upstream swagger defs. I'd be very surprised if those keys are actually optional; its more likely that Atlassian have misdocumented their own API.

Thanks for the quick reponse! Especially given that it took me months to reply. I'll go dig with Atlassian.