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

feat: introduce experimental unstable_getServerSession API #4116

Merged

Conversation

ThangHuuVu
Copy link
Member

@ThangHuuVu ThangHuuVu commented Mar 4, 2022

Reasoning 💡

Improve the getServerSession API to match the NextAuth API.

Checklist 🧢

  • Documentation
  • Tests
  • Ready to be merged

Affected issues 🎟

@vercel
Copy link

vercel bot commented Mar 4, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/4Sn474Kx6ArjZ923F4sxXM1Me7j4
✅ Preview: https://next-auth-git-fork-thanghuuvu-refactor-improv-abf15d-nextauthjs.vercel.app

@github-actions github-actions bot added the core Refers to `@auth/core` label Mar 4, 2022
@ThangHuuVu ThangHuuVu requested a review from ubbe-xyz March 4, 2022 08:57
const session = await getServerSession(req, res, authOptions)

if (session) {
// do something with the session
Copy link
Member

Choose a reason for hiding this comment

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

should we rather have a different response when there is or there isn't a session? Eg. return 401 if un authenticated

Comment on lines 27 to 47
if (session) {
// do something with the session
}

return {
props: {
session,
},
}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally!

@balazsorban44 balazsorban44 changed the title refactor: improve getServerSession API feat: introduce getServerSession API Mar 4, 2022
Copy link
Collaborator

@ubbe-xyz ubbe-xyz left a comment

Choose a reason for hiding this comment

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

Looking good for me ✅ ; I leave some small grammar suggestions 👍🏽

docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
docs/docs/configuration/nextjs.md Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview May 13, 2022 09:00 Inactive
@vercel vercel bot temporarily deployed to Preview June 14, 2022 13:27 Inactive
@balazsorban44 balazsorban44 changed the title feat: introduce getServerSession API feat: introduce experimental unstable_getServerSession API Jun 22, 2022
Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

Let's land this!

@vercel vercel bot temporarily deployed to Preview June 22, 2022 16:08 Inactive
Copy link
Contributor

@Dragate Dragate left a comment

Choose a reason for hiding this comment

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

Occurrences of getServerSession need to be replaced with unstable_getServerSession.

docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
docs/docs/configuration/nextjs.md Outdated Show resolved Hide resolved
Co-authored-by: Dragate <spidfair@gmail.com>
@ThangHuuVu
Copy link
Member Author

@balazsorban44 please merge it when you're back! 🙌

@magicspon
Copy link

Is this documented anywhere?
image

@Dragate
Copy link
Contributor

Dragate commented Jun 23, 2022

Is this documented anywhere? image

https://next-auth.js.org/configuration/nextjs#unstable_getserversession

@ndom91
Copy link
Member

ndom91 commented Jun 23, 2022

It will require 24hrs for Algolia to scrape the latest docs page deployment and update the search index.

@ThangHuuVu ThangHuuVu deleted the refactor/improve-getServerSession-API branch June 24, 2022 03:49
@larsqa
Copy link

larsqa commented Jul 13, 2022

The docs mention that this is an experimental feature, but nowhere to be found is why this is unstable. This makes it difficult for me as a user to figure out if I even should consider it before it becomes stable and look for alternatives.

Anyone mind to explain the "unstable" situation here?

@DavidJFelix
Copy link

I'm not super keen about how an API was removed (renamed) without a semver bump.

@ndom91
Copy link
Member

ndom91 commented Aug 3, 2022

@larsqa Theres a small note in the docs:

The unstable_getServerSession only has the prefix unstable_ at the moment, because the API may change in the future. There are no known bugs at the moment and it is safe to use. If you discover any issues, please do report them as a GitHub Issue and we will patch them as soon as possible.

The API was released with the unstable prefix, because we're still not 100% sure about a few things, in particular, we wanted to ensure we could still:

  • Make it take in the same parameters with the middleware method
  • Address that this API could be changed in the future releases when we finalize how to do authOptions properly 🤔

@DavidJFelix unstable_getServerSession was introduced in the 4.5.0...4.6.0 update

@DavidJFelix
Copy link

I think the issue really is a breaking change to getServerSession was introduced in the 4.5.0...4.6.0 update and it was documented as an addition of a new function, not the replacement/removal/renaming of an existing function. Semantics, I know, but that's SemVer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Refers to `@auth/core`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants