WICG / observable

Observable API proposal
https://wicg.github.io/observable/
Other
543 stars 12 forks source link

Priority list of operators #106

Open keithamus opened 5 months ago

keithamus commented 5 months ago

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 commented 5 months ago

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 commented 4 months ago

Just for posterity, it seems like much of this discussion is happening over on https://github.com/WICG/observable/issues/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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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. (https://github.com/tc39/proposal-async-iterator-helpers/issues/2, plus a few other issues addressing parts of this)

bakkot commented 4 months ago

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.