Spec `from()` static converter #160

keithamus · 2024-07-21T22:39:22Z

This fixes #159 by specifying the semantics of from(), while also then using that algorithm in takeUntil().

domfarolino · 2024-07-22T02:23:39Z

Thanks so much for adding this! It's been on my plate but alas I've not gotten to it yet. One thing is that we won't be able to land this without WPTs, so if you feel like writing those that would be great, otherwise I'm happy to do so, just FYI I am OOO for a bit coming up soon.

petamoriken · 2024-08-02T17:36:54Z

The ReadableStream.from spec is in the process of being changed to use the newly introduced async iterable WebIDL type, so Observable.from should be similarly specified.
whatwg/streams#1310

domfarolino · 2024-08-29T02:43:00Z

I've started to revamp this a little bit to make sure the really subtle semantics of [Symbol.iterator] method access and how we deal with various values of that method's record match (a) the existing tests as well as (b) a bunch of new ones I'm adding in https://chromium-review.googlesource.com/c/chromium/src/+/5824955.

benlesh

Most of this looks okay, just a couple of comments.

We're also missing ArrayLike. Which is supported in Array.from() et al.

ArrayLike should handle any object with a length and numeric keys/indices.

It's probably not a big deal, and we can add it later, I suppose, but I think that Iterator.from() handles it? (I'll have to double check)... Array.from() definitely does.

benlesh · 2024-08-29T18:55:08Z

spec.bs

+    1. <i id=from-observable-conversion><b>From Observable</b></i>: If |value|'s [=specific type=]
+       is an {{Observable}}, then return |value|.
+
+    1. <i id=from-async-iterable-conversion><b>From async iterator</b></i>: Let


It's important in here to make sure that we add a check and call to the async iterator's return() method, if it has one. Because if it's a generator, I'd expect gen.return() to be called so it hits the finally block:

function sleep(ms) { return new Promise(resolve => setTimeout(resolve, ms)); } async function* infiniteGenerator() { let n = 0; try { while (true) { await sleep(100); yield n++; } } finally { console.log('this must be called!'); } } const ac = new AbortController(); Observable.from(infiniteGenerator()) .subscribe(console.log, { signal: ac.signal }); setTimeout(() => { ac.abort(); // should cause log of "this must be called!" }, 333);

benlesh · 2024-08-29T18:57:57Z

spec.bs

+    1. If |iteratorMethodRecord|'s \[[Value]] is undefined or null, then jump to the step labeled <a
+       href=#from-promise-conversion>From Promise</a>.
+
+    1. If [$IsCallable$](|iteratorMethodRecord|'s \[[Value]]) is false, then [=exception/throw=] a


Similar to what I said above about async generators. This has to accommodate generators.

function* infiniteGenerator() { let n = 0; try { while (true) { yield n++; } } finally { console.log('this must be logged!'); } } const ac = new AbortController(); Observable.from(infiniteGenerator()) .subscribe((n) => { console.log(n); if (n === 4) ac.abort(); // Should force the logging of "this must be logged!" }, { signal: ac.signal })

domfarolino · 2024-08-29T19:02:45Z

@benlesh Can you make a WPT PR with tests for the cases you're describing please?

benlesh · 2024-08-29T19:16:05Z

@domfarolino :) It was in-process while you were typing that: web-platform-tests/wpt#47874

See WICG/observable#160, which specs the `Observable#from()` semantics, matching these tests. See also https://crbug.com/363015168 which describes the ways in which our current implementation of `Observable#from()`'s detection semantics are overbroad. This CL makes the implementation of the "detection semantics" match the desired behavior outlined in that issue, and adds a bunch of tests. [email protected] Bug: 363015168, 40282760 Change-Id: Id6cfdd45c44286b298e107635e4283b018f50aaf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5824955 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Mason Freed <[email protected]> Cr-Commit-Position: refs/heads/main@{#1349019}

domfarolino

Quick review just to document the current state of things. I think we're just about ready modulo the async iterator stuff, which I think needs a little work I've outlined below.

spec.bs

…rator] semantics (1/2), a=testonly Automatic update from web-platform-tests DOM: Fix `Observable#from()` [Symbol.iterator] semantics (1/2) See WICG/observable#160, which specs the `Observable#from()` semantics, matching these tests. See also https://crbug.com/363015168 which describes the ways in which our current implementation of `Observable#from()`'s detection semantics are overbroad. This CL makes the implementation of the "detection semantics" match the desired behavior outlined in that issue, and adds a bunch of tests. [email protected] Bug: 363015168, 40282760 Change-Id: Id6cfdd45c44286b298e107635e4283b018f50aaf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5824955 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Mason Freed <[email protected]> Cr-Commit-Position: refs/heads/main@{#1349019} -- wpt-commits: e74098ed10aa0a4fc6ad6b8d6e354895addd00d1 wpt-pr: 47881

domfarolino

A few more things I've noted. I'll work on these and try and fix most of them today.

spec.bs

…rator] semantics (1/2), a=testonly Automatic update from web-platform-tests DOM: Fix `Observable#from()` [Symbol.iterator] semantics (1/2) See WICG/observable#160, which specs the `Observable#from()` semantics, matching these tests. See also https://crbug.com/363015168 which describes the ways in which our current implementation of `Observable#from()`'s detection semantics are overbroad. This CL makes the implementation of the "detection semantics" match the desired behavior outlined in that issue, and adds a bunch of tests. R=masonfchromium.org Bug: 363015168, 40282760 Change-Id: Id6cfdd45c44286b298e107635e4283b018f50aaf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5824955 Commit-Queue: Dominic Farolino <domchromium.org> Reviewed-by: Mason Freed <masonfchromium.org> Cr-Commit-Position: refs/heads/main{#1349019} -- wpt-commits: e74098ed10aa0a4fc6ad6b8d6e354895addd00d1 wpt-pr: 47881 UltraBlame original commit: ef2aac8ce3d8e023f93a55c7593551be16b64d72

…rator] semantics (1/2), a=testonly Automatic update from web-platform-tests DOM: Fix `Observable#from()` [Symbol.iterator] semantics (1/2) See WICG/observable#160, which specs the `Observable#from()` semantics, matching these tests. See also https://crbug.com/363015168 which describes the ways in which our current implementation of `Observable#from()`'s detection semantics are overbroad. This CL makes the implementation of the "detection semantics" match the desired behavior outlined in that issue, and adds a bunch of tests. [email protected] Bug: 363015168, 40282760 Change-Id: Id6cfdd45c44286b298e107635e4283b018f50aaf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5824955 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Mason Freed <[email protected]> Cr-Commit-Position: refs/heads/main@{#1349019} -- wpt-commits: e74098ed10aa0a4fc6ad6b8d6e354895addd00d1 wpt-pr: 47881

This CL implements async iterator support for Observables, and adds tons of WPTs exercising subtle functionality of iterable and async iterable Observable conversion semantics. It implements the spec text in WICG/observable#160, and is a follow-up to https://crrev.com/c/5840672, which brings async iterable support to core bindings code. This CL amounts to a partial implementation of async iterable support; what's missing and what will be included as a follow-up is: 1. Support for calling the Async Iterator's `return()` method [1] when an Observable — when consuming an async iterable — aborts its subscription before iterable exhaustion. 3. A possible refactor to move some of the logic that handles the `ScriptIterator` into `ScriptIterator` itself, per discussion in [2]. [1]: https://tc39.es/ecma262/#sec-asynciterator-interface [2]: https://chromium-review.googlesource.com/c/chromium/src/+/5840672/comment/72df95a9_dd32d801/ [email protected] Bug: 40282760, 363015168 Change-Id: I5f31f4028613245025de71b8975fc92e9d1def0a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5850509 Reviewed-by: Mason Freed <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1356228}

The IteratorRecord#return() function exists as an optional method that sync and async iterator records can supply [1] [2]. They allow for the language, or any consumer of an iterable, to signal to the iterable that the consumer will stop consuming values prematurely (i.e., before exhaustion). This method must be invoked when the consumer aborts its subscription to an Observable that was derived from an iterable. The abort reason is supplied to the `return()` iterator function for completeness. This CL: 1. Adds tests for sync & async iterables 2. Implements this for async iterables A follow-up CL will implement this for sync iterables. The semantics are specified in WICG/observable#160. [1]: https://tc39.es/ecma262/#table-iterator-interface-optional-properties [2]: https://tc39.es/ecma262/#table-async-iterator-optional Bug: 40282760 Change-Id: Ie1091b24b233afecdec572feadc129bcc8a2d4d3

The IteratorRecord#return() function exists as an optional method that sync and async iterator records can supply [1] [2]. They allow for the language, or any consumer of an iterable, to signal to the iterable that the consumer will stop consuming values prematurely (i.e., before exhaustion). This method must be invoked when the consumer aborts its subscription to an Observable that was derived from an iterable. The abort reason is supplied to the `return()` iterator function for completeness. This CL: 1. Adds tests for sync & async iterables 2. Implements this for async iterables A follow-up CL will implement this for sync iterables. The semantics are specified in WICG/observable#160. [1]: https://tc39.es/ecma262/#table-iterator-interface-optional-properties [2]: https://tc39.es/ecma262/#table-async-iterator-optional Bug: 40282760 Change-Id: Ie1091b24b233afecdec572feadc129bcc8a2d4d3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5854985 Reviewed-by: Mason Freed <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Nate Chapin <[email protected]> Cr-Commit-Position: refs/heads/main@{#1359083}

… support for Observables, a=testonly Automatic update from web-platform-tests DOM: Implement async iterable conversion support for Observables This CL implements async iterator support for Observables, and adds tons of WPTs exercising subtle functionality of iterable and async iterable Observable conversion semantics. It implements the spec text in WICG/observable#160, and is a follow-up to https://crrev.com/c/5840672, which brings async iterable support to core bindings code. This CL amounts to a partial implementation of async iterable support; what's missing and what will be included as a follow-up is: 1. Support for calling the Async Iterator's `return()` method [1] when an Observable — when consuming an async iterable — aborts its subscription before iterable exhaustion. 3. A possible refactor to move some of the logic that handles the `ScriptIterator` into `ScriptIterator` itself, per discussion in [2]. [1]: https://tc39.es/ecma262/#sec-asynciterator-interface [2]: https://chromium-review.googlesource.com/c/chromium/src/+/5840672/comment/72df95a9_dd32d801/ [email protected] Bug: 40282760, 363015168 Change-Id: I5f31f4028613245025de71b8975fc92e9d1def0a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5850509 Reviewed-by: Mason Freed <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1356228} -- wpt-commits: 4b27fc79b6ce4edaf1d6a8c0072bd71fce62a6b2 wpt-pr: 48214

…Observables, a=testonly Automatic update from web-platform-tests DOM: Implement abortable async iterable Observables The IteratorRecord#return() function exists as an optional method that sync and async iterator records can supply [1] [2]. They allow for the language, or any consumer of an iterable, to signal to the iterable that the consumer will stop consuming values prematurely (i.e., before exhaustion). This method must be invoked when the consumer aborts its subscription to an Observable that was derived from an iterable. The abort reason is supplied to the `return()` iterator function for completeness. This CL: 1. Adds tests for sync & async iterables 2. Implements this for async iterables A follow-up CL will implement this for sync iterables. The semantics are specified in WICG/observable#160. [1]: https://tc39.es/ecma262/#table-iterator-interface-optional-properties [2]: https://tc39.es/ecma262/#table-async-iterator-optional Bug: 40282760 Change-Id: Ie1091b24b233afecdec572feadc129bcc8a2d4d3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5854985 Reviewed-by: Mason Freed <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Nate Chapin <[email protected]> Cr-Commit-Position: refs/heads/main@{#1359083} -- wpt-commits: 83154d0455e572de16e84ebee72f56df73d2ceb3 wpt-pr: 48280

… support for Observables, a=testonly Automatic update from web-platform-tests DOM: Implement async iterable conversion support for Observables This CL implements async iterator support for Observables, and adds tons of WPTs exercising subtle functionality of iterable and async iterable Observable conversion semantics. It implements the spec text in WICG/observable#160, and is a follow-up to https://crrev.com/c/5840672, which brings async iterable support to core bindings code. This CL amounts to a partial implementation of async iterable support; what's missing and what will be included as a follow-up is: 1. Support for calling the Async Iterator's `return()` method [1] when an Observable — when consuming an async iterable — aborts its subscription before iterable exhaustion. 3. A possible refactor to move some of the logic that handles the `ScriptIterator` into `ScriptIterator` itself, per discussion in [2]. [1]: https://tc39.es/ecma262/#sec-asynciterator-interface [2]: https://chromium-review.googlesource.com/c/chromium/src/+/5840672/comment/72df95a9_dd32d801/ R=masonfchromium.org Bug: 40282760, 363015168 Change-Id: I5f31f4028613245025de71b8975fc92e9d1def0a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5850509 Reviewed-by: Mason Freed <masonfchromium.org> Commit-Queue: Dominic Farolino <domchromium.org> Cr-Commit-Position: refs/heads/main{#1356228} -- wpt-commits: 4b27fc79b6ce4edaf1d6a8c0072bd71fce62a6b2 wpt-pr: 48214 UltraBlame original commit: 2a63fcd3013866bf2a31144f3dd0fa10b96130ac

…Observables, a=testonly Automatic update from web-platform-tests DOM: Implement abortable async iterable Observables The IteratorRecord#return() function exists as an optional method that sync and async iterator records can supply [1] [2]. They allow for the language, or any consumer of an iterable, to signal to the iterable that the consumer will stop consuming values prematurely (i.e., before exhaustion). This method must be invoked when the consumer aborts its subscription to an Observable that was derived from an iterable. The abort reason is supplied to the `return()` iterator function for completeness. This CL: 1. Adds tests for sync & async iterables 2. Implements this for async iterables A follow-up CL will implement this for sync iterables. The semantics are specified in WICG/observable#160. [1]: https://tc39.es/ecma262/#table-iterator-interface-optional-properties [2]: https://tc39.es/ecma262/#table-async-iterator-optional Bug: 40282760 Change-Id: Ie1091b24b233afecdec572feadc129bcc8a2d4d3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5854985 Reviewed-by: Mason Freed <masonfchromium.org> Commit-Queue: Dominic Farolino <domchromium.org> Reviewed-by: Nate Chapin <japhetchromium.org> Cr-Commit-Position: refs/heads/main{#1359083} -- wpt-commits: 83154d0455e572de16e84ebee72f56df73d2ceb3 wpt-pr: 48280 UltraBlame original commit: 3bc676bcae7a77bead1736e9310110ec2cb53e56