feat: add cookies.setSerialized to create a cookie from a string

[while1618]

closes #13680

---

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-kit-13681-svelte.vercel.app/

[github-actions]

pnpm add https://pkg.pr.new/@sveltejs/adapter-auto@13681
pnpm add https://pkg.pr.new/@sveltejs/adapter-cloudflare@13681
pnpm add https://pkg.pr.new/@sveltejs/adapter-netlify@13681
pnpm add https://pkg.pr.new/@sveltejs/adapter-static@13681
pnpm add https://pkg.pr.new/@sveltejs/adapter-node@13681
pnpm add https://pkg.pr.new/@sveltejs/adapter-vercel@13681
pnpm add https://pkg.pr.new/@sveltejs/amp@13681
pnpm add https://pkg.pr.new/create-svelte@13681
pnpm add https://pkg.pr.new/@sveltejs/enhanced-img@13681
pnpm add https://pkg.pr.new/@sveltejs/kit@13681
pnpm add https://pkg.pr.new/@sveltejs/package@13681

[changeset-bot]

🦋 Changeset detected

Latest commit: 449c33d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Minor

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

[benmccann]

here is the issue for this PR: #13680

I edited the PR description. If you say "fixes" or "closes" followed by the issue number then GitHub will link the issue and PR

[dummdidumm]

The thrown errors for validation make me a bit wary of whether want to add this - feels a bit safer to parse the cookie from a string, then set it - that way you're forced to handle this yourself and TypeScript can help you.

[benmccann]

@dummdidumm it may be my lack of typescript expertise speaking here, but how would typescript help?

[while1618]

Hey @benmccann, your suggestions make sense to me. Should I just commit them, or should we wait for another maintainer?

[benmccann]

I don't think there's much harm in adding my suggestions, but we'll need agreement for a larger group of maintainers before deciding whether to accept the PR

[while1618]

Okay, I'll commit them.

And what is your approach for PRs to be merged? Are we just waiting for enough maintainers to see this or you guys have some email/tag if opinion of others is required?

[while1618]

Hi @Rich-Harris, this PR has been opened for a while now, can you please take a look, and let me know if you agree with this approach, or you want me to close the PR? Thanks

[Rich-Harris]

To be honest I don't think this is the right API, since SvelteKit insists on having a path which may not be present in the received header. Surely it's easier to work with a structured representation of the cookie so that you can add a path (and modify options as needed)?

So instead of this...

const response = await fetch('...');

// oops, errors, no `Path=/` in the header
cookies.setSerialized(response.headers.get('set-cookie'));

...something like this?

const response = await fetch('...');

const { name, value, ...options } = cookies.parse(response.headers.get('set-cookie'));
cookies.set(name, value, { ...options, path: '/' });

Here, cookies.parse is just an alias for set_cookie_parser.parseString, not necessarily because it's the correct place for it, but it is convenient.

Thoughts?

[while1618]

Thanks for the response @Rich-Harris.

So, if I understand you correctly, your suggestion is to just expose parseString method from set_cookie_parser in cookies as cookies.parse and to let the client deal with it as he wants?

That sounds fine with me, but the one concern I have is this method normalized_same_site I added, because parseString will return sameSite as string | undefined, but cookies.set expect exact type for sameSite, so client will need to handle that.

Also, if strict or httpOnly is not set in cookie string (they are false), they will be undefined after parseString, so cookies.set will just use default values for strict and httpOnly, which is true in sveltekit, and you will not have the same cookie you received.

[teemingc]

...something like this?

const response = await fetch('...');

const { name, value, ...options } = cookies.parse(response.headers.get('set-cookie'));
cookies.set(name, value, { ...options, path: '/' });

Here, cookies.parse is just an alias for set_cookie_parser.parseString, not necessarily because it's the correct place for it, but it is convenient.

Thoughts?

It probably needs to return an array. So it would look more like:

const response = await fetch('...');
 
const new_cookies = cookies.parse(response.headers.get('set-cookie'));
new_cookies.map(({ name, value, options }) => {
	cookies.set(name, value, { ...options, path: '/', secure: options.secure ?? false, httpOnly: options.httpOnly ?? false });
});

That sounds fine with me, but the one concern I have is this method normalized_same_site I added, because parseString will return sameSite as string | undefined, but cookies.set expect exact type for sameSite, so client will need to handle that.

sameSite can be undefined for cookies.set. This isn't an issue. If it's not specified, it defaults to the browser default of 'lax'.

Also, if strict or httpOnly is not set in cookie string (they are false), they will be undefined after parseString, so cookies.set will just use default values for strict and httpOnly, which is true in sveltekit, and you will not have the same cookie you received.

It's probably better that the user handles this themselves as shown in the example above. We'll want to keep the defaults secure.

[teemingc]

We'll need to update this to the next minor version

[vercel]

Tests for setSerialized incorrectly access new_cookies as a plain object instead of using Map's .get() method, causing assertions to always fail

[Rich-Harris]

It probably needs to return an array

Ah right. I think we probably want to discourage headers.get('set-cookie') in favour of headers.getSetCookie() which is supported everywhere now — even though set-cookie-parser handles ambiguous headers (i.e. it can tell whether a comma is part of a cookie field like Expires or is a separator between cookies), it involves some trickery. getSetCookie is the correct way to deal with multiple cookies, and if we don't need to worry about the ambiguity we can ditch the dependency in favour of a simpler implementation.

So usage would be more like this:

const response = await fetch('...');

for (const str of response.headers.getSetCookie()) {
  const { name, value, ...options } = cookies.parse(str);
  cookies.set(name, value, { ...options, path: '/' });
}

[Rich-Harris]

closing in favour of #16203 — thanks