26217 2014-06-27 02:01:23 +0000 "If header does not contain two items" 2014-06-27 21:47:55 +0000 1 1 1 Unclassified WHATWG Fetch unspecified PC Linux RESOLVED FIXED P2 normal Unsorted 1 falken annevk bkelly bzbarsky d jaffathecake mike sideshowbarker+fetchspec oldest_to_newest 108409 0 falken 2014-06-27 02:01:23 +0000 http://fetch.spec.whatwg.org/#headers-class [[ If header does not contain two items ]] Does this mean "exactly two items" or "at least two items"? 108414 1 annevk 2014-06-27 06:55:08 +0000 I meant exactly two when I wrote it, but now I'm thinking that being somewhat forgiving might be okay too. Opinions anyone? 108425 2 bkelly 2014-06-27 15:39:57 +0000 I read it as exactly two. Personally I think this is ok as filling the header from the constructor is a convenience and it doesn't seem critical if it cannot handle all cases. I also think allowing more than two could confuse some people as they might start wondering, is it this: new Headers([ ["name", "value1", "value2"] ]); Or this: new Headers([ ["name, ["value1", "value2"]] ]); Which I don't think is possible with sequence<sequence<ByteString>> in webidl. So my vote is to keep it as exactly two. 108429 3 d 2014-06-27 17:22:39 +0000 This isn't working like a normal Map does, which IMO is bad. Normal Map: http://people.mozilla.org/~jorendorff/es6-draft.html#sec-map-iterable Summarized: for (var x of iterable) { var [k, v] = x; this.set(k, v); } Note that it only throws if x is not an object. If x is [], it will add (undefined, undefined). If x is [1, 2, 3], it will add (1, 2). If x is { 0: "foo", 1: "bar" }, it will add ("foo", "bar"). Note also that it explicitly calls `this.set`, which is important and should be preserved. 108431 4 annevk 2014-06-27 19:29:49 +0000 Domenic, I recommend filing a bug on IDL. If we are going to ship a first iteration by Q3 that is not going to be fixed that way. I fixed comment 0 per comment 2 meanwhile: https://github.com/whatwg/fetch/commit/fb8df66cdd69e5a5afcdd7e213875ec8cd299b10 108432 5 bzbarsky 2014-06-27 19:42:40 +0000 > var [k, v] = x; > Note that it only throws if x is not an object. Are you sure? http://people.mozilla.org/~jorendorff/es6-draft.html#sec-runtime-semantics-destructuringassignmentevaluation says the RHS gets iterated, albeit only as many times as there are things in the brackets on the LHS, which is subtly different from what sequence<> will do. It may be good to have an explicit way in IDL of expressing "iterator, get N values from it" for use here. Note that UAs don't necessarily implement the spec here yet, though they're working on it. e.g. see https://bugzilla.mozilla.org/show_bug.cgi?id=933276 > Note also that it explicitly calls `this.set`, which is important and should be > preserved. That's important if you want to allow subclassing and specifically want to allow subclasses to hook this constructor algorithm via overriding set(), correct? This is not the only obviously viable API design choice, fwiw. 108433 6 bzbarsky 2014-06-27 19:44:15 +0000 Oh, and I think requiring exactly two items in this case for now is not totally unreasonable so people don't start to depend on other behaviors while we sort this out. 108434 7 d 2014-06-27 19:53:51 +0000 > Are you sure? http://people.mozilla.org/~jorendorff/es6-draft.html#sec-runtime-semantics-destructuringassignmentevaluation says the RHS gets iterated, albeit only as many times as there are things in the brackets on the LHS, which is subtly different from what sequence<> will do. You are right; destructuring was being too clever. It's really just var k = x[0]; var v = x[1]; > It may be good to have an explicit way in IDL of expressing "iterator, get N values from it" for use here. My plan is to add the following to WebIDL: AddMapElementsFromIterable(iterableInput, dest, adder) so that this algorithm would do AddMapElementsFromIterable(init, this, "append"). I am forking WebIDL and working on a pull request right now, although I am temporarily stymied by the XSLT and the un-Windows-friendly Makefile @_@. > That's important if you want to allow subclassing and specifically want to allow subclasses to hook this constructor algorithm via overriding set(), correct? Yes, or instrumentation, or other aspect oriented programming. > This is not the only obviously viable API design choice, fwiw. I agree, but I see no reason to deviate from ES semantics here. 108435 8 bzbarsky 2014-06-27 20:03:53 +0000 Having some generic thing to initialize maps makes sense Note that this: for (var x of iterable) { var [k, v] = x; this.set(k, v); } Also doesn't match what Map does in ES6. What it does is actually pretty nasty to express in JS. It goes more like this, afaict: var iterGetter = iterable[Symbol.iterator]; if (typeof(iterGetter) != "function") throw new TypeError; var iter = iterGetter.call(iterable); if (typeof(iter) != "object" || !iter) throw new TypeError; var adder = this.set; if (typeof(adder) != "function") throw new TypeError; // it then checks that "iter" is not undefined, which seems superfluous while (true) { var val = GetNextValue(iter); // Not going to inline this, complete with the // break on no more values. var k = val[0]; var v = val[1]; adder.call(this, k, v); } or so. Idiomatic JS it's not. ;) 108436 9 bzbarsky 2014-06-27 20:04:47 +0000 > // it then checks that "iter" is not undefined, which seems superfluous Oh, nevermind, that can happen if the argument is undefined or null. 108437 10 d 2014-06-27 20:07:06 +0000 What you're missing is that for-of basically does all those steps for you :). I agree I omitted the typeof check on adder, but it's not really any different. 108438 11 bzbarsky 2014-06-27 20:21:14 +0000 My point is that the ES6 spec for Map and your for-of loop have observably different behavior in all sorts of situations. Starting with the one where Object.getOwnPropertyDescriptor(this, "set") is an accessor descriptor with a non-idempotent getter. 108439 12 bzbarsky 2014-06-27 20:24:09 +0000 And in particular, the ordering in the spec, where the maybe-iterable's iterator-creating function is called before the get of this.set but this.set is gotten (once) before starting to actually invoke .next() on the iterator means that there is no way to express that algorithm as a for-of loop as far as I can tell. 108444 13 d 2014-06-27 21:15:07 +0000 I believe this is black-box indistinguishable, assuming the code loads early enough to get access to unpolluted Function prototype. var call = uncurryThis(Function.prototype.call); var adder = dest[adder]; if (typeof adder !== "function") { throw new TypeError; } for (var x of iterable) { var k = iterable[0]; var v = iterable[1]; call(adder, dest, k, v); } The ordering of the errors would be different from the spec, but that is not black-box indistinguishable since the error text is unspecified. Anyway, pull-requested: https://github.com/heycam/webidl/pull/13 108448 14 bzbarsky 2014-06-27 21:36:01 +0000 > I believe this is black-box indistinguishable In cases when both the getter for the "adder" property on dest and the value of iterable[Symbol.iterator] are functions with side effects this code will result in the side effects occurring in the opposite order from the spec's current algorithm. Maybe the spec should be adjusted, of course. 108449 15 d 2014-06-27 21:47:55 +0000 Ah good call, thanks, you are of course right. Filed on ES6, since I imagine self-hosting implementations will want this fixed anyway: https://bugs.ecmascript.org/show_bug.cgi?id=3000