Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't generate new Access token when used with fallbackToLastPlayed #10

Closed
rocktimsaikia opened this issue Mar 3, 2022 · 2 comments · Fixed by #13
Closed

Don't generate new Access token when used with fallbackToLastPlayed #10

rocktimsaikia opened this issue Mar 3, 2022 · 2 comments · Fixed by #13
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@rocktimsaikia
Copy link
Owner

spotify-mini/index.ts

Line 72 in 50bc8ec

? { isPlaying, ...(await this.getLastPlayed()) }

Use the same access token in getLastPlayed() generated by currently playing API call when there is no ongoing track. It does not make sense to regenerate a new token instead of using the existing one
@rocktimsaikia rocktimsaikia added bug Something isn't working help wanted Extra attention is needed labels Mar 3, 2022
@melosh101
Copy link
Contributor

wouldnt it be ideal to add the access token to the class as a private property and refetch it when it expires since they expire after an hour? . would spare a couple of requests

@rocktimsaikia
Copy link
Owner Author

Yes. That should be the ideal implementation. PRs are open for this!
It should ideally only generate access token once the api throws an expired accesstoken error which is probably 401 if I am not wrong

melosh101 added a commit to melosh101/spotify-mini that referenced this issue Mar 10, 2022
@melosh101
add accesToken to client + add limit to getLastPlayed
93c124f 
add caching of the accesToken to the client
add limit to the getLastPlayed function to allow the user to pick the amount of tracks to recieve. max being 50 and minimum being 1.

fixes rocktimsaikia#7
fixes rocktimsaikia#10
@melosh101 melosh101 mentioned this issue Mar 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add accesToken to client + add limit to getLastPlayed melosh101/spotify-mini
2 participants
@rocktimsaikia @melosh101