-
Notifications
You must be signed in to change notification settings - Fork 15
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
Initial operators/methods to add, and a few to remove #126
Comments
Adding seems fairly reasonable, but I am frustrated by the regressions proposed here by removing useful functionality. |
I also think adding seems reasonable, although I think I lean towards putting As for removals, I was hoping we'd discuss this in a separate issue to keep the discussion of each more streamlined. But I've thought about this more and here are my thoughts. I am kind of torn between:
Basically I'm not sure if (1) is a strong enough concern to override the precedent of (2), and how important it is to stick with the precedent of (2) overall. Async iterator helpers' insistence on providing these operators actually makes me more confident that their inclusion in our proposal is appropriate and less problematic. Especially given our conversion operator One argument for the removal of these operators — largely in the event target use case — is that there is not clear "endpoint" of events, so things like |
FWIW I think at least For And sure, these are all pretty simple and can be implemented in other ways from other operators. But given that JS programers are likely to be familiar with these operators, and that they might reasonably want to reach for them, I would be inclined to keep them.
|
I think it's okay if it's all additive. I wouldn't want to trade anything I've mentioned above for
The tricky thing here is they return single values (as promises in this case, but as observables in RxJS), and most of the time what people want to set up is: "Every time someone enters a region, do X" or "every time someone enters a region do X, while they stay in it, do Y", or "Every time someone presses a key while ALT is held do X". Because of this I rarely see these being used. Instead I'd see things like: // you might want to check if a click-and-drag ever passes over a particular region
draggable.on('dragstart')
.flatMap(() =>
region.on('dragover')
.tap(e => e.preventDefault())
.takeUntil(document.on('dragend'))
.first()
)
.subscribe(() => console.log('dragged over the region'))
// For find, you might want to look for a key pressed while holding alt
input.on('keydown').filter(e => e.altKey).subscribe(e => {
// Do something every time the a key is pressed while ALT is held
console.log(`alt + ` + e.key);
}) I think of all of those mentioned, // For reduce, you might want want to find the maximum height of the cursor during a click-and-drag
movable.on('mousedown')
.flatMap(e => {
return document.on('mousemove')
.takeUntil(document.on('mouseup'))
.scan((maxY, { clientY }) => Math.max(maxY, clientY), e.clientY)
.last()
}) |
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
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
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}
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}
…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
…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
…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
Summary
Adding
Observable switchMap(mapper)
Observable catch(mapper)
Observable finally(callback)
Promise first()
Promise last()
Observable scan(reducer, initialValue?)
Removing
Promise some(predicate)
Promise every(predicate)
Promise find(predicate)
Promise toArray()
Promise reduce(reducer, initialValue?)
Desire to followup later
Observable tap(callback/Observer)
Reasoning: Why deviate from async iterator helpers?
reduce
andtoArray()
don't make sense for events,scan
doesWhile this does deviate from the async iterator helpers proposal, observables target a different set of use cases.
reduce()
andtoArray()
in particular make much more sense in an async iterable, where you might be iterating through an array of items, then performing some async action to get a new array of items; Where it doesn't make much sense to reduce mouse clicks or make an array of move events.To that end,
scan
is a much more common task, where one might want to continuously emit reduced state (ala Redux or React'suseReducer
) as it's changed by events, andscan().last()
is the same asreduce()
. Similarly, in my many years working with observables via RxJS, I've found thattoArray()
is usually adjacent to poorly written code, that does something like take an observable of arrays, flatten it to single values, do something with them, then re-accumulate them as an array. An iterable or simple array methods would likely be a better choice for such a task.catch
andfinally
It may be obvious, but iterators and async iterators simply have a different mechanism for communicating errors or finalization and handling them. Observable needs
catch
andfinally
methods that behave very similarly to the ones that promise provides.catch
has a common use case withflatMap
for keeping source streams alive when child streams error:finally
has use cases around clean up that you want to occur on unsubscription, completion, or error, and you reproduce between subscriptions to the same observable.first
andlast
The ability to get the first or last value from an observable as a promise is an important tool for interoperability with promise-based approaches.
last()
withscan()
allows users to createreduce()
ortoArray()
functionality easily.first()
also allows users to quickly get a promise from the new observable event API that is consumable in an async function:Where the alternative would be:
The need for
switchMap
switchMap
is a method that really takes the most advantage of observable's design, and is commonly the reason developers reach for observables: It has implicit unsubscription when it gets a new value from the source and then maps to a new observable stream it switches to.It can be used for many dynamic use cases that are difficult to compose with out it
Without
switchMap
all of the above scenarios would require doing some sort of weird dance withflatMap
andtakeUntil
in order to get the proper outcome.Why follow up on
tap
laterThe reason to follow up on
tap
later, is simply that you can usemap
to deal withtap
's most common use case, creating a side effect from a value.source.map(x => (sideEffect(x), x))
is the same assource.tap(sideEffect)
.tap
does have many other use cases, and it is a broadly used thing, but if it was down to picking betweentap
and the operators listed above it, we should pick the others.The text was updated successfully, but these errors were encountered: