This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.
Currently the spec requires you to define the "iterator behavior" and a bunch of other definitions "when you use "iterator;" on an interface. There should be an easier way to make an object's iterator just go over a set of values.
Note that we might want to represent these values using different objects. E.g. FormData internally has a list of values that are tuples. And when iterating over them we want these to be arrays. (We should also make it clear whether those arrays are reused or are fresh each time. Fresh each time seems easiest.) Note that we should also be clear with respect to whether the iterator returns a snapshot of the underlying data or not. Maybe with syntax? E.g. for NodeList the current idea is for the iterator to be live (match the semantics of the object), but I suspect most others we would like it to be static.
IDL should also take a position on whether to include things like forEach, size, ...
### Definitely do this ArrayLikeIterable => keys(), values(), entries(), [Symbol.iterator](), forEach, all === to their Array.prototype counterparts. (Assumes the host interface defines a .length property) Should require nothing from the author besides being zero-indexed and having a length. As we get proper Array subclasses, this should disappear; it's mostly for slapping onto older interfaces. ### Needs some thought MapLikeIterable => keys(), values(), entries(), [Symbol.iterator](), forEach, with key-value semantics. Spec authors need to define some kind of abstract ordered list of key-value pairs these can work off of. The ES6 spec formalizes this as an ECMASpeak List of ECMASpeak Records {[[key]], [[value]]}, with the additional wrinkle that it represents deletions as setting the key to `empty` in sans-serif font. That's kind of lame, but we do need spec authors to be rigorous somehow. I think spec authors should separately define size, if they want it? Or we could assume they want it, and if we find a case where they don't, we can define MapLikeIterableNoSize ### Unsure SetLikeIterable => like MapLikeIterable, but with the (admittedly weird) set semantics where keys and values are the same, and entries are just [v, v] arrays. Do we have any use case for this?
(In reply to Domenic Denicola from comment #3) > ### Unsure > > SetLikeIterable => like MapLikeIterable, but with the (admittedly weird) set > semantics where keys and values are the same, and entries are just [v, v] > arrays. > > Do we have any use case for this? Well, we have Set-like objects, such as FontFaceSet. ~TJ
> MapLikeIterable => keys(), values(), entries(), [Symbol.iterator](), forEach, with key-value semantics. I see no Symbol.iterator on Map in ES6. Am I just missing it?
(In reply to Boris Zbarsky from comment #5) > > MapLikeIterable => keys(), values(), entries(), [Symbol.iterator](), forEach, with key-value semantics. > > I see no Symbol.iterator on Map in ES6. Am I just missing it? https://people.mozilla.org/~jorendorff/es6-draft.html#sec-map.prototype-@@iterator
Thanks for the concrete proposal Domenic. What do you feel about the prototype of such objects? Do you think they should inherit from Array.prototype (like [ArrayClass] requires now), Map.prototype, etc.? Even with overridden methods that perform additional checking (like checking the types of arguments)? I suggested in http://lists.w3.org/Archives/Public/www-style/2014Jun/0476.html that FontFaceSet shouldn't duplicate the entire Set API -- just enough to be useful, and without the Set.prototype inheritance. But if people are happy with the entire Set API and we have a plan for all new Array/Map/Set-like things, I can be convinced. > As we get proper Array subclasses, ... Where are we on this (and presumably Map/Set subclasses)?
On use cases, BTW, we are using a maplike class for Web MIDI: http://webaudio.github.io/web-midi-api/.
From the WebMIDI point of view, we don't want methods that modify the map. i.e. We don't need the entire Map API.
Chris, thanks for the link. What that shows me is that we need to somewhat automate defining the actual iterator objects as well, ideally...
> What do you feel about the prototype of such objects? I think they should definitely not inherit from the prototypes. They are "-like", not "is a". > What that shows me is that we need to somewhat automate defining the actual iterator objects as well, ideally... If we copy the [[MapData]] structure of the ES6 spec exactly, then we could just reuse the CreateMapIterator and friends from the spec.
(In reply to Domenic Denicola from comment #11) > > What do you feel about the prototype of such objects? > > I think they should definitely not inherit from the prototypes. They are > "-like", not "is a". I 100% disagree, though of course what we really want is Traits. It's really dumb to have a bunch of things that are *similar to* Maps/Sets/etc, but not actually Maps/Sets/etc, such that if you define new methods on Map (like update(), or setdefault(), or other useful things from Python that JS hasn't sprouted yet) they don't show up on the other Map-like things. This is the same exact problem we had with Array-like things, where NodeLists dont' have map/filter/etc and I'm constantly writing: `function find(sel) {return Array.prototype.slice(document.querySelector(sel));}` just so I can get a real fucking Array and call .map() on them. There are sound theoretical reasons to not put Map.prototype in its prototype chain, but in my opinion they're strongly trumped by practical concerns, born out by historical complaints. Let's please not repeat the same errors; author usability trumps theoretical consistency here. (If we just *fixed ES Maps* to not be impossible to subclass safely, this wouldn't be nearly as much of a concern. Alas, es-discuss keeps shooting me down. It's a huge mistake, but what can you do when the spec editors don't agree?)
Right, what you want is indeed traits. Partially because subclassing to impose more restrictions is bad design, and partially because subclassing to impose more restrictions just plain doesn't work in JavaScript. (Unlike in other languages, where it's simply discouraged.) There is really no way it makes sense, neither theoretically nor practically, to put Map.prototype in the prototype chain. The Map.prototype methods are not generic, so this case is very different from Array. The correct thing to do here, instead of abusing the prototypal inheritance hammer, is to turn to other tools (whether they be some future version of traits, or just WebIDL).
(In reply to Domenic Denicola from comment #13) > Right, what you want is indeed traits. Partially because subclassing to > impose more restrictions is bad design, and partially because subclassing to > impose more restrictions just plain doesn't work in JavaScript. (Unlike in > other languages, where it's simply discouraged.) > > There is really no way it makes sense, neither theoretically nor > practically, to put Map.prototype in the prototype chain. The Map.prototype > methods are not generic, so this case is very different from Array. The Map.prototype methods *defined by authors/libraries* are generic. WebIDL can take care of defining that all the ES-defined methods are overridden appropriately (and use a different data structure than the [[MapData]] from Map, so you can't bypass any input restrictions by using Map.prototype.set). > The correct thing to do here, instead of abusing the prototypal inheritance > hammer, is to turn to other tools (whether they be some future version of > traits, or just WebIDL). WebIDL is a hammer we can use for language-defined things. Inheritance is currently the only hammer we can use for author-defined things.
(In reply to Anne from comment #1) > Note that we might want to represent these values using different objects. > E.g. FormData internally has a list of values that are tuples. And when > iterating over them we want these to be arrays. (We should also make it > clear whether those arrays are reused or are fresh each time. Fresh each > time seems easiest.) Any time we want to expose internal data through the iterator, we're going to need to use prose to define the values that get iterated, so I don't think we need anything in the IDL syntax here to handle FormData iteration. > Note that we should also be clear with respect to whether the iterator > returns a snapshot of the underlying data or not. Maybe with syntax? E.g. > for NodeList the current idea is for the iterator to be live (match the > semantics of the object), but I suspect most others we would like it to be > static. For objects that expose indexed properties in a non-live fashion, then we don't need to do anything special -- the default Array iterator is going to work on it too. So I think we don't need anything here either.
(In reply to Cameron McCormack from comment #0) > Currently the spec requires you to define the "iterator behavior" and a > bunch of other definitions "when you use "iterator;" on an interface. There > should be an easier way to make an object's iterator just go over a set of > values. I've added an "iterate a list of values" pre-defined iterator behaviour that you can reference, which avoids needing to define the stuff about the iterator state and returning values. https://github.com/heycam/webidl/commit/d948eb363201bb7f2d66cf52c391704660fb9f82 http://heycam.github.io/webidl/#dfn-iterate-a-list-of-values
That seems nice, but not sufficient for my three multimaps. FormData, URLSearchParams, and Headers, all have an associated list of sorts where each entry consists of a name and a value. I should be able to easily explain what the list is, what the keys/names are, and what the values, and then based on that IDL can "generate" keys(), values(), and Symbol.iterator behavior. Does that make sense? If you disagree I guess I could also define keys(), values(), entries() etc. on my own but then it seems I would like some kind of hook so I can define these methods to return iterators. (I could even see me defining this as some kind of MultiMapIterator mixin that I have implemented by each object. Then maybe later it can be uplifted to IDL.)
(In reply to Anne from comment #17) > That seems nice, but not sufficient for my three multimaps. FormData, > URLSearchParams, and Headers, all have an associated list of sorts where > each entry consists of a name and a value. > > I should be able to easily explain what the list is, what the keys/names > are, and what the values, and then based on that IDL can "generate" keys(), > values(), and Symbol.iterator behavior. > > Does that make sense? If you disagree I guess I could also define keys(), > values(), entries() etc. on my own but then it seems I would like some kind > of hook so I can define these methods to return iterators. (I could even see > me defining this as some kind of MultiMapIterator mixin that I have > implemented by each object. Then maybe later it can be uplifted to IDL.) Yeah, my commit just addresses comment 0. I agree we should have something that will auto-generate keys(), entries(), etc. So I think we need at least: * a way to declare an interface as having a read only array-like or map-like or set-like interface (though I might leave array-like for later), that generates entries(), forEach(), get(), has(), keys(), get size, values(), and @@iterator * a way to declare an interface as having a read-write map-like or set-like (but not array-like) interface, that generates all of the above plus clear(), delete(), and set() And maybe also: * a way to declare an interface as having a single value iterator that generates only entries(), keys(), values(), and @@iterator * a way to declare an interface as having a pair iterator that generates only entries(), keys(), values(), and @@iterator Syntax for that could be the following, which you use inside an interface: // gives you entries(), keys(), values(), @@iterator iterable<ScalarValueString, sequence<FormDataEntryValue>>; iterable<Node>; // gives you the above, plus forEach(), get(), has(), get size readonly maplike<DOMString, float>; readonly setlike<DOMString>; // gives you the above, plus clear(), delete() and set() maplike<DOMString, float>; setlike<FontFace>; Prose would be used to define the values that are exposed, like with the #dfn-iterate-a-list-of-values definition I just added. The behaviour of entries(), keys(), values(), @@iterator, forEach(), get() and has() would all be defined automatically based on that list of values. Prose would be required to define what happens for clear(), delete() and set(). FormData being a multi-map doesn't really fit with all of the Map object's methods -- get() is different, for example, as it returns only a single value. has() is the same. And would you want forEach()? We could have a built-in multi-map-like declaration, but given there's no MultiMap object in ES to model the interface after, I'm not certain which methods we'd want to expose there. So I assume for the moment that you'd just want: interface FormData { // all the existing members iterable<ScalarValueString, sequence<FormDataEntryValue>>; }; This makes me wonder whether we need the current iterator; declaration, which allows you to specify the behaviour of @@iterator but nothing else. Maybe all the time we want to make an object iterable we also want entries(), keys() and values() on it too?
(In reply to Cameron McCormack from comment #18) > Prose would be used to define the values that are exposed, like with the > #dfn-iterate-a-list-of-values definition I just added. The behaviour of > entries(), keys(), values(), @@iterator, forEach(), get() and has() would > all be defined automatically based on that list of values. Prose would be > required to define what happens for clear(), delete() and set(). Or, reading Domenic's comment again, the list of values would be manipulated by clear() etc. too, and so we could omit prose for them if no special behaviour is needed.
1) We want "iterator;". HTMLCollection is something where we cannot expose additional methods, but we want to expose Symbol.iterator. 2) I think we want a way to return an iterator similar to "iterator;", but for normal methods rather than Symbol.iterator. This would allow "polyfilling" at the standards level. E.g. with that I could define keys() and friends myself. 3) I would expect all iterable objects to have forEach(), not just maplikes. Iterating over the same values as Symbol.iterator. (In ECMAScript Array, %TypedArray%, Map, and Set have it.) Given HTMLCollection and similar legacy APIs, getting forEach() needs to be opt-in somehow. 4) I would expect iterable<ScalarValueString, FormDataEntryValue> (no sequenced value) as a multimap is a list with non-unique keys. (This incidentally makes it match get() a bit better I suppose. getAll() is for the "weirder" multimap view.) 5) iterable<> should probably imply Symbol.iterator and forEach(). 6) FormData seems hard to autogenerate the API for due to the filename argument. I noticed Headers currently does argument checks even for get() and has() (should maybe drop that). The way FormData, URLSearchParams, and Headers work is consistent, but I should probably try to abstract them a bit more myself first before we try to move logic into IDL for them (if that is even worth the tax on IDL, there might not be that many multimaps in the end).
(In reply to Anne from comment #20) > 1) We want "iterator;". HTMLCollection is something where we cannot expose > additional methods, but we want to expose Symbol.iterator. OK. > 2) I think we want a way to return an iterator similar to "iterator;", but > for normal methods rather than Symbol.iterator. This would allow > "polyfilling" at the standards level. E.g. with that I could define keys() > and friends myself. But once the iterable<> text is written up this would no longer be needed? It's possible in the future we might need to pass around iterator objects (and what to use an "iterator" type in interface member declarations), but I'd rather wait until we see we need it. > 3) I would expect all iterable objects to have forEach(), not just maplikes. > Iterating over the same values as Symbol.iterator. (In ECMAScript Array, > %TypedArray%, Map, and Set have it.) Given HTMLCollection and similar legacy > APIs, getting forEach() needs to be opt-in somehow. OK. Is distinguishing between "iterator;" and "iterable<>" (gosh this is going to be confusing, suggestions welcome) enough for determining whether to have a forEach()? > 4) I would expect iterable<ScalarValueString, FormDataEntryValue> (no > sequenced value) as a multimap is a list with non-unique keys. (This > incidentally makes it match get() a bit better I suppose. getAll() is for > the "weirder" multimap view.) OK, I'll ensure the internal map data supports non-unique keys, but have the default set() on maplikes avoid adding duplicates. (Which could then be overridden if a spec needed to.) > 5) iterable<> should probably imply Symbol.iterator and forEach(). I think I mentioned the former. Ack for the latter. > 6) FormData seems hard to autogenerate the API for due to the filename > argument. I noticed Headers currently does argument checks even for get() > and has() (should maybe drop that). The way FormData, URLSearchParams, and > Headers work is consistent, but I should probably try to abstract them a bit > more myself first before we try to move logic into IDL for them (if that is > even worth the tax on IDL, there might not be that many multimaps in the > end). All right let's just start with iterable<> for FormData and co.
How about legacyiterable<> (just Symbol.iterator, only for legacy stuff) and iterable<>?
Works for me. I was about to suggest something like [NoMethods] iterable<Element>; for HTMLCollection, but legacyiterable<> is fine.
I think you should get keys(), values(), and entries() for free whenever possible. Possible size too. See https://www.w3.org/Bugs/Public/show_bug.cgi?id=26183#c3
(In reply to Domenic Denicola from comment #24) > I think you should get keys(), values(), and entries() for free whenever > possible. Possible size too. See > https://www.w3.org/Bugs/Public/show_bug.cgi?id=26183#c3 size has really nothing to do with the iterable interface.
(In reply to Domenic Denicola from comment #24) > I think you should get keys(), values(), and entries() for free whenever > possible. Possible size too. See > https://www.w3.org/Bugs/Public/show_bug.cgi?id=26183#c3 Yes I'll make iterable<> add those automatically, but legacyiterable<> leave them out. I'm not sure about size, particularly because you might have an object that can iterate over an unknown-at-iteration-start-time list of values.
I've taken a crack at it. https://github.com/heycam/webidl/commit/1bc1e2904d3cd90d7856e184cb4b5f10d26b8d96 http://heycam.github.io/webidl/#idl-iterable http://heycam.github.io/webidl/#idl-maplike http://heycam.github.io/webidl/#idl-setlike http://heycam.github.io/webidl/#es-iterators http://heycam.github.io/webidl/#es-iterable http://heycam.github.io/webidl/#es-maplike http://heycam.github.io/webidl/#es-setlike Note that I didn't just set <maplikeinterface>.prototype.set to %MapPrototype%.set etc., because that just opens up the issues of how to deal with Map.prototype.set.call(<maplikeobject>) again. So instead I've got backing Map and Set objects hidden off an internal slot of objects that are maplike/setlike, and functions on their prototypes that just forward on to the corresponding built-in function on the hidden Map/Set.
(In reply to Cameron McCormack from comment #27) > Note that I didn't just set <maplikeinterface>.prototype.set to > %MapPrototype%.set etc., because that just opens up the issues of how to > deal with Map.prototype.set.call(<maplikeobject>) again. There's no issue here; the internal Map data isn't part of the prototype; it's a product of calling the Map constructor on the object. Since that doesn't happen, it doesn't matter what the prototype is.
(In reply to Tab Atkins Jr. from comment #28) > There's no issue here; the internal Map data isn't part of the prototype; > it's a product of calling the Map constructor on the object. Since that > doesn't happen, it doesn't matter what the prototype is. Yes it's not an issue if you don't have a [[MapData]] directly on the object. But not having a [[MapData]] directly on the object means that you can't make even the safe functions like <maplikeinterface>.prototype.values === %MapPrototype%.values, since it'll just throw.
Putting Map/Set on the prototype is not what TC39 wants. We should not do it. We should just port new methods as they come in and at some point traits will hopefully make things better.
(In reply to Anne from comment #30) > Putting Map/Set on the prototype is not what TC39 wants. We should not do > it. We should just port new methods as they come in and at some point traits > will hopefully make things better. Has TC39 explicitly said that, or just Domenic? Once again, my concern is not new Map methods (those won't work anyway, since they interact with [[MapData]] directly; IDL is always going to need to port them over with our custom delegating definition). It's about author-defined methods on Map, which operate via the built-in methods, and it would be unfortunate if those didn't work on map-likes.
I think they did. I don't have a pointer and I don't remember anyone advocating this prototype stuff other than you. Again, it seems what we want is traits.
(In reply to Anne from comment #32) > I think they did. I don't have a pointer and I don't remember anyone > advocating this prototype stuff other than you. Again, it seems what we want > is traits. Yeah, but that's nowhere on the roadmap, and in the meantime we'll have several years of things being kinda shitty for authors. :/
Reviewing... The setlike looks a bit strange, since it allows two types. It should only allow one type, I think. ES (for some reason) has keys() -> Iterable<T>, values() -> Iterable<T>, and entries() -> Iterable<[T, T]>, but it's the same T always. In the @@iterator definitions, you use CreateMapIterator and friends directly on the objects, but the objects do not have [[MapData]] internal slots, so this will immediately fail. I don't know how to solve this though, since if you put [[MapData]] on the object then you can use Map.prototype.set.call(obj, "bypassed your typechecking haha"). Maybe you could forward to [[BackingMap]].@@iterator? You have to be careful not to allow anyone access to [[BackingMap]] somehow. It is probably implicit, but I don't see anywhere that explicitly does type conversion on arguments passed to the forwarding functions. Am I correct in assuming this happens implicitly because they are WebIDL methods instead of JS methods? Or is this missing? Overall this looks pretty nice. Very batteries-included.
1) In http://heycam.github.io/webidl/#es-iterators, this bit: If the interface does not have an iterator should probably be: If the interface does not have an iterable declaration ? 2) For the indexed case, why can we not just say that the value of @@iterator is literally the initial value of Array.prototype[@@iterator] (which is %ArrayProto_values% as it happens)? I see no particular need to enforce belonging to the right interface, or do security check bits here; none of that stuff is needed for sanity in this case imo. 3) For the maplike/setlike cases, I guess we can't just have @@iterator point to the same object as Map.prototype[@@iterator] and Set.prototype.[@@iterator] because those do the branding checks Domenic points out? That would have avoided having the bug where "value" should probably be "key+value" for the Map to match how ES Map behaves. :( 4) In the forEach steps for iterable declarations, why not make sure callbackFn is callable before getting "values"? In fact, why not define the property location, as well as current steps 1-4 and 7-8 of its behavior by just saying that it should act as if the interface had: void forEach(Function callback, optional any thisArg = undefined); at the end of its IDL? That would also allow you to defer to the invoke a callback steps for parts of step 11, and remove the necessity for step 12, I think (because now you'd only be specifying prose for an IDL function, not the whole thing from start to finish). 5) For the forEach for setlike/maplike bits, seems you could do something similar. 6) For the setlike/maplike forEach, is a new callbackWrapper created on each forEach call, or is one created then reused? I believe this is page-detectable via examining the stack... 7) For the setlike/maplike forEach, we should be consistent about whether the third arg of callbackWrapper is "o" or "object". 8) Is the callbackWrapper anything more than callbackFn.bind(thisArg) (assuming a new one is created every time and that we're talking canonical bind here)? If so, is it worth making that clearer? 9) By the time I got to "entries" I was getting tired of the copy/pasted "The location of the property is determined as follows" bit. Could we refactor that into a definition or something. 10) "declared use" should be "declared using". 11) The section on "values" has the wrong property name, and no definition of the behavior the function should have. 12) The link to "iterator" in the first paragraph of the "Iterator prototype object" section is broken. 13) I don't follow step 4 of "forwards to the internal map object". Where is "this Function that implements the size property" coming from there? Copy/paste error? 14) In "forwards to the internal map object" after step 7 need to check whether "function" is callable, right? 15) Looks like the idea is that the implementation can interpose some sort of logic of its own for clear/set/delete but not for entries/get/has/keys/values, right? This does mean that the [[BackingMap]] can't be lazily computed/updated, correct? 16) Is [MapClass] still a thing? "Maplike declarations" paragraph 2 makes it sound like it is, but I assume that should say "maplike declaration"? 17) Similarly "Setlike declarations" paragraph 2 should not talk about "[SetClass]", I assume. 18) "forwards to the internal set object" step 4 looks like it also has a copy/paste issue. 19) "forwards to the internal set object" after step 7 need to check that "function" is callable. 20) Iterator prototypes should have a method named @@iterator on them that returns the "this" value. See what %ArrayIteratorPrototype%[@@iterator] does in ES6. 21) We need to define what the .name is for these @@iterator functions. Actually, for all Functions in Web IDL, now that it looks like .name is an ES6 thing. But we can reuse the "Unless otherwise specified, this value is the name that is given to the function in this specification..." language ES6 has for most cases; just not this case. 22) I think Domenic is right that actual type-enforcement is totally missing here for the mutator methods when those are not otherwise specified. Again, I think it would be good to define behavior here in terms of things acting as if they were effectively added to the IDL, which would get you the "this"-checks for free, and argument coercions to the right types for free... Would require a bit of finagling to, e.g., wrap up the "function" being called through to into an IDL Function. Or conversions back from IDL to ES values before doing the [[Call]]. Or some such. 23) The backing map and set are just chaining up to their proto, right? So if someone changes Set.prototype.entries that will change the behavior of .entries() on all setlikes? That's pretty nice!
(In reply to Domenic Denicola from comment #34) > Reviewing... > > The setlike looks a bit strange, since it allows two types. It should only > allow one type, I think. ES (for some reason) has keys() -> Iterable<T>, > values() -> Iterable<T>, and entries() -> Iterable<[T, T]>, but it's the > same T always. Yeah, copy-paste error in the green block there and in the following paragraph. (Which is odd because I remember taking care of that; must have lost some edits while git stashing.) Fixed: https://github.com/heycam/webidl/commit/7ceed8553feeab1a3ffbc0e138c68f0e795b99d4 > In the @@iterator definitions, you use CreateMapIterator and friends > directly on the objects, but the objects do not have [[MapData]] internal > slots, so this will immediately fail. I don't know how to solve this though, > since if you put [[MapData]] on the object then you can use > Map.prototype.set.call(obj, "bypassed your typechecking haha"). Maybe you > could forward to [[BackingMap]].@@iterator? You have to be careful not to > allow anyone access to [[BackingMap]] somehow. Yes my intention was to grab the [[BackingMap]] and call CreateMapIterator with that but I guess I forgot here. Fixed: https://github.com/heycam/webidl/commit/4de741337480e65c5b36f5bf667dbae421c3f7ac > It is probably implicit, but I don't see anywhere that explicitly does type > conversion on arguments passed to the forwarding functions. Am I correct in > assuming this happens implicitly because they are WebIDL methods instead of > JS methods? Or is this missing? No, it's missing. Added (at the expense of expanding this section out some more): https://github.com/heycam/webidl/commit/06828878e769bbbe5e20d5f9a55ae0bb8e093a22 > Overall this looks pretty nice. Very batteries-included. Thanks!
Thanks very much for the detailed review, Boris. (In reply to Boris Zbarsky from comment #35) > 1) In http://heycam.github.io/webidl/#es-iterators, this bit: > > If the interface does not have an iterator > > should probably be: > > If the interface does not have an iterable declaration > > ? Yes. I also fixed some other remaining references to #dfn-iterator. https://github.com/heycam/webidl/commit/0251e92e1f78f954ee6d520cab248109ee44c1fd > 2) For the indexed case, why can we not just say that the value of > @@iterator is literally the initial value of Array.prototype[@@iterator] > (which is %ArrayProto_values% as it happens)? I see no particular need to > enforce belonging to the right interface, or do security check bits here; > none of that stuff is needed for sanity in this case imo. I guess you're right; array iterators are intentionally generic, so we shouldn't need to do a check on |this|. The security check bits can of course then go. Convenient it has a %name% already! https://github.com/heycam/webidl/commit/006100af0c3206762d472762b6ecc4b757861b73 > 3) For the maplike/setlike cases, I guess we can't just have @@iterator > point to the same object as Map.prototype[@@iterator] and > Set.prototype.[@@iterator] because those do the branding checks Domenic > points out? That would have avoided having the bug where "value" should > probably be "key+value" for the Map to match how ES Map behaves. :( Yeah, that's why. :( I completely missed that Map.prototype.@@iterator === Map.prototype.entries and not values. But I think we ought to make <maplikeinterface>.prototype.@@iterator === <maplikeinterface>.prototype.entries, and similarly for setlike interfaces and "values". https://github.com/heycam/webidl/commit/030a5ecbf18a39a5f724d300734b71caba792b28 https://github.com/heycam/webidl/commit/56c2185c86a78b0ecb8696a8a6467f301bcb03f9 > 4) In the forEach steps for iterable declarations, why not make sure > callbackFn is callable before getting "values"? In fact, why not define the > property location, as well as current steps 1-4 and 7-8 of its behavior by > just saying that it should act as if the interface had: > > void forEach(Function callback, optional any thisArg = undefined); > > at the end of its IDL? That would also allow you to defer to the invoke a > callback steps for parts of step 11, and remove the necessity for step 12, I > think (because now you'd only be specifying prose for an IDL function, not > the whole thing from start to finish). I guess we could do that. I'm usually a bit wary of defining things "as if" some other things, since I've ready plenty of spec text that did something like which worked well for the usual case but was broken from some edge cases. I suppose I should be able to get it right though. :-) https://github.com/heycam/webidl/commit/ce4bf1eed4a4e978cee642f91be0d6622425113e Let me know if you think that looks OK and I'll try with it for setlike/maplike. > 5) For the forEach for setlike/maplike bits, seems you could do something > similar. > > 6) For the setlike/maplike forEach, is a new callbackWrapper created on > each forEach call, or is one created then reused? I believe this is > page-detectable via examining the stack... Ergh. I wonder what the best way to write "create a function one that captures an environment with these variables" is. Maybe it's just easier not to use {Map,Set}.prototype.forEach... Added a note for the moment. > 7) For the setlike/maplike forEach, we should be consistent about whether > the third arg of callbackWrapper is "o" or "object". It should be object. I removed o since we don't use it. https://github.com/heycam/webidl/commit/8ddacdf9189e2a8edc2158b079007a8111b17821 > 8) Is the callbackWrapper anything more than callbackFn.bind(thisArg) > (assuming a new one is created every time and that we're talking canonical > bind here)? If so, is it worth making that clearer? It's also fixing up the third argument passed to the callback to be the maplike object (and not the [[BackingMap]] inside it). > 9) By the time I got to "entries" I was getting tired of the copy/pasted > "The location of the property is determined as follows" bit. Could we > refactor that into a definition or something. Yes, that copying and pasting sucks. And I think I'm missing that wording on some members. I filed bug 26970 for this. > 10) "declared use" should be "declared using". https://github.com/heycam/webidl/commit/58c1f96f517b925c6774230644db00d29cb81ac6 > 11) The section on "values" has the wrong property name, and no definition > of the behavior the function should have. It's pointing to @@iterator so doesn't need its own behaviour definition. Fixed keys -> values though. > 12) The link to "iterator" in the first paragraph of the "Iterator prototype > object" section is broken. Fixed that to be "iterable declaration". > 13) I don't follow step 4 of "forwards to the internal map object". Where > is "this Function that implements the size property" coming from there? > Copy/paste error? Gah. Fixed by removing the name of the function we're defining (here and in various other functions). > 14) In "forwards to the internal map object" after step 7 need to check > whether "function" is callable, right? Yes. https://github.com/heycam/webidl/commit/bb36088f097b2f2a239fb6bd77643512945b9dca > 15) Looks like the idea is that the implementation can interpose some sort > of logic of its own for clear/set/delete but not for > entries/get/has/keys/values, right? This does mean that the [[BackingMap]] > can't be lazily computed/updated, correct? That script could mess with the forwarded-to property and therefore you can't just pretend you have a backing map/set is is something I realised in the shower this morning, and makes me wonder if it's a good idea or not to have the backing map/set at all. For example for the Font Loading API, it would be a pain to have to use an actual JS Map object as the storage for the FontFace objects. Interested to know what you and other think about this. > 16) Is [MapClass] still a thing? "Maplike declarations" paragraph 2 makes > it sound like it is, but I assume that should say "maplike declaration"? Yeah that should have been changed to "maplike declarations". https://github.com/heycam/webidl/commit/9a52bc13afa96836cc71b552612a281214d49150 > 17) Similarly "Setlike declarations" paragraph 2 should not talk about > "[SetClass]", I assume. Yep. Especially since [SetClass] never even got added to the spec. > 18) "forwards to the internal set object" step 4 looks like it also has a > copy/paste issue. Fixed above. > 19) "forwards to the internal set object" after step 7 need to check that > "function" is callable. Fixed above. > 20) Iterator prototypes should have a method named @@iterator on them that > returns the "this" value. See what %ArrayIteratorPrototype%[@@iterator] > does in ES6. Ah so it should. https://github.com/heycam/webidl/commit/9c9b0e71e13f834737ebb92d14a4937242c1528d > 21) We need to define what the .name is for these @@iterator functions. > Actually, for all Functions in Web IDL, now that it looks like .name is an > ES6 thing. But we can reuse the "Unless otherwise specified, this value is > the name that is given to the function in this specification..." language > ES6 has for most cases; just not this case. Yeah... oh, looks like we have bug 22392 for that. > 22) I think Domenic is right that actual type-enforcement is totally missing > here for the mutator methods when those are not otherwise specified. Again, > I think it would be good to define behavior here in terms of things acting > as if they were effectively added to the IDL, which would get you the > "this"-checks for free, and argument coercions to the right types for > free... Would require a bit of finagling to, e.g., wrap up the "function" > being called through to into an IDL Function. Or conversions back from IDL > to ES values before doing the [[Call]]. Or some such. Done earlier, though not by doing "as if" stuff. I suppose we could treat the backing maps/sets as objects that implement callback interfaces, and call them that way? > 23) The backing map and set are just chaining up to their proto, right? So > if someone changes Set.prototype.entries that will change the behavior of > .entries() on all setlikes? That's pretty nice! Yes that's true. Sounds nice, but the point about not being able to implement APIs as if you had a backing map/set you bring up in (15) is a worry. :(
> Fixed: https://github.com/heycam/webidl/commit/4de741337480e65c5b36f5bf667dbae421c3f7ac This forgot to replace "object" by "backing" in the Create*Iterator calls, no? > Added (at the expense of expanding this section out some more): I haven't reviewed this in detail yet; it'll probably happen when we go to implement it... > Let me know if you think that looks OK and I'll try with it for setlike/maplike. It looks pretty good. The description of where the property lives and its attributes is actually redundant, since having it in IDL would pin all that down. Those parts are only needed for forEach in the maplike/setlike cases, afaict. > I wonder what the best way to write "create a function one that captures an > environment with these variables" is. Hmm. A bound function? The problem is that you want to bind the _third_ argument (and the this value) but not the first two args. :( I'm not sure what a sane solution is here. As a UA implementor I can make this utility function not show up in things like Error stacks and .caller walks, of course (since I assume it's fundamentally an implementation detail we don't want to expose), but I'm not sure what the right way to spec this sort of thing is. > It's pointing to @@iterator so doesn't need its own behaviour definition. Ah, indeed. I missed that. > Interested to know what you and other think about this. I was assuming the requirement that there be an actual backing JS Map/Set was a conscious design choice. But yeah, how the UA interacts with that Map/Set is an interesting question. It could be done via [[Get]] and [[Call]] equivalents, which would let the page override methods on it (and e.g. prevent the UA from putting the right things in it). It could be done via saved clean copies of the Map/Set methods, or declaring that the prototype of this Map/Set lives in some clean global; I think these might be equivalent for sets as long as the Map/Set methods don't try to [[Get]] on the object... But for things like keys() and values() the returned iterator would also live in that other global, or something. Unless we wrapped it up in our own iterator... So both those approaches have some fragility and ickiness going on. I'm not sure what best to do here. :( > I suppose we could treat the backing maps/sets as objects that implement > callback interfaces, and call them that way? Yes. That doesn't address the basic "page can mess with it" issue, of course; it's just a different way to talk about doing [[Get]] and [[Call]]. Two other comments: 24) What is the point of legacyiterable? So far the only uses I've seen are in bug 23212 and look wrong to me. The way I see it, we need legacyiterable for cases that (A) don't have indexed getters (or do but want the iterator to iterate something _other_ than the indexed props) and (B) don't want the extra methods "iterable" adds. Are there any such cases? 25) Also as mentioned in bug 23212, it might be nice to have a way to get the newfangled iterable methods for something with an indexed getter without having to redefine its iteration behavior like iterable<> forces you to do. Perhaps an arrayiterable<> that just adds the methods but keeps %ArrayProto_values% as the @@iterator?
Also, 26) http://heycam.github.io/webidl/#es-iterators says that for indexed props %ArrayProto_values% is used unless there is an explicit iterable declaration, in which case the default iterator object is used (and its behavior needs to be defined). But http://heycam.github.io/webidl/#idl-iterable says that if you have both an iterable declaration and indexed props then the indexed prop values get iterated. These are contradicting each other and we should decide which behavior we actually want. See bug 23212 comment 4 for the relevant spec quotes.