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.

Sign up for GitHub

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

Jump to bottom

chore(docs): update securing pages tutorial #3982

Merged
merged 18 commits into from Jun 24, 2022
Merged

chore(docs): update securing pages tutorial #3982

merged 18 commits into from Jun 24, 2022

Conversation

ndom91
Copy link
Member

@ndom91 ndom91 commented Feb 15, 2022

Reasoning 💡

Draft PR to address #3973 and update some tutorials and other documentation to bring getServerSession and middleware related documentation up to speed.

Checklist 🧢

  • Documentation
  • Tests
  • Ready to be merged

Affected issues 🎟

@vercel
Copy link

vercel bot commented Feb 15, 2022 •
edited

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/GNdiqRGiEthjkn8ZnpLYrkHrCSR1
✅ Preview: https://next-auth-git-ndom91-gh3973getserversession-nextauthjs.vercel.app

@vercel vercel bot temporarily deployed to Preview February 15, 2022 20:10 Inactive
@vercel vercel bot temporarily deployed to Preview February 16, 2022 10:07 Inactive
@vercel vercel bot temporarily deployed to Preview February 16, 2022 10:22 Inactive
@ndom91 @ubbe-xyz
Update docs/docs/tutorials/securing-pages-and-api-routes.md
ebb6138 
Co-authored-by: Lluis Agusti <hi@llu.lu>
@vercel vercel bot temporarily deployed to Preview February 16, 2022 10:57 Inactive
@vercel vercel bot temporarily deployed to Preview February 16, 2022 17:51 Inactive
@ndom91
Copy link
Member Author

ndom91 commented Feb 16, 2022 •
edited

@balazsorban44 okay I think your comments have been addressed here. The last thing thats open afaik is deciding where the options arg comes from, as you mentioned. Right?

EDIT: And just to confirm, getServerSession is going to be imported from next-auth/next, while getSession still comes from next-auth/react?

@vercel vercel bot temporarily deployed to Preview February 16, 2022 18:10 Inactive
@stale stale bot added the stale Did not receive any activity for 60 days label May 25, 2022
@kfranqueiro
Copy link

As someone currently looking at upgrading an app from next-auth v3 to v4 who was very confused by things failing until stumbling across #4075, and subsequently very confused that this is not documented, evidently due to this PR being in limbo for over 3 months... is there a reason it hasn't been merged and is now in danger of being closed as stale?

@stale
Copy link

stale bot commented Jun 10, 2022

To keep things tidy, we are closing this issue for now. If you think your issue is still relevant, leave a comment and we might reopen it. Thanks!

@stale stale bot closed this Jun 10, 2022
@kfranqueiro
Copy link

Not sure why stalebot closed this when I commented on it? Does that not count as activity?

getServerSession is not mentioned at all in the docs currently on the site. This PR looked like it would address that. The fact that this was opened 5 months ago, never merged, and now closed seems like a big problem. How are people actually managing to use v4 successfully if the docs are this behind?

@ndom91
Copy link
Member Author

ndom91 commented Jun 13, 2022

Not sure why stalebot closed this as well. Theres some overlap between this and #4116 PR. No excuse as to why it hasn't been merged yet though, getServerSession should probably be officially mentioned in the docs at this point 🙈

@ndom91 ndom91 reopened this Jun 13, 2022
@stale stale bot removed the stale Did not receive any activity for 60 days label Jun 13, 2022
@ThangHuuVu ThangHuuVu mentioned this pull request Jun 23, 2022
Merged
5 tasks
balazsorban44
balazsorban44 requested changes Jun 23, 2022
View reviewed changes
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.

Now that #4116 is merged, could we get this PR up to date? 🙏

@ndom91
Copy link
Member Author

ndom91 commented Jun 23, 2022

Now that #4116 is merged, could we get this PR up to date? 🙏

Yup, was just wondering if I should still do this one or what. Will update it shortly 👍

@vercel
Copy link

vercel bot commented Jun 23, 2022 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
next-auth ⬜️ Ignored (Inspect) Jun 24, 2022 at 7:38AM (UTC)

@ndom91
Copy link
Member Author

ndom91 commented Jun 23, 2022

@balazsorban44 @ThangHuuVu alright this one is finally updated with unstable_getServerSession as well 🎉

Anything else that you guys would like updated here? Or good to go?

ThangHuuVu
ThangHuuVu requested changes Jun 24, 2022
View reviewed changes
Copy link
Member

@ThangHuuVu ThangHuuVu left a comment

Choose a reason for hiding this comment

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

@ndom91 unstable_getServerSession now accepts 3 arguments so I suggest the changes accordingly; please add the import for authOptions in the example/code as well. This is what I and @balazsorban44 came up with attempting to make this function looks similar to NextAuthNextHandler. 💡

docs/docs/getting-started/client.md Outdated Show resolved Hide resolved
docs/docs/getting-started/example.md Outdated Show resolved Hide resolved
docs/docs/getting-started/example.md Outdated Show resolved Hide resolved
docs/docs/tutorials/securing-pages-and-api-routes.md Outdated Show resolved Hide resolved
docs/docs/tutorials/securing-pages-and-api-routes.md Outdated Show resolved Hide resolved
apps/example-nextjs/pages/server.tsx Outdated Show resolved Hide resolved
apps/dev/pages/server.js Outdated Show resolved Hide resolved
apps/dev/pages/server.js Outdated Show resolved Hide resolved
apps/dev/pages/api/examples/protected.js Outdated Show resolved Hide resolved
apps/dev/pages/api/examples/session.js Outdated Show resolved Hide resolved
@ndom91 @ThangHuuVu
fix: update unstable_getServerSession arguments
ab9396d 
Co-authored-by: Thang Vu <31528554+ThangHuuVu@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview June 24, 2022 07:37 Inactive
@ndom91
Copy link
Member Author

ndom91 commented Jun 24, 2022

@ThangHuuVu great call with the arguments, I didnt realize that at first and was stuck for a minute during my upgrade to 4.6.1 in a sideproject of mine as well 😂

Appreciate all the recommended diffs btw!

Also updated the text a bit in the SSR recommendation page based on your feedback 👍

@ndom91 ndom91 requested a review from ThangHuuVu June 24, 2022 07:40
ThangHuuVu
ThangHuuVu approved these changes Jun 24, 2022
View reviewed changes
Copy link
Member

@ThangHuuVu ThangHuuVu left a comment

Choose a reason for hiding this comment

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

Yeah, great work, PR looks ready to be merged now!

@LOGOLFGODORD

This comment was marked as spam.

Sign in to view
@ndom91 ndom91 merged commit e8827cb into main Jun 24, 2022
@ndom91 ndom91 deleted the ndom91/gh3973/getServerSession branch June 24, 2022 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ubbe-xyz ubbe-xyz ubbe-xyz approved these changes

@ThangHuuVu ThangHuuVu ThangHuuVu approved these changes

@balazsorban44 balazsorban44 Awaiting requested review from balazsorban44

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ndom91 @ThangHuuVu @ubbe-xyz @kfranqueiro @LOGOLFGODORD @balazsorban44