Headers cleanup

[benmccann]

Headers type is defined in three places:

• SvelteKit
• Node.js
• node-fetch

We try to pass these around interchangeably, but that's almost certainly broken since it may be a string | string[] in Node.js, but not in the other two places.

I believe that Node.js has defined headers correctly while the other two are broken. There's actually an open issue for this in the node-fetch repo node-fetch/node-fetch#771. Unfortunately, I'm not quite sure what to do about this as long as that issue remains open

I think it's also possible that fetch headers are different than generic request headers? In which case, maybe it's just SvelteKit that's broken and the other two are both correct

[Rich-Harris]

I suspect we should use https://developer.mozilla.org/en-US/docs/Web/API/Headers everywhere, the same as we use URLSearchParams

[Conduitry]

It looks like that API only provides a way to retrieve a comma-separated string of all the multiple instances of a particular header. That sounds like it could be a problem. But maybe it's the platform's problem and not something we should try to solve? 😬

[Rich-Harris]

Oh, huh. I thought it was going to be the same as URLSearchParams.

Why are the people in charge so bad at this stuff

[Conduitry]

They even used to have a .getAll() that returned an array and decided no that was too helpful.

[benmccann]

It's pretty complicated. I think that you can generally combine multiple header values into a single one and it's fine, but the exception looked to me to be cookies. I'm not an expert, but don't think there's a way you can do that without possibly corrupting them and there seems to be a reproduction showing that node-fetch breaks cookies as a result: node-fetch/node-fetch#771 (comment)

[Rich-Harris]

Is node-fetch deviating from fetch behaviour though? I guess it doesn't really matter if set-cookie can't be represented by response.headers in the browser since (IIUC) set-cookie isn't included in the headers you're able to access

[benmccann]

Ah, yeah, you're right about that: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie. So I guess the node-fetch API probably is correct for that use case

I think our own Headers definition is the only one that's broken then. fetch headers can be string, but I think our Request and Response headers are wrong. E.g. consider the set-cookie example in the documentation - with this API you can only set a single cookie