-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat(react): make session requireable in useSession
#2236
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/nextauthjs/next-auth/EuDgeRfzdP4vdMCRFMRuL9VomnnV |
Codecov Report
@@ Coverage Diff @@
## next #2236 +/- ##
=========================================
+ Coverage 9.94% 10.74% +0.79%
=========================================
Files 83 83
Lines 1397 1414 +17
Branches 383 393 +10
=========================================
+ Hits 139 152 +13
- Misses 1039 1042 +3
- Partials 219 220 +1
Continue to review full report at Codecov.
|
Co-authored-by: Kristóf Poduszló <kripod@protonmail.com>
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.
Wow, very neat 🎸 💯 ✨
I'm leaving quite a lot of suggestions 💬 , in an effort to further polish the API, but I consider it's already very nice 💚
Let's have a call to go through it together 👍🏽
Co-authored-by: Lluis Agusti <hi@llu.lu>
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.
LGTM 👍
@lluia had some requests which first need to be reviewed again, when this PR is updated. 😊 |
Remove `loading` value from `useSession` in V4 given we already return a `status`value which scales better to identify the different states of the session request.
• rename `action` to `onFail` • reword documentation • document `useSession` return • reword `<SessionProvider />` usage to state is mandatory
Rename the handler name on `useSession` when authentication didn't pass to be more accurate.
A living session could be a requirement for specific pages (like dashboards). If it doesn’t exist, the user should be redirected to a page asking them to sign in again. Sometimes, a user might log out by accident, or by deleting cookies on purpose. If that happens (e.g. on a separate tab), then `useSession({ required: true })` should detect the absence of a session cookie and always return a non-nullable Session object type. When `required: true` is set, the default behavior will be to redirect the user to the sign-in page. This can be overridden by an `action()` callback: ```js const session = useSession({ required: true, action() { // .... } }) if (session.status === "Loading") return "Loading or not authenticated..." // session.data is always defined here. ``` Co-authored-by: Kristóf Poduszló <kripod@protonmail.com> Co-authored-by: Lluis Agusti <hi@llu.lu> BREAKING CHANGE: The `useSession` hook now returns an object. Here is how to accommodate for this change: ```diff - const [ session, loading ] = useSession() + const { data: session, status } = useSession() + const loading = status === "loading" ``` With the new `status` option, you can test states much more clearly.
Reasoning 💡
A living session could be a requirement for specific pages (like dashboards). If it doesn’t exist, then the user should be redirected to a page that asks them to sign in again.
Sometimes, a user might log out by accident, or by deleting cookies on purpose. If that happens (e.g. on a separate tab), then
useSession({ required: true })
should detect the absence of a session cookie and always return a non-nullable Session object type.When
required: true
is set, the default behavior will be to redirect the user to the sign-in page. This can be overridden by anaction()
callback:Checklist 🧢
Affected issues 🎟
Fixes #1210
Related: #2269
BREAKING CHANGE:
The
useSession
hook now returns an object. Here is how to accommodate for this change: