-
Notifications
You must be signed in to change notification settings - Fork 364
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
[SDK-2173] Expand on behaviour of checkSession in docs #666
Conversation
README.md
Outdated
@@ -82,7 +82,7 @@ const auth0 = new Auth0Client({ | |||
|
|||
//if you do this, you'll need to check the session yourself | |||
try { | |||
await getTokenSilently(); | |||
await checkSession(); | |||
} catch (error) { | |||
if (error.error !== 'login_required') { |
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.
Isnt this a bit weird now? checkSession
will never throw login_required
(https://github.com/auth0/auth0-spa-js/blob/master/src/Auth0Client.ts#L575) as it is part of the RECOVERABLE_ERRORS
https://github.com/auth0/auth0-spa-js/blob/master/src/constants.ts. So it will never throw login_required.
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.
Very true, let me revert
src/Auth0Client.ts
Outdated
* It should be used for silently logging in the user when you instantiate the | ||
* `Auth0Client` constructor. You should not need this if you are using the | ||
* `createAuth0Client` factory. | ||
* `createAuth0Client` factory. If you are using the `Auth0Client` constructor | ||
* directly, you can use `checkSession` immediately afterwards to check login state. |
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 paragraph speaks about:
- using constructor
- not using constructor
- using constructor
I think it reads a bit weird. Could we keep the parts about using the constructor together?
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.
Good point. Actually I think we can just drop this second part.
Modified the doc comments on
checkSession
to better explain its behaviour.Also updated the usage example in the readme from
getTokenSilently
tocheckSession
.