Is a missing HeadersInit treated differently from an empty one?

[bzbarsky]

The relevant IDL is:

dictionary ResponseInit {
  HeadersInit headers;
};
typedef (sequence<sequence<ByteString>> or record<ByteString, ByteString>) HeadersInit;

Is passing {} for ResponseInit equivalent to passing { headers: {} }? I can't quite tell from the steps at https://fetch.spec.whatwg.org/#dom-response. In particular, is step 6.1 needed? If it is, chances are those two forms are not identical at the moment....

/cc @wanderview @annevk

[bzbarsky]

Similar for RequestInit and Request. See https://fetch.spec.whatwg.org/#dom-request step 13, which is a pretty serious problem depending on how whatwg/webidl#76 shakes out. And step 29.

For now, sorting this out blocks me implementing record in Gecko...

/cc @domenic @annevk @wanderview

[bzbarsky]

I guess here's my real concern. If the Response constructor just took three arguments there, a status, statusText, and HeadersInit, then passing nothing would be exactly equivalent to passing {} for the HeadersInit, per IDL spec. But because the three are packaged up in a ResponseInit dictionary, suddenly passing nothing for the HeadersInit can be different from passing {} (arguably a bug in IDL), and fetch in fact gives them different behavior...

This dependency on whether arguments are passed individually or in a dictionary is deeply unfortunate from my point of view.

[annevk]

So we count { headers: undefined } as not present, right? Is passing {} not something more explicit? Or do we treat that as we treat undefined?

[bzbarsky]

So we count { headers: undefined } as not present, right?

Well, that depends on what you mean by "we".

If you do new Headers(undefined) that is the same as doing new Headers({}). And whatwg/webidl#76 is all about how this should behave.

But at the moment per the IDL spec { headers: undefined } and {} are required to have the same behavior as each other. And { headers: null } and { headers: {} } are likewise required to have the same behavior as each other. But the two behaviors are not required to be identical (again, at the moment), and the fetch spec in fact defines them to be quite wildly different as far as I can tell. Or at least making them both behave like { headers; {} } fails a whole bunch of fetch and serviceworker tests quite spectacularly.

[annevk]

If we're just looking at Fetch, it only branches on whether the headers member is present. It leaves defining being present up to IDL. I guess you're saying it's always present since it takes a record? I'm not really sure I'm following.

[bzbarsky]

If we're just looking at Fetch, it only branches on whether the headers member is present.

Right, and I question whether that's the right call.

I guess you're saying it's always present since it takes a record

I'm saying I would like IDL to say it's always present. But if I actually do that, then tons of fetch tests fail, because its "present and empty" behavior is different from its "not present" behavior. In some APIs, but not others, to add to the confusion.

[annevk]

I guess I don't understand why a record would always be present. Surely you should be able to not pass a record?

[bzbarsky]

Well, it's a general API design decision. For dictionaries, IDL explicitly has identical behavior for null, undefined, and {} as operation arguments. It does not have that for dictionaries as dictionary members, but I think it should; that's what the discussion in whatwg/webidl#76 is about.

For records, we made the behavior match dictionaries, because I guess people felt conceptually they were kind of the same (maybe because they were still thinking of the "half-open dictionary" case?). Note that this doesn't match what I had done with MozMap in Gecko, where I made it much more like a sequence than like a dictionary in terms of all this stuff (e.g. you can't pass undefined or null for a MozMap argument, and MozMap isn't forced to be an optional arg in trailing position, etc).

It's possible that speccing record to be like dictionary was just a mistake. After all, {} as a dictionary just means "no values provided for any of the members", so it makes sense to give null and undefined the same behavior. But for a record {} is more like an explicit "the list is empty" which maybe shouldn't mean the same thing as "no list provided".

So @jyasskin, @domenic, what behavior do we want here?

[annevk]

So in the case of Response objects I don't think it would make a difference to change the "present check" to an "empty check". A Response object always starts out without headers. It's not clear to me what step 6.1 of https://fetch.spec.whatwg.org/#dom-response would actually do. I think that's a no-op and potentially some old copypasta (would require some digging).

In the case of Request objects it seems like it might be problematic as we want to be able to know whether you are just copying a request or modifying it. And when you're modifying it, you might not want to remove all the headers or that might be exactly what you want. If we only have an "empty check", those semantics would go lost, I think. Or am I missing something?

[bzbarsky]

So I should note that no matter what we do for records passing [] for a HeadersInit will unambiguously specify "no headers". The real question is whether {} should have the same behavior as [], and what behaviors null and undefined should have.

It sounds like you'd like undefined to mean "just copy the headers". What about null?

[annevk]

Yeah, I think undefined should mean that we copy the headers of the original request. I don't really care for null one way or another. It seems these days we try to special case null less, so maybe whatever would it would end up doing if we didn't give it special treatment.

[annevk]

The Response object constructor step 6.1 is indeed copypasta per 5ba5186 and needs to be removed. I'll wait with that until we have a plan.

If in general we want omission and {} to be equivalent, perhaps we should introduce [AllowOmission] now (well soon) attributes are easier to use.

[domenic]

Having been asked to weigh in, here are my weak preferences:

• I sympathize with {} meaning explicit "the list is empty". Let's keep that. So { headers: {} } means an empty headers list.
• I think undefined for any dictionary member should be equivalent to the member not being present. So { headers: undefined } should be the same as { } (which I believe means "copy the input"?)
• In general I think it probably makes sense for null and undefined to not be convertible to records. So { headers: null } should be an error. { headers: undefined } is not an error per above, since it's a dictionary member, but if we had a function doStuffWithHeaders(record<DOMString, DOMString> headers) then IMO doStuffWithHeaders(undefined) or doStuffWith() should throw.

I don't think we need a new extended attribute for any of this.

[bzbarsky]

OK, so that sounds like we should make record be more like sequence than dictionary in the IDL spec, right? That is, it requires an object for its value, doesn't have special "must be optional in trailing position" stuff, is distinguishable from nullable types, etc. Unlike sequence, any object can be a record, which mostly affects distinguishability` checking.

I would be quite happy with that, since that's what Gecko implements for MozMap already. I just wish I hadn't wasted the time to align Gecko with the current IDL spec here. :(

If we're all agreed with that, do you or @jyasskin have bandwidth to update IDL? If not, I can try to do that.

[annevk]

@bzbarsky I created whatwg/webidl#304.

[TimothyGu]

This should be fixed by whatwg/webidl#305 already.

[annevk]

Indeed, thanks!