Top-gg-Community
feat: adapt to v1
Full diff between this and the commit prior to #91: https://github.com/Top-gg-Community/node-sdk/compare/24a2ee1e093e3072f8dc1d2d2292bdc94636ec84...null8626:node-sdk:master
Tested with npm run test. All that's left now is updating the README.
Not sure I agree with the concept of a separate class to denote a changing of versions. Either we should fit methods into the old class definition or we bump a major version. Is there a discussion on this that I missed?
Veld explicitly proposed that newer endpoints use a separate v1 class that inherit the older one. I believe he wanted this so that developers have a clear distinction on which is a v1 endpoint and which is a legacy endpoint. For example legacy's hasVoted can be confused with v1's getVote because they serve similar a purpose. I originally planned for the hasVoted method to use the v1 equivalent for it, but it would be a breaking change, and Veld is opposed to breaking changes that would require a major update in this situation (probably because if it's a major update, developers might miss it because they set the dependency versioning of @top-gg/sdk in their package.json to ^3.1.0 or whatever.)
Not only that, this is also supported by the fact that the v1 endpoints require a newer API token, while the legacy endpoints work with both legacy and newer API tokens.
@velddev mind weighing in on this? What's the aversion to a breaking major version bump? "Missing an update" doesn't really matter if things aren't changed to not break.
E.g. most of the old methods still use the v0 endpoints, so if v0 gets shut off it won't matter if people updated to the new package version or not. Even people who go out of their way to implement the ApiV1 will also start failing when v0 stops.
I don't see any advantage for shying away from a breaking version here, and seems to break the line between service and library to change how the client is interacted with for an API change.
The goal of these changes is to build the foundation for the v1 API. The legacy API is finished. Any change we introduce shouldn't cause users who use the v0 API to have to change their code and make the experience adding new functionality more cumbersome.
A breaking change would dictate that we edit the current schema, and as long as the current API does not do that, we will not need to do this to the end user.
What we instead should do is have a new client which is built for the v0 API so that the end user can gradually adopt new features without the weight of having to migrate their current code to a new arbitrary refactored client version.
An example: how many times have you had to change your code because a library did a breaking change? Was it a good experience having to do that?
Now imagine that the breaking change does not give you any benefits.
Refactoring for the sake of it, while great for our code maintainability, is selfish towards the end user since every hour that is spent on refactoring, every user of the library will spend an equal amount of time figuring out how to fix their code
I think the disconnect comes from the fact that we're transitioning to a V1 API without it being able to expose the same functionality as V0.
I imagine this will add more refactoring for users than not. But if the V1 isn't going to able to do what V0 does then it is what it is.
I think the disconnect comes from the fact that we're transitioning to a V1 API without it being able to expose the same functionality as V0.
I imagine this will add more refactoring for users than not. But if the V1 isn't going to able to do what V0 does then it is what it is.
For now not everything will be supported by the v1 API. I can add the stats update endpoint relatively soon, but v1 tokens are supported on the v0 api, so any api call to the v0 api will still work with the v1 api token.
The largest reason to have a new class for this is because everything changed. New authentication patterns, new error handling system, new data models. It will be a mess to implement both in one class.
Full diff between this and the commit prior to #91: 24a2ee1...null8626:node-sdk:master
Tested with
npm run test. All that's left now is updating the README.
README updated.