node-strava / node-strava-v3

API wrapper for Strava's v3 API, in Node
MIT License
356 stars 109 forks source link

TS interface does not match API response #142

Open mschilde opened 1 year ago

mschilde commented 1 year ago

I have a question regarding the provided TS interfaces. In v.2.2.0 the DetailedActivityResponse got added.

While it's nice to have a non-any API response, the interface leaves out a lot of the attributes that the Strava API call (activities.get) actually returns.

What's the idea here? Is this a stub, or are all omitted attributes deprecated by Strava and should not be used?

markstos commented 1 year ago

@thibaultleouay You authored that change. Could you answer this question?

thibaultleouay commented 1 year ago

@mschilde which fields are missing ?

I used that to map the fields

https://developers.strava.com/docs/reference/#api-models-DetailedActivity

Ps : I haven't looked at the Strava api doc for a while and last time I checked it was not the best one

E.g the api returned some fields which were supposed to be already deprecated and removed

mschilde commented 1 year ago

@thibaultleouay

Well, lots of fields are missing :-)

I'm talking about this interface: https://github.com/node-strava/node-strava-v3/blob/main/index.d.ts#L243

A call to activities.get returns lots of data that is not part of the interface. Pretty much everything that is listed in the Strava API docs is returned for a cycling activity.

Some of the missing fields are marked as deprecated in the Strava API docs, e.g. type, while others are not, e.g. segment_efforts or device_name.

Hence my basic questions about the intention of the TS interfaces.

When you make a call to activities.get you DO end up with an JSON structure that has vastly more attributes than the type describes. Which means that if you currently are using any of the missing fields in your code you have to fight the types in this SDK or cannot upgrade.

I can help with a PR or examples if you like.

thibaultleouay commented 1 year ago

@mschilde

Go for a pr 🚀

markstos commented 9 months ago

Agreed. PR welcome.