Iterator.from should guarantee a branded object, and helpers should brand-check the receiver

[ljharb]

Additionally, iterators produced by a generator, or by any of the builtin iterators, should not need wrapping.

This allows Iterator.from to be reliably used like Promise.resolve - given any user-provided alleged iterator-like object, I can pass it through Iterator.from and I'll have a properly-functioning "real" iterator.

[devsnek]

i thought we came to the conclusion that this was not meaningful...

given any user-provided alleged iterator-like object, I can pass it through Iterator.from and I'll have a properly-functioning "real" iterator.

because this is already true?

helpers should brand-check the receiver

why?

[bakkot]

Can you describe what you're asking for in terms of observable behavior of code, rather than in terms of spec-internal concepts like brands?

[ljharb]

Sure.

const iteratorLikes = [
  { next() {} },
  { __proto__: Iterator.prototype, next() {} },
  { __proto__: Iterator.prototype },
];

iteratorLikes.forEach(x => Iterator.prototype.map.call(x, x => x)); // each of these should throw

iteratorLikes.map(x => Iterator.from(x)).forEach(x => Iterator.prototype.map.call(x, x => x)); // these should all not throw

[bakkot]

// each of these should throw

Why?

[bakkot]

Let me give the argument against those throwing: right now it would be entirely reasonable to have code which looks like [Symbol.iterator]: () => ({ __proto__: [].values().__proto__.__proto__, next() { /* ... */ } }), and it would be very strange for iterators produced in that way to suddenly gain a map method which throws if you invoke it.

[ljharb]

Why would that be strange? That's how most builtins behave - merely making an object with the same prototype chain simply doesn't ever guarantee you can use all the methods on it, because internal slots exist.

[bakkot]

It would be strange because right now there is no brand to check, so there is nothing to discourage writing the above snippet. There is currently exactly one method on Iterator.prototype, and it doesn't brand check, so there's nothing to suggest the above code is bad.

In any case, why should it throw?

[zloirock]

Iterator.from should guarantee a branded object

It's already true, but "brand" in this case is just the iterators protocol and Iterator.prototype in the prototype chain.

helpers should brand-check the receiver
each of these should throw

Definitely not. Iterators are protocol - and the first 2 satisfy the requirements of required parts of the protocol.

That's how most builtins behave - merely making an object with the same prototype chain simply doesn't ever guarantee you can use all the methods on it, because internal slots exist.

It's the same protocol like array-like. It's the same like

Array.prototype.map.call({ length: 1, 0: 1 }, x => x)

[zloirock]

Also, just see this a quite popular use case.

[ljharb]

Can you provide some examples of how it's popular?

[zloirock]

At least - all polyfills of iterators, transpiled generators, etc. Without setting a brand as you want (that's impossible in engines without proper subclassing support) those helpers will not work with them. This is just the closest use case to me.

[ljharb]

Well sure, any engine that needs iterators polyfilled wouldn't have iterator helpers either - polyfills aren't relevant here, since an Iterator Helpers polyfill would be able to handle the difference.

Any use cases that aren't polyfills?

[devsnek]

I'm still extremely confused on what the use case is for the helper methods throwing. Can someone elaborate?

[zloirock]

@ljharb sorry, but it's full bullshit.

You propose that Number.range(0, 10).map(fn) can't be polyfilled.

[zloirock]

All cases of iterator usage support generic iterators that can be created via protocol in the userland - all syntax features like for-of, spread, etc. All built-ins like Map or Promise.all. Why do you think that those helpers should not support such iterators?

[zloirock]

Also, why did you ignore all comments about the protocol?

[zloirock]

@devsnek it seems for unknown for me reason he thinks that all iterators should have an internal private slot that can be created only in built-in iterators and iterators created in the userland via protocol, something like { __proto__: Iterator.prototype, next }, are unacceptable for helpers from this proposal. I can't understand any motivation for this.

[ljharb]

it's full bullshit

Please remember we have a Code of Conduct.

[ljharb]

I'm still extremely confused on what the use case is for the helper methods throwing. Can someone elaborate?

It ensures that only proper iterators that follow the semantics we expect can be used with the helper methods, just like how Promise.resolve(x).then() is necessary and Object.setPrototypeOf(x, Promise.prototype); x.then() doesn't and shouldn't work.

[zloirock]

A very bad comparison. Here is much closer

Object.setPrototypeOf(x, Promise.prototype); x.catch(fn); x.finally(fn);

that works and should work.

[zloirock]

And it works for the same reason why should work on methods from this proposal - a protocol, "thenable" in this case.

[bakkot]

I am confused about what you mean by "proper iterators that follow the semantics we expect". Can you say that more precisely, i.e. in terms of what observable behaviors are allowed or ruled out for "proper iterators", and how having Iterator.prototype.map.call(x) throw would help ensure that?

just like how Promise.resolve(x).then() is necessary and Object.setPrototypeOf(x, Promise.prototype); x.then() doesn't and shouldn't work.

I mean, that "doesn't work" because then is the thing which defines the thenable protocol; there is nothing to which Promise.prototype.then could defer. But Object.setPrototypeOf(x, Promise.prototype); x.finally() does work as long as x has a then method, because finally is defined in terms of the protocol on its receiver.

This is exactly like how Iterator.prototype.map is defined in terms of the iterator protocol on its receiver, so as long as x has a next method, Object.setPrototypeOf(x, Iterator.prototype); x.map() works.

(I don't think this is always a good idea, because not every class needs to be a protocol - Promise shouldn't have been, but it is - but Iterator is already a protocol; that ship has sailed.)

[devsnek]

It ensures that only proper iterators that follow the semantics we expect can be used with the helper methods

This is the part I don't understand. We explicitly want these methods to work on iterators, which have a pretty clear definition. Are you asking to change the definition of iterators? Are you saying the methods should only work on a subset of iterators? Something else? To what end?

[ljharb]

#117 (comment)

It's also worth noting that the behavior of Iterator.from(obj) will differ if obj inherits from an %Iterator.prototype% from another realm: Iterator.from(new class extends Iterator {}) will not check for next, but Iterator.from(new class extends someIFrame.contentWindow.Iterator {}) will check for next.

This is another reason we need to use internal slot instead of instanceof mechanics, altho this one, at least, can be alternatively handled with some kind of "%IteratorPrototype% from any realm" prose inside an AO that recurses up the prototype chain.

[zloirock]

No, we don't need it. I even don't want to argue why since you just ignored all previous arguments.

[zloirock]

Instances of "%IteratorPrototype% from any realm" anyway should be wrapped in Iterator.from since they are... from another realm. They could not have any polyfilled in the current realm methods and they will not be instances of prototypes from this realm that is bad for object classification.

[ljharb]

Polyfilling concerns never have or will dictate how proposals operate, so I'm not sure why that keeps getting brought up.

[zloirock]

Polyfilling is just one example of a changing environment.

[bergus]

@ljharb All proposals are evaluated in terms of web compatibility and developer experience. Why do you think polyfills should be ignored?

[Mr0grog]

I’m not a committee member or anyone special, but as a regular old developer who’s really excited to see these helpers land, I’m a little confused by why the helpers need to brand-check like this. I can see value in Iterator.from(x) making some extra guarantees for me if I want it, but requiring it in the helper methods definitely doesn’t match my expectations or my experience with other parts of ECMAScript — @zloirock’s Array.prototype.map.call example seems directly analogous, and in general I would have expected that I could do something similar with these new helpers and any object I could use for-of/for-await-of on. Unless I’m misunderstanding, this brand-check would definitely break that expectation.

I’m not super worried about the polyfilling case (the argument that an environment that needs polyfills won’t have anything else that’s doing the brand-checking makes sense, even if it means no polyfill will be able to match the expected behavior exactly, and some code may break when moving from a polyfill to the real thing, which is obviously not great). But it seems like there’s plenty of value to be had in supporting other interesting use-cases people might come up with.

Like @zloirock’s Array.prototype.map example, the comparison to promises also seemed reasonable to me, but I’m not sure I understand the example given:

just like how Promise.resolve(x).then() is necessary and Object.setPrototypeOf(x, Promise.prototype); x.then() doesn't and shouldn't work

I don’t think I would have expected that to work — why would I think the built-in then would behave as if the object it’s attached to was already resolved, or that it resolves to itself? (Which is how I read the implied expectations of that code). On the other hand, I would certainly expect this to work, and it does:

// Not actually a promise, but meets the protocol requirements.
const notAPromise = {
  then (onResolve, onReject) {
    setTimeout(() => onResolve(5), 100);
  }
};

const result = await notAPromise;

(This involves the same sort of “you need to implement the protocol methods” contract iterators do and involves no special branding. It seems much more analogous to what’s being talked about here to me. I tried this out in Node.js, Safari, and Firefox, but will admit I have not read over the spec to see if they aren’t all technically misbehaving.)

[ljharb]

@bergus absolutely the web compat of existing polyfills matters a ton - more than of most code. However, if polyfillability were a workable concern, no new syntax would land, and a bunch of existing proposals wouldn't have been able to land as-is. The committee has explicitly decided many times that while polyfills are important to consider, polyfill ability is not.

[zloirock]

The new syntax is about transpilers. It seems transpilability is a concern since almost all syntax is transpilable. Polyfills about built-ins. Proxy is not properly polyfillable - how often it's used now? Even in use cases where it's extremely useful. And polyfillable Promise, Map, Array.from? However, Proxy is unpolyfillable because it's really required - unlike this case.

Adding a brand check here will make ALL iterators unpolifillable in the future - I already added an example:

Number.range(0, 10).map(fn);

will not work since polyfilled Number.range will not set the required brand.

Sure, it can be worked around by polyfilling methods from this proposal everywhere - even after 10, 20, 30... years - and it will increase the size of bundles of almost all websites that use polyfills to some dozen of KB forever.

But since a brand check here is not something really required - it's full bullshit.

If the concern about polyfillability where it's possible and not required will not be revised - it will be required to take some actions.

[michaelficarra]

It is not clear what kind of concrete solution we could have here. Iterators are not — and will not be — branded.

[ljharb]

"will not be" is a pretty absolute statement to be making on a proposal you hope to advance :-/

[zloirock]

Rather making branded objects from iterators looks like a proposal. Destructive and absolutely unrelated to the current proposal.

[michaelficarra]

@ljharb Sorry, maybe "cannot be" would have been better wording? We do not control where iterators appear. They can just be created anywhere by anyone without a brand and without our blessing.

[ljharb]

@michaelficarra absolutely. but we can force those to be passed through Iterator.from first, just like we do with Promises.

[bakkot]

@ljharb I don't know what you mean by "just like we do with Promises". { __proto__: Promise.prototype, then(){ /* ... */ } }.finally(fn) works fine.

[ljharb]

And all the ways that it still works "fine" even if the then is not well-behaved, are because the Promise produced by the .finally call is branded and ensures as many constraints as it can. The same should apply to anything Iterator.from returns - that a misbehaving next method should be as maximally constrained as possible, via a brand.

[bakkot]

the Promise produced by the .finally call is branded

No, it isn't.

[ljharb]

It's not? Promise.resolve(theThing) === theThing will be true, no? If not, I messed up on the finally proposal :-)

[bakkot]

let mySpecialThing = {};
let fakePromise = {
  __proto__: Promise.prototype,
  then(){ return mySpecialThing; }
};
fakePromise.finally(()=>{}) === mySpecialThing // true

[ljharb]

Missed opportunity. None the less, await and async function returns all have the properties I'm talking about, as does Promise.resolve, so I'd expect the same from an Iterator coercion method.

[bakkot]

I am still not clear on the property you're talking about. Can you describe it in terms of the observable behavior for Iterator.from that you're asking for?

[bakkot]

Concretely: because iterators do not have internal state visible to the spec, the only possible behavior for Iterator.prototype.map is to invoke this.next(). It's true that we could add a brand and check that brand, but we'd still have to call this.next() at some point; there's no "iterator state" to store in an internal slot or anything like that.

Above you suggested that { __proto__: Iterator.prototype, next() {} }.map(fn) should throw. Presumably this would be accomplished by adding a brand, checking that brand, and then invoking this.next() anyway.

This hypothetical brand check would add complexity, and makes otherwise functional code throw, so there needs to be some reason for it.

As we've just discussed, the brand check would make these helper methods unlike Promise.prototype.finally (and Promise.prototype.catch, for that matter), so this is suggesting that we break with precedent; it's not just doing what we always do.

So: what advantage does the additional brand check have? Above you said "it ensures that only proper iterators that follow the semantics we expect can be used with the helper methods", but I have no idea what "proper iterators that follow the semantics we expect" means or how this brand would help verify that. Maybe you can write that out? What specific behavior constitutes a "proper iterator", and how does the proposed brand check ensure that behavior is followed?