Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Priority list of operators #106

Open
keithamus opened this issue Jan 30, 2024 · 7 comments
Open

Priority list of operators #106

keithamus opened this issue Jan 30, 2024 · 7 comments

Comments

@keithamus
Copy link
Collaborator

I'm curious (and can't seem to find evidence) of the decision behind providing the initial operators. On one hand iterator helpers offer a great list to match; symmetry is great! On the other, perhaps some of these APIs are less useful for Observables, and perhaps some result in awkward APIs which have issues (#69).

Given Observable is modeled from user-land libraries such as rxjs, I presume we have some fairly high quality empirical data on what kind of operators users feature in their code. Are we able to qualify that the iterator helpers list overlaps with this data? Are there any outliers that are excluded, or any in the list which are underused?

@keithamus
Copy link
Collaborator Author

keithamus commented Jan 31, 2024

One potential source of data is a poll @benlesh produced back in September 2023:

In a (probably poorly conducted) poll of ~800 respondents on what RxJS operators they use, The top twenty are:

Operator Percentage (%)
map 83.5443038
switchMap 71.39240506
filter 69.36708861
combineLatest 66.32911392
tap 64.30379747
takeUntil 58.48101266
catchError 49.11392405
debounceTime 41.01265823
mergeMap 38.48101266
take 38.35443038
forkJoin 33.41772152
concatMap 28.10126582
merge 23.29113924
first 21.64556962
concat 17.21518987
firstValueFrom 16.32911392
finalize 15.56962025
lastValueFrom 13.67088608
scan 12.40506329

(Aside: this list is 19, not 20).

Currently this proposal wishes to maintain symmetry with the iterator helpers proposal, while also adding a couple of additional operators (finally,takeUntil). So producing a table that maps to the above popular methods to the methods proposed in this spec, we have a table like this:

Observable operator In iterator helpers? RxJS Operator Percentage (%)
map map 83.5443038
#52 ?? switchMap 71.39240506
filter filter 69.36708861
?? combineLatest 66.32911392
forEach? tap 64.30379747
takeUntil takeUntil 58.48101266
?? catchError 49.11392405
?? debounceTime 41.01265823
flatMap mergeMap 38.48101266
take take 38.35443038
?? forkJoin 33.41772152
?? concatMap 28.10126582
?? merge 23.29113924
?? first 21.64556962
?? concat 17.21518987
first firstValueFrom 16.32911392
finally finalize 15.56962025
?? lastValueFrom 13.67088608
?? scan 12.40506329
reduce reduce (less than 12.4%)
some N/A? N/A?
every every (less than 12.4%)
find find (less than 12.4%)
drop skip (less than 12.4%)

I'm sure @benlesh or @domfarolino could help fill this table in where I've maybe got some things wrong here. However, some observations:

  1. Iterator helpers tends to have generally good overlap with the popular operators from this survey, but it also implements a lot of methods that aren't necessarily as popular.
  2. The some seems to not exist in RxJS - perhaps I can be corrected on this.
  3. takeUntil is not in the iterator helpers, but this definitely helps demonstrate its place in this proposal.
  4. first is also not in the iterator helpers, and is equivalent to RxJs's firstValueFrom, not first.
  5. There are many holes in this table. 11 of the top 19 aren't to be implemented.
  6. Of all of the most popular RxJS operators, only firstValueFrom and lastValueFrom convert an Observable into a promise, the remaining 17 operators convert an 1-n Observables to a new Observable.
  7. 6 of the intended iterator helper methods return a Promise.
  8. RxJS' reduce, every, and find all return Observables, however the proposed reduce, every, & find in this proposal return Promise.

@domfarolino
Copy link
Collaborator

Just for posterity, it seems like much of this discussion is happening over on #126 which seems to be pointed at a more hardened list of initial operators to launch with. So maybe this issue can specifically be reserved for follow-on operators that we can consider in the fullness of time / later?

@tabatkins
Copy link

I do think it's might be helpful to add a third value to the "in iterator helpers" column which indicates the ability is fundamentally irrelevant to iterators and thus naturally wouldn't be part of that proposal. (This would cover all the *Map functions, for example; iterator helpers, being sync, only needs a single flatMap, while an async API introduces the possible variants.)


Separately, I agree with Ben Lesh's comment (somewhere in this repo...) that the natural/safe analogue of sync flatMap is async concatMap, not mergeMap. mergeMap introduces footguns that users need to be very wary of; concatMap just "acts like flatMap straightforwardly. The table should be updated to list that as the analogue instead.

@domenic
Copy link
Collaborator

domenic commented Mar 7, 2024

Note that observables are attempting to align not just with sync iterator helpers, but also async iterator helpers. If there are things about async that mean the combinators should be different from sync, then that's good feedback for the async iterator helpers proposal too.

@tabatkins
Copy link

Ah, right, it's been a bit since I saw the async-iterator-helpers proposal. Looks like this exact topic is called out as the reason it was split from sync iterator helpers, so it could be resolved without slowing down sync stuff. (tc39/proposal-async-iterator-helpers#2, plus a few other issues addressing parts of this)

@bakkot
Copy link
Contributor

bakkot commented Mar 7, 2024

The main relevant difference between observables and iterators is that it is much more natural to only care about the latest result when working with observables. A lot of the operators in the above table reflect that: at least switchMap, combineLatest, debounceTime , forkJoin, and lastValueFrom.

I think it's reasonable for observables and iterators to support slightly different sets of operators insofar as it is driven by that distinction.


Separately, I agree with Ben Lesh's comment (somewhere in this repo...) that the natural/safe analogue of sync flatMap is async concatMap, not mergeMap. mergeMap introduces footguns that users need to be very wary of; concatMap just "acts like flatMap straightforwardly. The table should be updated to list that as the analogue instead.

The flatMap in async iterator helpers will indeed be analogous to concatMap, not mergeMap; the table is wrong.


Looks like this exact topic is called out as the reason it was split from sync iterator helpers, so it could be resolved without slowing down sync stuff.

Eh... the way I would put it is that we wanted to design async iterators so they could support consumer-driven concurrency, rather than that we specifically wanted to investigate having additional methods. I think it's likely we'll end up with at least one additional method (buffered), and maybe a couple more, but the core question is whether the other, preexisting operators support being pulled from concurrently. Which is a question which isn't relevant to observables because observables are push rather than pull.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 24, 2024
This CL implements the following Observable operators:
 - some()
 - every()
 - find()

Their inclusion is to be consistent with Iterator helpers [1] and Async
iterator helpers [2], and is discussed in
WICG/observable#126 and to a lesser extent
WICG/observable#106.

See WICG/observable#137 for the specification.

[1]: https://github.com/tc39/proposal-iterator-helpers
[2]: https://github.com/tc39/proposal-async-iterator-helpers

For WPTs:
Co-authored-by: [email protected]

Bug: 40282760
Change-Id: I0fcbb9d716d6ef1727b050fd82a6fa20f53dea4b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 29, 2024
This CL implements the following Observable operators:
 - some()
 - every()
 - find()

Their inclusion is to be consistent with Iterator helpers [1] and Async
iterator helpers [2], and is discussed in
WICG/observable#126 and to a lesser extent
WICG/observable#106.

See WICG/observable#137 for the specification.

[1]: https://github.com/tc39/proposal-iterator-helpers
[2]: https://github.com/tc39/proposal-async-iterator-helpers

For WPTs:
Co-authored-by: [email protected]

Bug: 40282760
Change-Id: I0fcbb9d716d6ef1727b050fd82a6fa20f53dea4b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 29, 2024
This CL implements the following Observable operators:
 - some()
 - every()
 - find()

Their inclusion is to be consistent with Iterator helpers [1] and Async
iterator helpers [2], and is discussed in
WICG/observable#126 and to a lesser extent
WICG/observable#106.

See WICG/observable#137 for the specification.

[1]: https://github.com/tc39/proposal-iterator-helpers
[2]: https://github.com/tc39/proposal-async-iterator-helpers

For WPTs:
Co-authored-by: [email protected]

Bug: 40282760
Change-Id: I0fcbb9d716d6ef1727b050fd82a6fa20f53dea4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5426595
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Dominic Farolino <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1293776}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 29, 2024
This CL implements the following Observable operators:
 - some()
 - every()
 - find()

Their inclusion is to be consistent with Iterator helpers [1] and Async
iterator helpers [2], and is discussed in
WICG/observable#126 and to a lesser extent
WICG/observable#106.

See WICG/observable#137 for the specification.

[1]: https://github.com/tc39/proposal-iterator-helpers
[2]: https://github.com/tc39/proposal-async-iterator-helpers

For WPTs:
Co-authored-by: [email protected]

Bug: 40282760
Change-Id: I0fcbb9d716d6ef1727b050fd82a6fa20f53dea4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5426595
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Dominic Farolino <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1293776}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 2, 2024
…d()` Observable operators, a=testonly

Automatic update from web-platform-tests
DOM: Implement `some()`, `every()`, `find()` Observable operators

This CL implements the following Observable operators:
 - some()
 - every()
 - find()

Their inclusion is to be consistent with Iterator helpers [1] and Async
iterator helpers [2], and is discussed in
WICG/observable#126 and to a lesser extent
WICG/observable#106.

See WICG/observable#137 for the specification.

[1]: https://github.com/tc39/proposal-iterator-helpers
[2]: https://github.com/tc39/proposal-async-iterator-helpers

For WPTs:
Co-authored-by: [email protected]

Bug: 40282760
Change-Id: I0fcbb9d716d6ef1727b050fd82a6fa20f53dea4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5426595
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Dominic Farolino <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1293776}

--

wpt-commits: 8c6b479fae23badbb86e9c993f5b5fbb66e16905
wpt-pr: 45875
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue May 2, 2024
…d()` Observable operators, a=testonly

Automatic update from web-platform-tests
DOM: Implement `some()`, `every()`, `find()` Observable operators

This CL implements the following Observable operators:
 - some()
 - every()
 - find()

Their inclusion is to be consistent with Iterator helpers [1] and Async
iterator helpers [2], and is discussed in
WICG/observable#126 and to a lesser extent
WICG/observable#106.

See WICG/observable#137 for the specification.

[1]: https://github.com/tc39/proposal-iterator-helpers
[2]: https://github.com/tc39/proposal-async-iterator-helpers

For WPTs:
Co-authored-by: [email protected]

Bug: 40282760
Change-Id: I0fcbb9d716d6ef1727b050fd82a6fa20f53dea4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5426595
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Dominic Farolino <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1293776}

--

wpt-commits: 8c6b479fae23badbb86e9c993f5b5fbb66e16905
wpt-pr: 45875
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jun 1, 2024
…d()` Observable operators, a=testonly

Automatic update from web-platform-tests
DOM: Implement `some()`, `every()`, `find()` Observable operators

This CL implements the following Observable operators:
 - some()
 - every()
 - find()

Their inclusion is to be consistent with Iterator helpers [1] and Async
iterator helpers [2], and is discussed in
WICG/observable#126 and to a lesser extent
WICG/observable#106.

See WICG/observable#137 for the specification.

[1]: https://github.com/tc39/proposal-iterator-helpers
[2]: https://github.com/tc39/proposal-async-iterator-helpers

For WPTs:
Co-authored-by: [email protected]

Bug: 40282760
Change-Id: I0fcbb9d716d6ef1727b050fd82a6fa20f53dea4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5426595
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Dominic Farolino <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1293776}

--

wpt-commits: 8c6b479fae23badbb86e9c993f5b5fbb66e16905
wpt-pr: 45875
@errorx666
Copy link

I'm surprised to not see my favorite operator, distinctUntilChanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants