Adds explaination of flatMap semantics
#86
Conversation
README.md
Outdated
| @@ -507,6 +507,10 @@ We propose the following operators in addition to the `Observable` interface: | |||
| - `finally()` | |||
| - Like `Promise.finally()`, it takes a callback which gets fired after the | |||
| observable completes in any way (`complete()`/`error()`) | |||
| - `flatMap()` | |||
There was a problem hiding this comment.
I don't think this should appear both in this "exceptional" list, and in the list below about iterator helpers. Seems like it should just exist in the list of iterator helpers, with a note that the semantics are a little different (as you provided below).
README.md
Outdated
|
|
||
| - When you subscribe to `result`, it subscribes to `source` immediately. | ||
| - Let there be a `queue` of values that is empty. | ||
| - Let there be an `innerSignal` that is either `undefined` or an `AbortSignal` |
There was a problem hiding this comment.
Which is it? Does this mean to represent the signal that's passed into the subscription of result? (Clarifying that might be good)
There was a problem hiding this comment.
That's correct. I'll add something.
There was a problem hiding this comment.
Actually, I do explain that below: Pass the innerSignal to the subscribe for the inner observable.
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
|
|
||
| `flatMap` generally behaves like `Iterator.prototype.flatMap`, however, since it's push-based, | ||
| there can be a temporal element. Given that, it behaves much like RxJS's `concatMap`, which is | ||
| one of the most useful operators from the library. |
There was a problem hiding this comment.
which is one of the most useful operators from the library.
I think we can remove this.
|
|
||
| ```ts | ||
| const result = source.flatMap((value, index) => | ||
| getNextInnerObservable(value, index), |
There was a problem hiding this comment.
What's the trailing comma here?
There was a problem hiding this comment.
Probably aggressive formatting rules. I'll have a look.
| - When you subscribe to `result`, it subscribes to `source` immediately. | ||
| - Let there be a `queue` of values that is empty. | ||
| - Let there be an integer `current index` that is `0`. | ||
| - Let there be an `innerSignal` that is either `undefined` or an `AbortSignal`. |
There was a problem hiding this comment.
initially....? Where does this come from? It comes from the subscribe() method being called on the Observable returned by flatMap(), right? Maybe we should mention that. After reading the rest I'm actually thinking that innerSignal is not the one passed into subscribe. So... I'm not sure :)
There was a problem hiding this comment.
It's a bit of state for the internal algorithm. If innerSignal is undefined, there's no current inner subscription. If innerSignal is an AbortSignal, we have an inner subscription, and we need to wait for it to complete before we start another. So we need to buffer the value.
Basically:
let innerSignal: AbortSignal | undefined;| - Take the first one from the `queue` and return to the **"mapping step"**. | ||
| - If the `queue` is empty | ||
| - If `isSourceComplete` is `true` | ||
| - Complete `result`. |
There was a problem hiding this comment.
Sometimes you have a period and sometimes you don't. Can you edit this whole algorithm to be consistent one way or another?
There was a problem hiding this comment.
Maybe I can.
Maybe I can't
| - If `isSourceComplete` is `true` | ||
| - Complete `result`. | ||
| - If `isSourceComplete` is `false` | ||
| - Wait |
There was a problem hiding this comment.
Hmm, what is this supposed to do? Certainly we're not blocking the main thread....
There was a problem hiding this comment.
No... Just "wait for another event"... do nothing, I guess.
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
|
A few outstanding comments from a while ago still.. I'll wait for those to get resolved. |
|
Since this operator has already made it into the spec https://wicg.github.io/observable/#dom-observable-flatmap, I think this may not be necessary, and seems to have no activity these days anyways. So I'll close it. |
Basically this is describing RxJS's
concatMap. I tried to add it as detailed as possible because "It works likeIterator.prototype.flatMapis only partially true, since there's the element of asynchrony involved here. If all sources and inner sources are synchronous, though, it is exactly likeIterator.prototype.flatMap.