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

fix: add getSetCookie to session cookies if supported #8631

Merged

Conversation

edenstrom
Copy link
Contributor

@edenstrom edenstrom commented Sep 18, 2023

☕️ Reasoning

I saw that .getSetCookie were finally supported in the edge runtime when deployed to Vercel. This uses getSetCookie if supported, otherwise falls back to .get('set-cookie)

Deployed a version with this patch here:

Reproduction deployment: next-auth-error-vercel.vercel.app
Reproduction repo: edenstrom/next-auth-error-vercel

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

This solves the long standing issue were chunked cookies didn't work when using the edge runtime deployed to Vercel.

#7443 (comment)

Related issues:

📌 Resources

@vercel
Copy link

vercel bot commented Sep 18, 2023

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

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
auth-docs ⬜️ Ignored (Inspect) Visit Preview Sep 27, 2023 6:47am
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Sep 27, 2023 6:47am

@vercel
Copy link

vercel bot commented Sep 18, 2023

@edenstrom is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@edenstrom edenstrom closed this Sep 19, 2023
@edenstrom
Copy link
Contributor Author

Closed due to not currently working.. Fixing.

@cipriancaba
Copy link

hey @edenstrom are you still working on a fix for this?

@edenstrom edenstrom reopened this Sep 27, 2023
@edenstrom
Copy link
Contributor Author

hey @edenstrom are you still working on a fix for this?

Yes, reopened PR with fix. :)

Works here. https://next-auth-error-vercel.vercel.app/

@cipriancaba
Copy link

You're a lifesaver 🔥

@viktorlarsson
Copy link

Could we get this merged?

@balazsorban44 balazsorban44 merged commit f2c23db into nextauthjs:feat/nextjs-auth Sep 29, 2023
12 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants