-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
refactor: make pkce option a client setting #646
Conversation
…e-js into j0/add_global_pkce_option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One storage naming issue in _isPKCEFlow
otherwise LGTM, thanks.
private async _isPKCEFlow(): Promise<boolean> { | ||
const currentStorageContent = await getItemAsync( | ||
this.storage, | ||
`${this.storageKey}-code-verifier` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`${this.storageKey}-code-verifier` | |
`${this.storageKey}-oauth-code-verifier` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good catch! Have renamed everything else to code-verifier
instead so as to keep it generic
Leaving this with a refactor: prefix so semver doesn't trigger another release as I'd like to get another three PRs in
Upd: looks like refactor triggers a release too |
🎉 This PR is included in version 2.20.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Refactor pkce to be a client setting so it doesn't have to be configured on invocation for each method.
exchangeSessionForCode
in the background ifdetectSession
is enabledflowType
option from signInWithOAuth