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

[edge] support request.cookies as a map #4745

Merged
merged 1 commit into from Jun 21, 2022

Conversation

Schniz
Copy link
Contributor

@Schniz Schniz commented Jun 21, 2022

☕️ Reasoning

in next Next.js versions, NextRequest.cookies will be an instance of NextCookies which is
some kind of a Map, instead of a plain object.

This commit checks whether there's a get function in req.cookies, and acts accordingly,
to make sure we will support newer Next.js versions with Edge Functions/Middleware

I don't see any tests here for that. I can introduce some, but not sure if it makes sense. Please let me know what to do with this!

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

Please scout and link issues that might be solved by this PR.

Fixes: INSERT_ISSUE_LINK_HERE

📌 Resources

in next Next.js versions, NextRequest.cookies will be an instance of NextCookies which is
some kind of a Map, instead of a plain object.

This commit checks whether there's a `get` function in req.cookies, and acts accordingly,
to make sure we will support newer Next.js versions with Edge Functions/Middleware
@vercel
Copy link

vercel bot commented Jun 21, 2022

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

1 Ignored Deployment
Name Status Preview Updated
next-auth ⬜️ Ignored (Inspect) Jun 21, 2022 at 5:57PM (UTC)

@github-actions github-actions bot added the core Refers to `@auth/core` label Jun 21, 2022
@balazsorban44 balazsorban44 merged commit 73d489b into nextauthjs:main Jun 21, 2022
@Schniz Schniz deleted the support-cookies-as-map branch June 21, 2022 18:28
@@ -132,7 +132,10 @@ export class SessionStore {

for (const name in req.cookies) {

Choose a reason for hiding this comment

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

@Schniz & @balazsorban44
I think this needs to be a for-of loop for Map to work.
I'm running into this issue where the cookie isn't being read in the middleware
The for-in loop just iterates the properties of the Map object and not the actual constants

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are totally right

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

3 participants