proposal-iterator-helpers
proposal-iterator-helpers copied to clipboard
tc39
should AsyncIterator.protoype.filter have a fast path for sync predicates?
As mentioned by @rbuckton in https://github.com/tc39/proposal-iterator-helpers/issues/117#issuecomment-1238559467.
Also it was questioned whether true/false should be the fast path or truthy/falsey non-thenables. I would only support true/false.
Why only true / false? It can be at least all primitives - someone returns 0 or undefined as something falsy.
Given the choices:
- always await
- await if not
trueorfalse - await if
IsPromise
I would choose always await, it seems to be the least chaotic.
@devsnek IsPromise works only with native promises - but it can be a userland implementation, so it's not an option.
@zloirock Yes we could support other primitives, but it gets more murky. What would the algorithm be? If it's an object, assume it's thenable, otherwise ToBoolean it? It would prevent weird things like putting a then method on String.prototype, but that's probably okay.
The algorithm would be if Type is Object, go the slow path. Seems reasonable to me.
If we allow other primitives it's likely to run into weird timing issues. It's easy to image a predicate function which returns either an object or null, whcih would have an extra tick only in the null case. By contrast a predicate function which returns only true or false would have a consistent number of ticks in both cases.
On the other hand, maybe "number of ticks" is a thing not really worth caring about?
Only in that the desire is to minimize them in a predictable way, which i think “returns an object or null” achieves.
I think in these cases the more important thing is consistent behavior. this await will not be the "slow" part of real world code, the I/O from whatever async api is driving it will be.
.filter optimization is not the most interesting similar case. For example, why we should await the result of AsyncIterator.protoype.forEach sync callback that in many cases will return just undefined?
Nope. All methods from this proposal should be run in serial, .filter too. I mean sync .forEach callback. If the result of the callback is undefined or another primitive, not promise. Similar to true of false for .filter from this issue.
What i mean is, conceptually, forEach is for side effects - filter is supposed to be pure.
Obviously they all need to run in order, but failing to await forEach would cause bugs in otherwise reasonable code - filter would not.
It's not obvious to me how failing to await forEach would cause bugs in the specific case that the function passed to forEach returned undefined. We'd have to wait when it it returned a Promise, of course, but I don't see a problem (except for consistency) with avoiding the await undefined in the case that the function passed to forEach synchronously returns undefined.
...and the same situation with callbacks of almost all methods from this proposal - .map, .every, etc. - they are similar here and the resolution of this issue should be similar for all of them.
@zloirock Yes we could support other primitives, but it gets more murky. What would the algorithm be? If it's an object, assume it's thenable, otherwise ToBoolean it? It would prevent weird things like putting a
thenmethod onString.prototype, but that's probably okay.
Putting a then on String.protoype doesn't turn "foo" into a thennable. The Await algorithm, and Promise resolution in general, only calls .then on Objects, per 27.2.1.3.2 Promise Resolve Functions, Step 8.
@bakkot actually yeah, good point. that suggests they can all have this fast path.
I will point out that having a fast path is dangerously close to "releasing Zalgo". My original question was more to whether the result returned from the callback to filter should be awaited at all. I'm a bit wary about a conditional fast path, but I don't think it quite has the same consequence as the concerns described in the linked article.
@rbuckton Ah, I didn't know about that test. That makes calling ToBoolean on all primitives more reasonable, or at least consistent.
@rbuckton all methods of async iterators from this proposal that accept a callback accept async functions, I don't see any reason to make an exception only for .filter.
Also, to my point about "releasing Zalgo", there is still an await that occurs prior to evaluating the filter callback (awaiting the result of next()), as well as an await prior to being able to observe the result (since the new iterator's next() also returns a Promise), so having a fast path to skip an extra await here isn't likely to be an issue.