Pre-Stage-3 reviews

[js-choi]

As per the TC39 process document.

• [x] @ljharb
• [x] @nicolo-ribaudo

Editors:

• [ ] @syg
• [ ] @michaelficarra
• [ ] @bakkot

[nicolo-ribaudo]

I opened https://github.com/tc39/proposal-array-from-async/issues/19 with an observation/idea, but apart from that the spec looks good. :heavy_check_mark:

There is a minor editorial bug: the Abstract Closure should capture thisArg.

[js-choi]

Since #20 has been merged, I have marked your review as completed, @nicolo-ribaudo. Thank you again.

[ljharb]

For my review:

• I'd love to see the %Array.prototype.values% simplification upstreamed to Array.from in 262 as a PR, both as an improvement and also to minimize from/fromAsync diffs
• after the spec PR lands, it'd be great to figure out if there's a way to use one or more common AOs to maximally share steps between from and fromAsync

Otherwise, LGTM.

[js-choi]

@nicolo-ribaudo, @ljharb: The spec has had significant changes due to the reversion of #20 in #27, so that its behavior would better match for await and AsyncIterator.prototype.toArray (see https://github.com/tc39/proposal-array-from-async/issues/19#issuecomment-1179810964). Hopefully we’ve got everything set now.

I’d like to request another review of the proposal. I would like to try advancing this proposal to Stage 3 at the 2022-09 plenary meeting. Thank you again; please let me know here if you may not be able to adequately review by 2022-09-01. Thank you sincerely for your time.

[js-choi]

@ljharb, @nicolo-ribaudo: This is another ping before next week’s plenary meeting to re-review the proposal spec within the next few days. Let us know once you re-review—or if you anticipate that you won’t be able to properly review it by then. Thank you!

[nicolo-ribaudo]

I will do this on Friday, sorry for being late.

[ljharb]

Spec seems good to me.

[nicolo-ribaudo]

I will do this on Friday, sorry for being late.

I forgot to reply before plenary, but I re-reviewed the proposal spec and it still looks good.

(Also, I like how you extended AsyncBlockStart because I need the same for another proposal!)

[js-choi]

We got Stage 3, conditional on editor approval.

@syg, @michaelficarra, @bakkot: Please let us know when you have been able to review the spec.

(The typo that @waldemarhorwat found during the plenary has already been fixed.)

[mgaudet]

This is already shipping in Safari Technical Preview, despite being only 'conditionally' stage 3. Firefox has an implementation and is ready to start shipping to nightly when this is officially stage 3.

Any updates on editorial review @syg, @bakkot or @michaelficarra ?

[michaelficarra]

@mgaudet It'd be easiest for me to do a proper review if the proposal was opened as a PR against ecma262.

We should land https://github.com/tc39/ecma262/pull/2942 and https://github.com/tc39/ecma262/pull/3021 first, and we should deduplicate the async-to-sync fallback behaviour that's present in both GetIterator and fromAsync.

It looks like we construct the this value more than once when the parameter is an array-like. This seems like a mistake. I think steps 3.e/3.f are meant to be nested under step 3.j. This is a normative change and committee should be notified. edit: Looks like you already noticed this as well in #35.

Step 3.k.viii seems unnecessary.

In 3.k.vii.4.b, we should be using set, not let. Ecmarkup linting should've caught this. Maybe you need to upgrade or enable the strict linting flag? If you open a PR, the CI will also run these checks. edit: #40

[mgaudet]

I'm not the author of this proposal -- just trying to get this into a state where Firefox can ship our implementation.

[js-choi]

My apologies for the radio silence; I’ve been swamped by work since last winter. I’ve been talking with @michaelficarra privately about next steps:

• I will merge #40 soon, which is just an editorial change.
• We will discuss #35 as an item at the plenary next week. I am planning on attending.
• I’ll try to make a pull request based on tc39/ecma262#2942 sometime in the next four weeks, so that @michaelficarra can give a proper Stage-3 review.

Thanks everyone for your patience.

[js-choi]

40 has been merged. #35 has also been merged, as per the plenary consensus on 2023-05.

As far as I know, our next steps for actual Stage 3 are:

• [ ] Resolve #33.
• [ ] Resolve #43.
• [ ] Maybe resolve #23?
• [ ] Write a pull request against tc39/ecma262#2942 for @michaelficarra and the other editors to formally review. (I am uncertain whether this pull request should be against tc39/ecma262’s main branch or the pull request’s branch.)