achievements-app / psn-api

A JavaScript library that lets you get trophy, user, and game data from the PlayStation Network.
https://psn-api.achievements.app
MIT License
273 stars 32 forks source link

Please open a v3.0.0-dev branch for breaking changes #118

Closed TheYuriG closed 1 year ago

TheYuriG commented 1 year ago

A few of the issues opened here (#30, #29) require a change in the name of some functions, which would break the current version.

I guess this would mean that we also need to separate the documentation website in v2 and v3, with a section for breaking changes for new, incoming users.

Lastly, should we put a deprecation warning on the current functions so users looking into upgrading to v3 are ready for the change?

PS: Amazing library. I intend to contribute a lot here starting mid-March. Currently building a massive trophy database backend using this. Flawless work.

wescopeland commented 1 year ago

Hi! Thank you for your feedback. Contributions are of course welcome!

Now that the library has gained some traction, versioned docs would make sense.

I am happy to create a v3.0.0 canary branch for breaking changes, however I'm inclined to believe that as soon as those changes are in we should just ship 3.0.0. This would render the creation of such a branch as a moot point.

It might be best to add exports with the new names and mark the existing ones as deprecated in a patch release. If you are looking for a first PR, this would be a great candidate.

wescopeland commented 1 year ago

Other thoughts on a potential v3:

I have recently built https://github.com/RetroAchievements/retroachievements-api-js, where I had great success with both of these. This library originally began as a fork of psn-api.

TheYuriG commented 1 year ago

I am happy to create a v3.0.0 canary branch for breaking changes, however, I'm inclined to believe that as soon as those changes are in we should just ship 3.0.0. This would render the creation of such a branch as a moot point.

Not my library so none of my business how you run it, but I think we still have a bunch of good features we could add that could be ported to 2.x.x, like universal game search from #88. I don't think we have any big changes yet that justify going to 3.0.0 right away? Maybe you could provide some clarification here about the 2 things you mentioned, as I trust you know more about them.

I've created my own singleton component that handles the entire connection management process behind the scenes and reconnects and refreshes when necessary, which I think could be a great addition to this library as well. We can discuss this better halfway through March, as I'm currently laser-focused on creating my backend. Maybe I can even bring some other upgrades I'm yet to discover by then.

wescopeland commented 1 year ago

I think we still have a bunch of good features we could add that could be ported to 2.x.x

My perception is these function renames would not be earth-shattering changes, especially if a deprecation notice lands in 2.x.x. Of course I am happy to minimize breaking changes as much as possible. The best path forward might be to rename the functions themselves, then re-export them under the old names with a @deprecated tag. Then all references to the old names would be scrubbed from the documentation. New library consumers will use the new names, while existing library consumers will see an annoying strike through the old name in VSCode.

Maybe you could provide some clarification here about the 2 things you mentioned, as I trust you know more about them.

microbundle is generally better maintained than dts-cli and I've seen a ~45% reduction in bundle sizes using it over dts-cli.

Docusaurus is kind of a pain to work with; I've found VitePress to generally be a faster and more productive docs site generator.

TheYuriG commented 1 year ago

The best path forward might be to rename the functions themselves, then re-export them under the old names with a @deprecated tag. Then all references to the old names would be scrubbed from the documentation. New library consumers will use the new names, while existing library consumers will see an annoying strike through the old name in VSCode.

Yeah, this sounds like the perfect solution to me!

microbundle is generally better maintained than dts-cli and I've seen a ~45% reduction in bundle sizes using it over dts-cli.

Docusaurus is kind of a pain to work with; I've found VitePress to generally be a faster and more productive docs site generator.

Alright man, looks exciting. Looking forward to getting to work on this with you soon. :)

PS: I forgot to suggest this, but one of the ideas I had for 3.0.0 is removing entirely the need to pass the authorization as a parameter to functions, also being able to set a set user to fetch data from (that is not the same as the logged-in user, as I know some people are afraid of using this with their actual gaming account, doing something wrong, getting rate limited or even banned from PSN for API abuse).

wescopeland commented 1 year ago

Looking forward to getting to work on this with you soon. :)

Likewise!

PS: I forgot to suggest this, but one of the ideas I had for 3.0.0 is removing entirely the need to pass the authorization as a parameter to functions, also being able to set a set user to fetch data from (that is not the same as the logged-in user, as I know some people are afraid of using this with their actual gaming account, doing something wrong, getting rate limited or even banned from PSN for API abuse).

I would definitely be open to suggestions on this. One reason it currently works this way is to keep the functions pure so dead code elimination can work properly when someone imports the library. By keeping everything pure, if someone only uses two functions from psn-api, those will be the only two functions that make it into their build.

One potential approach could be to have some kind of authorized client class, where authorization is handled in the constructor. Then users could decide if they want better DX for slightly larger bundle sizes.

TheYuriG commented 1 year ago

I would definitely be open to suggestions on this. One reason it currently works this way is to keep the functions pure so dead code elimination can work properly when someone imports the library. By keeping everything pure, if someone only uses two functions from psn-api, those will be the only two functions that make it into their build.

Oh, that's cool, you had tree shaking in mind when you did it this way.

One potential approach could be to have some kind of authorized client class, where authorization is handled in the constructor. Then users could decide if they want better DX for slightly larger bundle sizes.

Yeah that sounds like a good compromise, that way users can opt for smaller bundle sizes or DX comfort.