[Done] API response types + Typescript

[jameslinimk]

Hey, I've been using this package for a bit and saw there were no types, so I decided to make them. I was able to generate a Typescript client from the Swagger Docs, but some responses are incomplete, so I'm just going through them one by one.

Methods done

• All done

[jameslinimk]

1. Sorry about the inconsistent spacing. Ill add eslint
2. I'll take a look at the python typings and use something like py-ts-interfaces to help me
3. I have to import from JS when working with relative imports see here
4. The reason I ignore JS files, is to compile them to .js and .d.ts files, and those are going to be the files uploaded to npm, but I was going to do that once I finished
5. The "85ad13f7-3d1b-5128-9eb2-7cd8ee0b5741": number is if you look at the API result you can see that 85ad13f7-3d1b-5128-9eb2-7cd8ee0b5741 is in every Cost object. I believe this is because this is the ID of Valorant Points.
6. I don't think there is a better way to improve the Rank typing, other than manually updating it every month. I could create a github workflow that automatically checks for a new act/episode and edits the type.
7. For the map images and rank images, I'll change that to calls to valorant-api

[jameslinimk]

Also, do you have any answers for the 2 issues in the first message?

[raimannma]

1. Sorry about the inconsistent spacing. Ill add eslint
2. I'll take a look at the python typings and use something like py-ts-interfaces to help me
3. I have to import from JS when working with relative imports see here
4. The reason I ignore JS files, is to compile them to .js and .d.ts files, and those are going to be the files uploaded to npm, but I was going to do that once I finished
5. The "85ad13f7-3d1b-5128-9eb2-7cd8ee0b5741": number is if you look at the API result you can see that 85ad13f7-3d1b-5128-9eb2-7cd8ee0b5741 is in every Cost object. I believe this is because this is the ID of Valorant Points.
6. I don't think there is a better way to improve the Rank typing, other than manually updating it every month. I could create a github workflow that automatically checks for a new act/episode and edits the type.
7. For the map images and rank images, I'll change that to calls to valorant-api

1. That would be good
2. You would probably use absolute imports, I don't think it's a good convention to use that .js appendix
3. Simply build them into a build folder and ignore that. But yeah after everything is rewritten in typescript this ignore works too.
4. I am not sure but maybe this value can change, I think @Henrik-3 knows better.
5. You could type it like e[number]a[number] ? Or isn't that explicit enough ?

[jameslinimk]

2. Absolute imports wouldn't change anything, though because only the .d.ts files have imports, Ill remove the .js extension. Also, importing the js files is very common and most projects that don't compile to js and .d.ts do it.
3. From my tests, no
4. I guess I could, tho, I would use e${number}a${"1" | "2"| "3"}

I plan to finish all the methods by today

[jameslinimk]

@raimannma I just finished implementing all the methods and making changes, may you review them?

Also, I have 2 questions,

1. What does the endpoint v1/store-offers return? I assumed by looking at the response that it returned all in-app-purchases, but I might be wrong. The swagger docs say nothing about it.
2. v2/store-featured seems to be bugged (I get an error "TypeError: dataset.levels.find is not a function"), but v1/store-featured works fine. Is this intentional?

[raimannma]

The dist folder should only contain automatically built files and should not be tracked via git in my understanding.

[jameslinimk]

The dist folder should only contain automatically built files and should not be tracked via git in my understanding.

Oh yeah, I forgot about that. I'm currently working on better comments using tsdoc instead of jsdoc. Perhaps documentation can be made typedoc if you would like. After that, I believe it should be ready

[jameslinimk]

@raimannma Done. I changed the README a bit, if you wouldn't mind taking a look. I'm going to go to bed now.

[jameslinimk]

(Sorry about the amount of Testing action commits)

Anyway, @raimannma I did most of the changes (i believe all except storing all of the endpoints in a config), and included real tests which test the structure of the data from the API to the local interface. I also got approval from @Henrik-3 to change the README and he said it looked good. I ran all the tests and all of the methods work well.

I got the github action to automatically create the docs on a docs branch which auto updates each time a commit is made to the main branch.

(the docs hyper link in my fork of the repo doesn't work. It will work once the docgen is run on the main repo)

[jameslinimk]

Done. I removed the JS files and added scripts to compile them

[raimannma]

Also you should squash your commits together in a few commits with better descriptions.

[jameslinimk]

Done

[Henrik-3]

Holy Shit what happened here.... I am back at home now but it's already like 2am here, so i am gonna review the stuff tommorow or on Sunday, but all in all it looks pretty promising. One thing i will change is the versioning since it should be in line with current API Version which 3.0.0 isn't, but i am gonna request the changes tommorow if needed.

I have to be honest i never used TS for big projects like because of the lack of time to learn the TS stuff so for the next API update i am gonna be fcked :D

@jameslinimk if u wish i will give u the Package Maintainer Role on the server if you want to maintain the package in the future (for sure i will create pull requests for further packages if you don't want to maintain it but then i can't guarantee the correctness in regard of TS stuff until i learned it which will need some time)

[raimannma]

Holy Shit what happened here.... I am back at home now but it's already like 2am here, so i am gonna review the stuff tommorow or on Sunday, but all in all it looks pretty promising. One thing i will change is the versioning since it should be in line with current API Version which 3.0.0 isn't, but i am gonna request the changes tommorow if needed.

I have to be honest i never used TS for big projects like because of the lack of time to learn the TS stuff so for the next API update i am gonna be fcked :D

@jameslinimk if u wish i will give u the Package Maintainer Role on the server if you want to maintain the package in the future (for sure i will create pull requests for further packages if you don't want to maintain it but then i can't guarantee the correctness in regard of TS stuff until i learned it which will need some time)

I can also help with the TS stuff.

[Henrik-3]

Feel free to do so :D

[raimannma]

Just also for completeness here is the link to the auto generated docs from @jameslinimk's fork: https://jameslinimk.github.io/unofficial-valorant-api/

[raimannma]

I also have a comment about endpoint versioning.

In the current code you only are able to fetch the latest endpoint version so for leaderboard v2, for mmr v2 and so. It would be also good to support all the older versions as some are using them. I saw that you did this for the leaderboard, by creating a leaderboard v1 method.

But that should be possible for all endpoints.

Also I would like to add the version to the method name, cause that decreases compatibility issues.

If @Henrik-3 adds a new endpoint version to the api and changes the endpoint also in this package, then the users automatically fetch the new endpoint. But if the methods have their version in name they fetch the same endpoint even after updating their dependencies.

I hope this is understandable.

See also my method naming in the Python Wrapper:

• https://raimannma.github.io/ValorantAPI/_source_files/valo_api.endpoints.html#module-valo_api.endpoints.leaderboard
• https://raimannma.github.io/ValorantAPI/_source_files/valo_api.endpoints.html#module-valo_api.endpoints.mmr_details

[Henrik-3]

As a reminder: The current version should be 100% compatible with the last update, so we kinda need a version field in the parameter.

We can change this in the future with a new major release or add them as an optional thing

[raimannma]

Also what needs to be done is automatic deployment to npm or at least a script to do so, but maybe doing that in a seperate PR.

[raimannma]

As a reminder: The current version should be 100% compatible with the last update, so we kinda need a version field in the parameter.

We can change this in the future with a new major release or add them as an optional thing

Yeah, the response should be exactly the same to do it in a minor release.

[Henrik-3]

As a reminder: The current version should be 100% compatible with the last update, so we kinda need a version field in the parameter. We can change this in the future with a new major release or add them as an optional thing

Yeah, the response should be exactly the same to do it in a minor release.

I said that because of your idea with the method names, we can add them as an optional thing but should keep normal one without the version in it's name for now too

[raimannma]

As a reminder: The current version should be 100% compatible with the last update, so we kinda need a version field in the parameter. We can change this in the future with a new major release or add them as an optional thing

Yeah, the response should be exactly the same to do it in a minor release.

I said that because of your idea with the method names, we can add them as an optional thing but should keep normal one without the version in it's name for now too

Yeah, I also support both in the python wrapper. As a method name or as parameter.

[jameslinimk]

Aight so I'll

• add an optional version parameter to all functions with multiple versions
• create new types and have it change the return type based on the version param
• create new tests for all the v1 functions
• Double check names as i belived i renamed 1 function because the orignal name was confusing

For leaderboard v2, I'll add new errors so it'll throw if any of the optional parameters are set and version is v2 as only v1 has functionality for those

Maybe keep the new functions and add deprecated functions to keep backwards compatibility?

I'm just making a checklist for myself here cause I'll work on this tommorow, I just came back from a flight

[jameslinimk]

Also @Henrik-3 I know it's pretty small of an issue, but some return types use different naming conventions. Like some will be camal cause while others snake case

[raimannma]

Also @Henrik-3 I know it's pretty small of an issue, but some return types use different naming conventions. Like some will be camal cause while others snake case

You mean raw & store endpoints are different to the other?

[jameslinimk]

Also @Henrik-3 I know it's pretty small of an issue, but some return types use different naming conventions. Like some will be camal cause while others snake case

You mean raw & store endpoints are different to the other?

Bro I stg I'm so tired. I thought when i was typing the endpoints I saw some camal case in non raw api stuff

Sorry

[Henrik-3]

Getting back to this: I try (!!!) to update this to v3 that is coming soon. Never worked with TS, hopefully the shit i do doesn't look that bad (correct me if there is stuff that doesn't make sense) ^^