-
Notifications
You must be signed in to change notification settings - Fork 82
add useAutoSignin hook #1476
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
add useAutoSignin hook #1476
Conversation
Thanks for providing this. Question why do you need |
Hello @pamapa , excuse me for the late answer - unfortanetly I was ill and thus not able to answer asap. We mostly use signinRedirect and ocassionally signinPopup. So as you said, we too only need these. Based on you comment I assume, it would be sufficient to pass either signinRedirect or signinPopup as options to the hook. Update: I adjusted the pull request to only allow signinRedirect and signinPopup as options. |
💡: Fix tsdoc based on api extractor warnings
src/useAutoSignin.ts
Outdated
type UseAutoSignInReturn = { | ||
isLoading: boolean; | ||
isAuthenticated: boolean; | ||
isError: boolean; |
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.
Lets use the same name/type for error as already used in useAuth
: error?: Error;
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.
Sure, I changed it to return the error as in useAuth instead and thus adapted the type to Pick return types from the already defined AuthState interface.
Please check if you are happy with that🙂
looks good, there is one last small thing remaining (see above) |
Closes/fixes #1475
Checklist