types: HeadersInit does not accept arrays as values, but it will work

[KianNH]

HeadersInit is defined as type HeadersInit = Headers | Iterable<Iterable<string>> | Record<string, string> - the one that's of note is Record<string, string>.

The runtime will accept an array as the value for a given key, and fold them into a single header by joining them with a comma, but the types don't allow this.

I assume it's defined here, and should instead be Record<string, string | string[]>.

workerd/src/workerd/api/http.h

Line 134 in c0ea735

JSG_TS_DEFINE(type HeadersInit = Headers | Iterable<Iterable<string>> | Record<string, string>);

[KianNH]

Iterable<Iterable<string>> would also need updating to Iterable<Iterable<string | string[]>> to support this - but there seems to be another issue with that type.

It'll accept [ 'lol', 'bar', 'lol2', 'bar2' ] which is then rejected at runtime: TypeError: Incorrect type: the provided value is not of type 'Sequence'.

[KianNH]

I guess another viewpoint is, should it accept an array of values?

https://fetch.spec.whatwg.org/#headers-class appears to say no.

[kentonv]

The runtime is not intentionally designed to allow an array here. The reason it works is because of the general JavaScript behavior of being able to coerce anything to a string. ["foo","bar"].toString() is foo,bar. This just happens to be the correct way to combine multiple header values under HTTP, but this is entirely an unintended coincidence.

[KianNH]

Thanks for the insight Kenton, that makes sense.

The issue is actually off the back of what Undici (through Miniflare) does - where they allow arrays as values, especially for setting multiple Set-Cookie headers. It got me curious as to what the behaviour should be as per the spec.

Someone reported this discrepancy on the Wrangler repo: cloudflare/workers-sdk#2743

The change in Undici is nodejs/undici#1598, but I can't really find anything to back that up as something that is supposed to be possible under the Fetch spec - it's also possible that change isn't strictly what causes the behaviour since they also have some set-cookie specific handling in general.

[mrbbot]

Hey! 👋 I think we should keep these types the same for now, considering this is what TypeScript's lib.webworker.d.ts looks like (https://github.com/microsoft/TypeScript/blob/43cc362cef72e5fa1372f59736a9c4b55d85def0/lib/lib.webworker.d.ts#L6110) and #321 didn't change the constructor implementation/types. It seems like the correct way of specifying multiple Set-Cookie headers is via an array of key-value pairs? The existing types should support this. 👍

[KianNH]

It seems like the correct way of specifying multiple Set-Cookie headers is via an array of key-value pairs? The existing types should support this. 👍

This makes sense, but the current workers-types uses Iterable<Iterable<string> which does not line up with the lib.webworker.d.ts types.

Whilst both will accept [ ['Set-Cookie', 'sessionId=38afes7a8'], ['Set-Cookie', 'sessionId=38afes7a9'] ] which is correct, Iterable<Iterable<string> will accept ['foo', 'bar'] and you will hit a runtime error like described here: #388 (comment)

Shouldn't the workers-types be changed to match the lib.webworker.d.ts types exactly?