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.

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

token changed callback #42

Merged
merged 10 commits into from
Aug 18, 2023
Merged

Conversation

pavanjoshi914
Copy link
Contributor

@pavanjoshi914 pavanjoshi914 commented Aug 3, 2023

Fixes #31

src/OAuth2User.ts Outdated Show resolved Hide resolved
src/OAuth2User.ts Outdated Show resolved Hide resolved
@rolznz
Copy link
Contributor

rolznz commented Aug 4, 2023

@pavanjoshi914 can you please update the README with a section on how to subscribe to token changes?

event function will return empty promises onwards
signed-off-by: pavan joshi <pavanj914@gmail.com>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
shift emitting event in refreshAccessToken itself
handle types
emit events even when user manually refreshes token
signed-off-by: pavan joshi <pavanj914@gmail.com>
@@ -84,8 +98,11 @@ export class OAuth2User implements OAuthClient {
const token = processTokenResponse(data);
this.token = token;
resolve({token});
this._tokenEvents.emit("tokens", this.token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also add a verb here? (like in tokenRefresh, accountChange, ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

src/OAuth2User.ts Outdated Show resolved Hide resolved
src/OAuth2User.ts Outdated Show resolved Hide resolved
src/OAuth2User.ts Outdated Show resolved Hide resolved
@rolznz
Copy link
Contributor

rolznz commented Aug 8, 2023

@pavanjoshi914 can you test these changes with the Alby extension using yarn link? using these callbacks should be much cleaner than the current solution.

README.md Outdated Show resolved Hide resolved
src/OAuth2User.ts Outdated Show resolved Hide resolved
pavanjoshi914 and others added 2 commits August 17, 2023 17:35
change event name to tokenRefreshed
signed-off-by: pavan joshi <pavanj914@gmail.com>
@pavanjoshi914
Copy link
Contributor Author

@rolznz @bumi i tested it i can receive token from the method. are we good to merge this?

@bumi
Copy link
Contributor

bumi commented Aug 17, 2023

good for me. @rolznz feel free to merge it.

@rolznz rolznz merged commit f4b34f3 into getAlby:master Aug 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add token changed callback
3 participants