Maplike/setlike iterators shouldn't use MapIteratorPrototype/etc

[tabatkins]

#1138 switched maplike/setlike interfaces to use Infra maps/sets, and as part of that had to redefine some text out of the ES spec, notably reusing the language of CreateMapIterator and subbing in a new closure that worked on the infra map/set rather than the ES Map/Set it previously used.

Per discussion with @bakkot and @ljharb, the ES spec doesn't, in fact, intend for this to be pluggable in quite this way; notably, apparently it's bad to create an iterator inheriting from %MapIteratorPrototype% but with a different iteration behavior. (CreateIteratorFromClosure looked pluggable, but in reality the second and third arguments should be unique per call site.)

They suggest instead that I should define a new prototype object, descended from %IteratorPrototype%, and define its methods itself; this is very short and simple to do, as shown in the definition of %MapIteratorPrototype%. (I shouldn't just use %GeneratorPrototype%, like a userland generator, because then I have to worry about .return(), which I don't want to worry about.)

Patch incoming.

[domenic]

-1; reusing %MapIteratorPrototype% and %SetIteratorPrototype% is something all engines do, and something we intentionally designed. ES should allow this kind of layering so that other specs can vend such classes, without having to wrap things up into a [[BackingMap]]/[[BackingSet]].

[bakkot]

If it's intended, that's fine with me. (The behavior in #1138 does work with the ES spec as currently written, though we didn't really intend for it to when we did the refactoring in tc39/ecma262#2045.)

It's conceptually weird to me, and I'm surprised it doesn't cause problems for implementations - I would have expected implementations of MapIteratorPrototype to need to hold an actual ES Map - but I don't think it'll actually matter for users.

Edit: Specifically, the main user-observable consequence of re-using MapIteratorPrototype is that doing (new Map)[Symbol.iterator].next.call(webIdlMapLike[Symbol.iterator]()) will work (and conversely, with new Map and webIdlMapLike switched). This is a little surprising to me as a user, but not a big deal because as a user I will literally never notice that. But I'm not sure that engines will actually want to implement that behavior. If engines are fine with this, I don't see a problem.

[ljharb]

It's perfectly reasonable to ask 262 to provide these affordances for layered specs; that doesn't mean we do at the moment, though.

@domenic are you saying there's existing web iterators where %MapIteratorPrototype%.next.call won't throw?

[bakkot]

reusing %MapIteratorPrototype% and %SetIteratorPrototype% is something all engines do

I couldn't find any examples of this right now, poking around a little, though it's entirely possible I've just missed it. E.g., FontFaceSet is a setlike, but in FF, Chrome, and Safari, document.fonts[Symbol.iterator]().next === (new Set)[Symbol.iterator]().next is false.

[domenic]

Nah I'm saying that there are web iterables that return iterators that are instances of MapIteratorPrototype and vend keys/values based off of a separate internal data store (which the IDL spec used to model as an internal %Map%)

[domenic]

I couldn't find any examples of this right now, poking around a little, though it's entirely possible I've just missed it.

Hmm OK, I thought I tested this a while ago but perhaps not. Well, if all browsers are violating the spec in the same way, then perhaps I need to reconsider my position :). Please hold for more investigation and testing...

[domenic]

This is tricky because there aren't too many maplikes and setlikes in web platform specs and even fewer are implemented widely. Findings so far:

• Firefox uses %MapIteratorPrototype% for AudioParamMap, EventCounts, and RTCPeerStats. It does not use %SetIteratorPrototype% for FontFaceSet.
• Safari uses %MapIteratorPrototype% for RTCPeerStats. It does not use %SetIteratorPrototype% for FontFaceSet.
• Chrome does not use %MapIteratorPrototype% or %SetIteratorPrototype% for any of the above. However, it does use the same iterator prototype object for AudioParamMap, EventCounts, and RTCPeerStats.

Given this total lack of interop we could kind of go in any direction.

My preference would be for the current spec (modulo layering fixes), i.e. use %MapIteratorPrototype% and %SetIteratorPrototype%. That would avoid telling Safari and Firefox, who implemented maplikes "correctly" (i.e. matching the spec), to do something new.

Alternatives include:

• Codify Chrome behavior to avoid (ab)using %MapIteratorPrototype%/%SetIteratorPrototype%.
• Codify Safari/Firefox behavior where %MapIteratorPrototype% is used for maplikes but a new prototype is used for setlikes. Would that setlike prototype be shared between all setlikes, or setlike-specific? It's unclear what each browser implements since they seem to both have only one setlike, but I would lean toward shared if we went this route...

[ljharb]

It seems like the ideal approach is for 262 to provide affordances to make it easy for, eg, AudioParamMap to have its own iterator prototype (rather than weirdly using Map's iterator prototype).

Why would it be desirable to use the iterator prototype that is exclusively intended for Maps with "not Maps"?

[domenic]

Because they're maplike.

[tabatkins]

That last option (using %MapIteratorPrototype% but not %SetIteratorPrototype%) doesn't seem very sensical; it's just specifying bugwards compat, rather than a behavior that actually makes sense.

Why would it be desirable to use the iterator prototype that is exclusively intended for Maps with "not Maps"?

I mean, these are Maps in the important senses; that's the whole point of being maplike. Aside from some internal impl details, authors can treat them exactly like any other map.

[ljharb]

right, but it's not a Maplike Iterator, it's a Map iterator. It's not intended for, designed for, or meant for Map-likes.

[domenic]

Also, please note that you're incorrect in talking about what they were "intended" for; maplikes were co-designed with Maps in the ES2015 timeframe and reusing %MapIteratorPrototype% (and allowing it to be reused on a spec-level, using the [[BackingMap]] strategy) was a conscious design decision of the people involved, including on the ES side.

[ljharb]

@tabatkins a Map is something that has a [[MapData]] slot, that i can Object.getOwnPropertyDescriptor(Map.prototype, 'size').get.call and receive a meaningful answer. If these qualify, then Map's IteratorPrototype makes sense.

@domenic thanks for the correction on the history of it; it seems telling that such a thing hasn't been discussed in committee, and nobody seemed to know about it, when we discussed refactoring these builtins, since at least 2014.

[domenic]

a Map is something that has a [[MapData]] slot, that i can Object.getOwnPropertyDescriptor(Map.prototype, 'size').get.call and receive a meaningful answer.

I know you are very focused on internal slots and brands, but that does not hold for everyone, and it certainly did not hold for the ES2015 editor. At the time TC39/the Web IDL community (mostly on public-script-coord) designed Map as a contract based on a set of public methods, and thought that every class which conformed to those methods had an equal right to vend %MapIteratorPrototype% instances from those methods.

[ljharb]

The committee's makeup has certainly shifted; the impetus seems to currently be towards minimizing observable lookups by this implicit "contract" (which for Map is just "the constructor calls .set", and Set .add, from what I can tell), and moving towards something that exclusively uses brands for the receiver and uses brand + fallback-to-lookup for arguments.

It's unfortunate that WebIDL has designed a bunch of things with an understanding that TC39 seems to not have widely (or at least, overtly) shared for the last half-decade.