stage 3 tracking

[michaelficarra]

• [x] all open PRs merged or rejected
• [x] all open issues addressed or closed
• [x] spec text finalised
• [ ] reviewer sign-off
  • [ ] Richard Gibson (@gibson042)
  • [ ] Jordan Harband (@ljharb)
  • [ ] Ron Buckton (@rbuckton)
• [ ] editor sign-off
  • [ ] Kevin Gibbons (@bakkot)
  • [ ] Shu-yu Guo (@syg)
• [x] presentation to committee prepared

[michaelficarra]

Things have gotten really busy for me and I don't think I'll be able to prepare this for stage 3 (at least not on my own) in time for the stage advancement deadline on 10th Sep.

[Jack-Works]

Maybe also wait for tc39/ecma262#2045 to merge

[jorendorff]

I think @avandolder is interested in helping. I can help too.

The fact that tc39/ecma262#2045 is stalled is a major obstacle. If there's anything I can do to help there, let me know! You can ping me on IRC any time.

[Jack-Works]

Hmm cause there's nothing to update with #2045... I didn't get a clue about what should I change to make it merged, or it just waiting for some editor to press the merge button?

[bakkot]

The editors need to review it, which will take a while because it's a substantial change.

[michaelficarra]

@jorendorff That PR is on the top of my list for review. I'll hopefully get to it this week, possibly even today. An additional review from you (even if it just results in an approval) would be helpful.

[mpcsh]

I'm not sure how I missed this, but I will unfortunately not be able to provide the stage 3 review I committed to at the previous plenary. my sincere apologies to the champions and stakeholders. I will be doubly careful to ensure I stay on top of review commitments going forward.

[michaelficarra]

@mpcsh That's fine, we will ask for a new one at the meeting next week: https://github.com/tc39/agendas/commit/868028cbc63ddee53a04ca88cfe07ac154377ed0

[michaelficarra]

Update: @codehag has volunteered (possibly to be replaced in the future by @jorendorff).

[benjamingr]

Hey - are there any updates on this proposal? When do you expect to present it again?

(I am asking because Node.js has started to integrate async iterators in many places and this would be very useful for Node!)

[karlhorky]

@benjamingr out of interest, can you link to an example or two of how Node is using iterators in the codebase?

Also eagerly awaiting this proposal!

[benjamingr]

@karlhorky

• Every stream is async iterable (so you can for await (const chunk of createReadStream('file')), and http requests are async iterable).
• Events are async iterable with on (on(emitter, 'foo') returns an async iterable).
• Intervals are async iterable (soon).
• File system watchers are async iterable soon

But mostly streams and all stream helpers (like pipeline), the easiest way to get a Node.js stream is to use Readable.from with an async generator.

The "new way" of writing Node.js is getting rather pervasive:

• Using promises for APIs (like fs/promises).
• Using async iterators for anything that can be async iterable.
• Using AbortController for cancellation in all APIs.

I came here because I really wanted a setInterval(1000).map(someFn) and we had a discussion about this.

[benjamingr]

And to clarify - I think it's safe to say Node.js is very interested in this proposal and since a bunch of our use cases arose from web platform APIs I am sure a very compelling case can be made for it in browsers too.

[devsnek]

This proposal was originally made by me and Domenic, so I do hope our respective ecosystems find it useful 😅

[Jack-Works]

Though I'm happy to see iterator helpers and usage of for await of but I want to mention it is really easy to get into the performance trap (write serial code when it can be parallel, just like the misuse on await).

for await (const req of connection) {
    // easy to write but actually wrong in some cases
    req.response(await longTimeAsyncIO(req))
    // oops, actually turn handling into one-by-one,
    // and if it fails it actually block all future connections
}

for await (const req of connection) {
    try { longTimeAsyncIO(req).then(req.response.bind(req)) } catch {}
}

[benjamingr]

@Jack-Works I believe the whole point of “for await of” with async iterators is to serialize code so it appears sync yet works with backpressure (not requesting more items from the iterator ahead of time).

Making async code look and behave serially is the major selling point of async/await for me.

It is very easy to just consume and process several bits from the iterator at once (call .next several times without waiting for the first one to fulfill).

Also I suggest we move this discussion elsewhere as it does not relate to this proposal’s status :)

[rektide]

Just mentioning, although not yet stage 3, we have seen some implementation begin (flagged), in Firefox, according to the README.

[benjamingr]

That’s great thanks for the update :)

[michaelficarra]

@gibson042 @ljharb @rbuckton This proposal is ready for review. Note the 3 open issues which will be resolved by the remaining open PRs depending on committee decision when this is presented for stage 3. Please review those PRs in addition to the current spec text.

[michaelficarra]

Reminder @gibson042 @ljharb @rbuckton that this proposal is potentially up for stage 3 at the next meeting. The slides have been posted to the agenda. Please review this proposal ahead of the meeting.

[ljharb]

Spec review:

• spec: needs updating to minimize usage of Type()
• spec: https://tc39.es/proposal-iterator-helpers/#sec-iteratorprototype.filter and https://tc39.es/proposal-iterator-helpers/#sec-asynciteratorprototype.filter - "filterer" is a weird argument name. why not "predicate"?
• spec: https://tc39.es/proposal-iterator-helpers/#sec-iteratorprototype.flatmap and https://tc39.es/proposal-iterator-helpers/#sec-asynciteratorprototype.flatmap - is there nothing that can be shared between these, and the yield * steps (for both sync and async), since all three are "spreading" another iterator?
• spec: https://tc39.es/proposal-iterator-helpers/#sec-iteratorprototype.some and https://tc39.es/proposal-iterator-helpers/#sec-iteratorprototype.every and https://tc39.es/proposal-iterator-helpers/#sec-iteratorprototype.find and https://tc39.es/proposal-iterator-helpers/#sec-asynciteratorprototype.some and https://tc39.es/proposal-iterator-helpers/#sec-asynciteratorprototype.every and https://tc39.es/proposal-iterator-helpers/#sec-asynciteratorprototype.find - can the argument be named "predicate" instead of "fn"?

• 211: normatively unconvinced; seems editorially correct

• 213: editorially correct; normatively, i'd prefer it brand-check

• 214: editorially correct; normatively, simpler than #213 but super inconsistent/weird

• 212: editorially correct; personally i think this should be skipped and deferred to the getIntrinsic proposal.

[rbuckton]

Do we normally have headings as deep as "3.1.2.2.2.1.1"? The formatting for the headers looks off for https://tc39.es/proposal-iterator-helpers/#sec-wrapforvaliditeratorprototype-object and a few others.

[michaelficarra]

@rbuckton The section numbers are correct. It's not terribly unusual. See 14.7.5.10.2.1, 16.2.1.5.1.1, 16.2.1.5.2.1, 16.2.1.5.2.2, 16.2.1.5.2.3, 16.2.1.5.2.4, 16.2.1.5.2.5.

[michaelficarra]

@ljharb

• "filterer" is a weird argument name. why not "predicate"?

• can the argument be named "predicate" instead of "fn"?

Sure, though I'll wait on wait on the resolution to #211 first. In the slides, which assume #211 is merged, I've named them predicateWithCounter.

[rbuckton]

• 3.1.2.2.2 Iterator.from(O)

  • Steps 2.b and 2.c.i - This does an OrdinaryHasInstance check for %Iterator%, but does not check whether the iterator has a next() method, while GetIteratorDirect does perform a check for a valid next method. As a result, its possible to use Iterator.from on a subclass of Iterator that does not define next, but not on a regular object that does not define next. Is that intended?

• 3.1.3.2.2 AsyncIterator.from(O)

  • Step 3.b - calls OrdinaryHasInstance with %AsyncIterator.prototype%, but should use %AsyncIterator% instead.
  • Steps 3.b and 3.c.i - This also does not check that a next method is present.

• 3.1.4.1.1 %IteratorHelperPrototype%.next()

  • Step 1 - Generator brands are supposed to be strings, not spec enums.

• 3.1.4.1.2 %IteratorHelperPrototype%.return()

  • Step 2 - Generator brands are supposed to be strings, not spec enums.

• 3.1.5.1.1 %AsyncIteratorHelperPrototype%.next()

  • Step 1 - Generator brands are supposed to be strings, not spec enums.

• 3.1.5.1.2 %AsyncIteratorHelperPrototype%.return()

  • Step 2 - Generator brands are supposed to be strings, not spec enums.

• 3.1.6.2 Iterator.prototype.map(mapper)

  • mapper is probably fine, though some iteration libraries use the term selector for 1:1 mappings.
  • Step 4 - Generator brands are supposed to be strings, not spec enums.

• 3.1.6.3 Iterator.prototype.filter(filterer)

  • I think either predicate or just callbackfn might be a better name here than filterer. The term predicate is commonly used in many iteration libraries, and callbackfn is the same generic parameer name we use for Array.prototype.filter.
  • Step 4 - Generator brands are supposed to be strings, not spec enums.

• 3.1.6.4 Iterator.prototype.take(limit)

  • Step 3 - Is there a reason not to let ToIntegerOrInfinity handle NaN (returning 0) like we do for Array.prototype.at, Array.prototype.fill, Array.prototype.slice, etc.? This NaN handling decision seems arbitrary and a potential refactoring hazard, i.e., refactoring from array.slice(0, x) to array.values().take(x) could potentially throw a RangeError that wouldn't have occurred previously. The only times we override the NaN handling of ToIntegerOrInfinity in the spec is when the number is used as an index from the end of an array like Array.prototype.lastIndexOf, or in Annex B. Generally when indexing from the left side of an array we treat NaN as 0, and when indexing from the right side of an array we treat NaN as +Infinity (or equivalent).
  • Step 7 - Generator brands are supposed to be strings, not spec enums.

• 3.1.6.5 Iterator.prototype.drop(limit)

  • Step 3 - Same comment re: ToIntegerOrInfinity handling of NaN.
  • Step 7 - Generator brands are supposed to be strings, not spec enums.

• 3.1.6.6 Iterator.prototype.indexed()

  • Step 3 - Generator brands are supposed to be strings, not spec enums.

• 3.1.6.7 Iterator.prototype.flatMap(mapper)

  • mapper is probably fine, though some iteration libraries use the term projection for 1:n mappings.
  • Step 3.a.vi - I'm also surpised this doesn't support user-defined iterators and depends on @@iterator. I assume that for user defined iterators the user will have to use Iterator.from explicitly?
  • Step 4 - Generator brands are supposed to be strings, not spec enums.

• 3.1.6.8 Iterator.prototype.reduce(reducer [, initialValue])

  • Step 3.b - I find it surprising that reduce will throw if there is no initialValue and the iterator is empty, since Array.prototype.reduce does not throw.

• 3.1.7.2 AsyncIterator.prototype.map(mapper)

  • Same comment as 3.1.6.2 re: mapper/selector.
  • Step 4 - Generator brands are supposed to be strings, not spec enums.

• 3.1.7.3 AsyncIterator.prototype.filter(filterer)

  • I think either predicate or just callbackfn might be a better name here than filterer. The term predicate is commonly used in many iteration libraries, and callbackfn is the same generic parameer name we use for Array.prototype.filter.
  • Step 3.a.vi - Just want to verify that the result of the filterer callback is intended to allow promises. It seems potentially wasteful to Await if every filterer ends up only returning true or false (or rather, a non-thenable "truthy" value or "falsy" value).
  • Step 4 - Generator brands are supposed to be strings, not spec enums.

• 3.1.7.4 AsyncIterator.prototype.take(limit)

  • Step 3 - Same comment re: ToIntegerOrInfinity handling of NaN.
  • Step 7 - Generator brands are supposed to be strings, not spec enums.

• 3.1.7.5 AsyncIterator.prototype.drop(limit)

  • Step 3 - Same comment re: ToIntegerOrInfinity handling of NaN.
  • Step 7 - Generator brands are supposed to be strings, not spec enums.

• 3.1.7.6 AsyncIterator.prototype.indexed()

  • Step 3 - Generator brands are supposed to be strings, not spec enums.

• 3.1.7.7 AsyncIterator.prototype.flatMap(mapper)

  • mapper is probably fine, though some iteration libraries use the term projection for 1:n mappings.
  • Step 3.a.vi and viii - We do a lot of awaiting and async iterator wrapping here. I just want to verify that the intended return value from the mapper is something like Promise<AsyncIterable<T> | Iterable<T>> | AsyncIterable<T> | Iterable<T>.
  • Step 3.a.viii - I'm also surpised this doesn't support user-defined iterators and depends on @@iterator/@@asyncIterator. I assume that for user defined iterators the user will have to use AsyncIterator.from explicitly?
  • Step 4 - Generator brands are supposed to be strings, not spec enums.

• 3.1.7.8 AsyncIterator.prototype.reduce(reducer [, initialValue])

  • Steps 1, 2, and 3.b - Since this method is intended to return a Promise, should we not be throwing this error as a promise rejection? This method doesn't feel like it matches the way that other Promise-returning functions are written such as AsyncGenerator.prototype.next and appears to gloss over some of the async operation mechanics. The spec doesn't currently define what behavior a "built-in async function" has.
  • Step 3.b - I find it surprising that reduce will throw if there is no initialValue and the iterator is empty, since Array.prototype.reduce does not throw.

• 3.1.7.9 AsyncIterator.prototype.toArray()

  • This method also seems to gloss over async method semantics as is does not create a PromiseCapability record.
  • Steps 1 and 2 - Shouldn't argument validation be a promise rejection?
  • This also applies to 3.1.7.10, 3.1.7.11, 3.1.7.12, and 3.1.7.13

[rbuckton]

The spec doesn't currently define what behavior a "built-in async function" has.

I'd be fine with the wording of the scalar methods on AsyncIterator.prototype if the proposal spec clearly defined how algorithm steps for a "built-in async function" should work. Just adding an Await isn't sufficient to consider this complete specification text.

Methods like Iterator.prototype.filter get around this hand-wavy-ness by specifying an Abstract Closure and passing it to CreateIteratorFromClosure, which handles all of the iterator scaffolding and explicitly calls out that Yield shorthand is safe to use.

[bakkot]

Re: handling of NaN, see https://github.com/tc39/proposal-iterator-helpers/issues/169. This was also mentioned during the presentation in July.

[bakkot]

Iterator.prototype.reduce Step 3.b - I find it surprising that reduce will throw if there is no initialValue and the iterator is empty, since Array.prototype.reduce does not throw.

[].reduce(x => x) does in fact throw. Am I not understanding the comparison?

AsyncIterator.prototype.flatMap Step 3.a.vi and viii - We do a lot of awaiting and async iterator wrapping here. I just want to verify that the intended return value from the mapper is something like Promise<AsyncIterable | Iterable> | AsyncIterable | Iterable.

Yup. It's gross, but consistent - await expects Promise<T> | T, and for await expects AsyncIterable<T> | Iterable<T>, so if you're iterating the result of a potentially-async function you compose them and get Promise<AsyncIterable<T> | Iterable<T>> | AsyncIterable<T> | Iterable<T>.

AsyncIterator.prototype.reduce Steps 1, 2, and 3.b - Since this method is intended to return a Promise, should we not be throwing this error as a promise rejection?

See https://github.com/tc39/proposal-iterator-helpers/issues/218 for the under-specification issue. I believe the intent of "built-in async function" is that they behave like normal async functions, i.e. all ThrowCompletions are wrapped up in promise rejections.

[michaelficarra]

• This does an OrdinaryHasInstance check for %Iterator%, but does not check whether the iterator has a next() method, while GetIteratorDirect does perform a check for a valid next method. As a result, its possible to use Iterator.from on a subclass of Iterator that does not define next, but not on a regular object that does not define next. Is that intended?

@rbuckton I think you're confused here. The OrdinaryHasInstance test is done in the path that is supporting Iterables. Are you thinking of a case where an object implements Symbol.iterator and returns something that inherits from Iterator.prototype but then also has a non-callable next own-property?

[rbuckton]

Are you thinking of a case where an object implements Symbol.iterator and returns something that inherits from Iterator.prototype but then also has a non-callable next own-property?

Yes, but also any class that subclasses Iterator, since %IteratorPrototype% also has a [Symbol.iterator] method.

class BadIterator extends Iterator {}
const iter = Iterator.from(new BadIterator()); // succeeds
typeof iter.next === "undefined";

const iter2 = Iterator.from({}); // throws TypeError because there's no `next` method.

[rbuckton]

Is the proposal's %Iterator.prototype% replacing %IteratorPrototype%? In https://tc39.es/proposal-iterator-helpers/#sec-well-known-intrinsic-objects it seems to be referred to in both ways, but I can't find any examples of similar aliasing in the main spec.

[rbuckton]

[].reduce(x => x) does in fact throw. Am I not understanding the comparison?

Ah no, I was scanning the steps for reduce and jumped to step 8 and missed the check on step 4.

[michaelficarra]

Is the proposal's %Iterator.prototype% replacing %IteratorPrototype%? In tc39.es/proposal-iterator-helpers/#sec-well-known-intrinsic-objects it seems to be referred to in both ways, but I can't find any examples of similar aliasing in the main spec.

It's just an alternative way to reference it. In the well-known intrinsic objects section, we allow this kind of reference via

A reference such as %name.a.b% means, as if the "b" property of the value of the "a" property of the intrinsic object %name% was accessed prior to any ECMAScript code being evaluated.

[rbuckton]

Yes, but no other entry in the table is structured the way its structured in the proposal. All of the %XPrototype% entries in that table are listed individually because there's no way to directly access them by dotting off of something as you quoted. Having both %IteratorPrototype% and %Iterator.prototype% in the spec seems unnecessarily confusing (as evidenced by the fact I had to ask for clarification), but is something that can be addressed in the actual ecma262 PR.

But this goes to my original point. The algorithm steps in Iterator.from and AsyncIterator.from don't reliably check for next and assume any iterable whose iterator inherits from %Iterator.prototype% (or %AsyncIterator.prototype%) is safe to use when that's not necessarily the case.

If you're ok with that, then that's fine. But I wanted to make sure that was an intended consequence rather than something that was overlooked.

[ljharb]

In the actual PR, we wouldn't list %IteratorPrototype% at all, we'd list %Iterator%, and we'd change any mention of %IteratorPrototype% in 262, 402, or HTML to use %Iterator.prototype%.

[rbuckton]

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.

[michaelficarra]

@gibson042 @ljharb @rbuckton Please do another review. You may want to review each of the recently merged PRs individually, or you can review the spec text as a whole. I think the section most in need of review is the new built-in async functions section (and associated machinery) added in https://github.com/tc39/proposal-iterator-helpers/pull/240. GetIteratorFlattenable could also possibly use another set of eyes.

I plan to ask for stage 3 at the upcoming November meeting.

[michaelficarra]

Note to reviewers about the built-in async functions:

I've opened #245 which removes that infrastructure from this proposal and instead relies on https://github.com/tc39/ecma262/pull/2942 to add it. You can still review that PR if you like, but you should no longer feel obligated to do so. Now you can focus on just the parts that are relevant to this proposal.

[michaelficarra]

Fixed by #251.