"Don't run arbitrary code that you don't trust" is, simply put, not a tenable position. I'm pretty sure that the supermajority of actually evaluated JavaScript comes from NPM dependencies, which only a fool would fully trust. There are thus good reasons for limiting the powers available to partially trusted code, as supported by projects like Hardened JavaScript and LavaMoat. The language itself can foster or hinder that, and being able to absorb an arbitary iterable or iterator into a built-in %Iterator% instance that interacts with its source only through the next/return/throwiterator interface would fall into the former category.
But there's also an independent reason to prefer an Iterator.from that always wraps its input—that's what the other from functions do! Array.from, Uint8Array.from (and %TypedArray%.from in general), and all of the incoming Temporalfrom functions have the common characteristic that their output is never an identity reference to their input. This is a nice pattern to rely upon, and I would support a normative change to bring Iterator.from into alignment. I just don't know if I'm willing to champion such an effort.
I'm pretty sure that the supermajority of actually evaluated JavaScript comes from NPM dependencies, which only a fool would fully trust.
Strong disagree. Imposing a runtime cost on all your users simply to be defensive against code in your dependencies is the wrong side of the tradeoff for almost everyone. There are extremely rare cases where that runtime cost is worth it, although most of those cases would be better served by simply avoiding those dependencies entirely (especially these days). Even then, those cases need this defensiveness only in application code at the boundary between that code and its dependencies, which is a tiny fraction of all code and is not something the language should be optimizing for.
(And as you well know, you would need considerable discipline beyond just coercing a single interface to get any actual security benefits.)
I would support a normative change to bring Iterator.from into alignment
I would be strongly opposed to such a change, which would add cost for little-to-no benefit. Whether a function returns is an identity reference to its input is not a property that matters in almost any circumstance; the important thing is whether it shares state. Iterators cannot be cloned and therefore inherently share state. So there's not any particular point in adding a wrapper.
(Anyway, you can also use Iterator.from with iterables, which will usually have this not-the-same-reference property, and to achieve the defensiveness goal you'd still need to wrap the result of [Symbol.iterator]() even in that case.)
This isn't incurring any per-step runtime cost because (the pointlessness of this example aside) we don't need to use any operations that don't have a purpose in the transformation pipeline
Just to try to make the difference a little more concrete, it's the difference between these two things:
import { map, filter } from Iterator;
let values = [...iter
|> wrap() // omitted, so does not add a layer of next() calls
|> map(x => x, %)
|> filter(_ => true, %)];
// and
let values = Iterator
.from(iter) // needed for chaining, adds a layer of calls
.map(x => x)
.filter(_ => true)
I meant the cost of having Iterator.from add a wrapper even when its argument already inherits from Iterator. Since almost all iterators do indeed so inherit, it has no overhead except a single call with the current semantics, and would have considerably more if we changed it as you propose.
Subclasses can always override a method in a fucked up way
Yes. My issue, plainly stated, is that this is the one and only place in the language where iterator is treated like a class and not a protocol. Every other place where the language accepts an iterable argument, it only uses that argument by consuming the iterator protocol.
There is now exactly one place where this is not true.
Here is a problem demo with no subclassing at all:
In all of these cases, it's not an iterator argument, it's an iterator receiver, so the protocol doesn't apply - although when wrapped in Iterator.from, the protocol is invoked on the wrapped iterator just fine.
The same would be true if you did evilIter.next = () => 42;, entirely unrelated to iterator helpers.
For the record, across the dominant three implementations, the benchmarked performance of Iterator.from(…).reduce((_, x) => x) over 10k integral number array elements seems like it would drop by just under 20% in current JavaScriptCore and by just under 10% in current SpiderMonkey and V8 (JSC benefiting more from just dropping prototype traversal, SM and V8 benefiting more from an own receiver-bound next).
In all of these cases, it's not an iterator argument, it's an iterator receiver, so the protocol doesn't apply
Huh? Array.from takes an iterator argument. This isn't a class method so there's no receiver. The iterator argument is consumed by interacting with it through the iterator protocol, which is the only thing we can be sure is safe to do.
OK yeah, but what is the problem here? Yes the identity of the next function changed, but while that's outlandish my reading of the iterator protocol is that it's still completely conformant.
The novel problem I am pointing out is that a valid iterator may behave in two (or more) completely different ways depending ONLY on how it is consumed.
Even if someone could carry out your attack it would not have this property: the behavior of the consumer would not be relevant.
Dropping a prototype traversal? Huh? Which prototype traversal?
The one inherent to OrdinaryHasInstance in Iterator.from, and also in looking up methods on the result of Iterator.from.
Why would it be a perf benefit to have an "own receiver-bound" next?
The benefit to having an own next is obvious—no need for prototype traversal. The benefit to "receiver-bound" is because %WrapForValidIteratorPrototype%.next is similar to Function.prototype.bind but at invocation time rather than construction time, which apparently makes a huge difference in SpiderMonkey (SetPrototypeOf(ArrayIteratorPrototype, null) drops performance of Iterator.from(arr10k).reduce(…) by almost 40%, but Array.prototype[Symbol.iterator] = function(){ const iterator = Apply(ArrayPrototypeValues, this, []); iterator.next = iterator.next.bind(iterator); return SetPrototypeOf(iterator, IteratorPrototype); } drops performance by less than 20% as noted above).
could you share the benchmark code please?
I was using esbench.sh with arr = Array(10_000).fill(10_000).map((_, i) => i) in the setup and %Array.prototype% / %ArrayIteratorPrototype% manipulation in the init.
For the current baseline, the snippet manually called iterator = arr[IteratorSymbol]() and proto = GetPrototypeOf(GetPrototypeOf(iterator)) (to account for cost that would go away if this change were adopted) and then result = { iterator, proto, reduced: Iterator.from(iterator).reduce((_, x) => x) }.
For the proposed change and the above bind-based variant, result = { iterator: undefined, proto: undefined, reduced: Iterator.from(arr).reduce((_, x) => x) }.
Look, in my book fast only matters for code that is semantically correct. If people can't use an API because it's dangerous and unpredictable, it isn't all that important the precise speed of it.
You're pointing out that if the instanceof check inside Iterator.from fails, the current spec creates a wrapping iterator which has per-step runtime cost because it wraps a function around next.
I'm saying we could keep all the perf benefits while eliminating the pitfalls.
Why isn't this just fine?
let { map, filter } = Iterator;
let iter = [1, 2, 3][Symbol.iterator]();
iter = map(x => x, iter);
iter = filter(_ => _, iter);
This doesn't really need to wrap anything, at all, ever.
The reason you might still want to do some wrapping would be purely as a perf optimization for the implementation of map which would run most smoothly if its internal iter.next was monomorphic.
.map() is plenty safe and predictable. The only way it can screw up is if you deliberately write an iterator class that goes out of its way to do the wrong thing, which is difficult to do! And if this is the fear, then every class can do the same thing with every method. Anything that relies on any class-internal behavior can be screwed with if you choose to screw with it. Don't do that, and don't run code from others that does that; it's as simple as that.
This is an odd tunnel-visioning on one particular method that is not in any way remarkable in this regard.
Iterator is a pattern that abstracts over storage. That's what it does: it lets you provide data without revealing any details of how you stored it. At an API boundary it lets you provide data without revealing any details of how you stored it.
Let me see if I can say this again: the iterator pattern is used at API boundaries to abstract over how data is stored.
The iterator pattern abstracts over the particular means of data storage. At API boundaries.
So repeat after me: what is remarkable about the next() method? It is used at API boundaries.
Why is it useful to use just a next method at API boundaries? It offers a clear contract that does not expose anything about how data is stored.
I would like to say that phrase a few more times. A clear contract. A clear contract. It offers a clear contract. The clear and unambiguous contract is that you return items in order from the next method. That the contract is clear and unambiguous is what makes it so incredibly useful for coordinating in low-trust situations. It isn't subject to interpretation. There is a single source of truth.
You are asserting that iterators are used in high-trust situations and that it doesn't matter at all if they have a single source of truth or a clear contract. In asserting that you are asserting that iterators should NEVER be used at trust boundaries, and if you think that I can't even imagine what you think iterators are for
Let me see if I can summarize what I would add to the MDN docs:
You must never use Iterator.from(iter) if you do not trust the code which constructed iter. If you do not trust the code that constructed iter, for example because an iterator was provided to you by a plugin, the safe way to construct a proper iterator from such an iterator is Iterator.map.prototype.call(iter, _ => _).
The reason Iterator.map.prototype.call(iter, _ => _) is safe and Iterator.from(iter) is not is that Iterator.from may choose to return the original iterator while Iterator.prototype.map may not. If the original iterator has defined an iter.map function, Iterator.from will use it in place of using the iteration protocol. This causes a breakdown in single source of truth: a consumer using Iterator.from may appear to get different data depending on how it uses the data afterwards.
A consumer using Iterator.map.prototype.call(iter, _ => _) is guaranteed to be defensively safe, and always represents the same underlying data no matter how it is consumed, which is to say no matter which calls like .filter or .flatMap are chained onto the expression next.
You are asserting that iterators are used in high-trust situations and that it doesn't matter at all if they have a single source of truth or a clear contract. In asserting that you are asserting that iterators should NEVER be used at trust boundaries, and if you think that I can't even imagine what you think iterators are for
That is not at all what I'm asserting, and this appears to be the source of your disconnect from what everyone else is saying.
What I (and Jordan, and bakkot) are saying is that iterators do a simple, obvious thing, and if you are working with code that fucks that up for some reason and does something bizarre, you're working with broken code. It's not easy to do that, you have to be purposeful to screw things up. (You need to subclass Iterator and override the map() method with something broken.) It's normal to depend on library code to provide you with good iterators, and just .map() them, or .filter() them, etc; it's not normal to distrust every iterator you're handed and defensively re-wrap them in a "plain" Iterator so that you're 100% certain of what .map() does.
And this point is repeated across every interface. If you have reason to believe the library you're using does broken things, don't use that library!
If you are very specifically writing code for a no-trust environment, that's a different story, of course, and there's lots of special, often unergonomic things you have to do in such a situation to make things "safe". But normally, you trust code to obey advertised API contracts, and don't defensively re-verify them constantly.
Alternately, it's possible that the entire thing is the result of you expecting Iterator.from() to have a particular meaning (always generating a fresh "plain" Iterator), rather than what it actually does (guarantees that what it returns is an Iterator or subclass, wrapping in a "fresh" one if it's passed something that's not an Iterator), and then trying to justify why your original understanding must be the correct one. Dunno, this thread has been weird.
Should it or should it not be safe to assume that the result of evaluating this expression is Iterator.prototype.map?
The WHOLE advertised point of this method is to facilitate exactly this usage. So why is the reality that it may do that and it may do something completely different.
Nobody has yet defended this behavior as safe or correct.