fix(cookies): be able to split set-cookie without getAll
#255
Conversation
🦋 Changeset detectedLatest commit: efb3789 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const setCookie = | ||
| // @ts-expect-error See https://github.com/whatwg/fetch/issues/973 | ||
| responseHeaders.getAll?.('set-cookie') ?? | ||
| responseHeaders.get('set-cookie') ?? |
There was a problem hiding this comment.
praise: Thanks for removing this hard dependency on getAll()!! I banged onto it while working on isomorphic, but I had to stop for changing priorities. Thanks a million!
| * Based on: https://github.com/google/j2objc/commit/16820fdbc8f76ca0c33472810ce0cb03d20efe25 | ||
| * Credits to: https://github.com/tomball for original and https://github.com/chrusart for JavaScript implementation | ||
| */ | ||
| function splitCookiesString(cookiesString: string) { |
There was a problem hiding this comment.
suggestion: we already have this function in packages/node-utils/src/edge-to-node/headers. Can we share it between the two packages? maybe a third package called internals-utils? @Kikobeats, @javivelasco, what do you think?
There was a problem hiding this comment.
yes, we can create a package for that, also for testing it properly.
There was a problem hiding this comment.
Seen with @Kikobeats, let's merge this as is, we'll do the code sharing in subsequent work.
Thanks again!
Currently, if a user sets multiple cookies in a series, only the second cookie is set in the
set-cookieheader.When this is merged, I will update https://github.com/vercel/next.js/tree/canary/packages/next/src/server/web/spec-extension/cookies to drop the code and use the pre-compiled version of this package again.
Slack thread 1, Slack thread 2