tc39 / proposal-observable

Observables for ECMAScript
https://tc39.github.io/proposal-observable/
3.07k stars 90 forks source link

Context around argument to `complete` #87

Closed benlesh closed 8 years ago

benlesh commented 8 years ago

This is not an argument against passing a value to complete... rather I'd like context, because I need to figure out how it can work into RxJS. I've not included it yet in that library, because it's easier to add an argument later than to remove (which would be more breaking).

My questions:

  1. When should this value be used/leveraged by users? What are some real use cases for it?
  2. How could this argument be used in a way that is an anti-pattern? Does this enable anti-patterns?
  3. Combinators: How would this work with common combinators? Always drop? reduce?
    • concat?
    • flatMap?
    • zip?

I guess I'd be looking for some sort of guidance to provide to people about what it's for and when to use it. My gut right now tells me to tell people "Never use it". haha. But you've got it in here for a reason, so that can't be right. (but seriously, never use it :P)

benlesh commented 8 years ago

(This relates to #77, but isn't the same, because it's just to gain context)

benjamingr commented 8 years ago

Giving this a shot to start discussion.

To be fair these examples are made up - I'm not sure how contrived they are, but they feel strange to me, not sure if because I am used to Rx or because inherently they make less sense. Here are some theoretical examples, I am not suggesting any of them for Rx or es-observable:

When should this value be used/leveraged by users? What are some real use cases for it?

  • Doing a toPromise, it would make sense to treat it as a completion value, there was a notion of promise progression (in my opinion misguided, by people who were not familiar enough with Rx) and that would model into an es-observable nicely.
  • Doing something like .reduce or .scan could complete with a value rather than emit a value.
  • Doing something like making an http request - where all the progress notifications
  • Basically, anywhere when an observable models a one time event and there is built up to it. Anything we currently just emit the completion value on next and complete without a value. Like a more expressive less constrained promise.

It is also useful for example in order to build a generator pump - like co does with promises - only with observables, where returning a value from the generator returns it from the observable.

How could this argument be used in a way that is an anti-pattern? Does this enable anti-patterns?

Maybe, maybe not. Personally I doubt it'll be used - like the iterable completion value isn't used. Maybe @jhusain can shed some light here though.

Combinators: How would this work with common combinators? Always drop? reduce?

This is an interesting question, in my experience there is no general solution for this. @kriskowal might be able to shed some light here - about how for arbitrary values you can have composable return value, composable stream values but not both (or at least, we haven't figured it out). This is why promise progress was dropped.

Here are some wild guesses. I have just as much experience with completion values in observables as the next guy. Note that with iterators, since we have IteratorResult we don't really care about completion events - most of the time unlike in Rx where often it is needed (for good reasons).

concat - return a Set of completion values. flatMap - return the last completion value. zip return an array of completion values.

Honestly since we dropped duality as a goal perhaps we should revisit https://github.com/zenparsing/es-observable/issues/77 and consider not composing the completion value. I hope @headinthebox or @kriskowal can shed some light. In particular if @kriskowal or @domenic can find the discussion about promise progression and why it's hard/impossible to have a sequence of notifications and a completion event compose together in a sound way.

benjamingr commented 8 years ago

ping.

zenparsing commented 8 years ago

I have complete accepting an argument for consistency with the iteration model; it should be possible to push the results of an iterator to an observer without loss. Observable.from should be sending along the done value when it converts from iterables, and it's an oversight on my part that it currently does not.

That's the entire rationale, and I believe that it's sufficient.

benjamingr commented 8 years ago

I think that it doesn't work, and I think it doesn't work for iterators either and adding it for iterators was probably a mistake.

I think @kriskowal wrote a post about why a while ago in the whatwg mailing list which I can't find, but promises have tried this before with progression and back tracked.

See https://github.com/petkaantonov/bluebird/issues/91, and also @domenic in this discussion:

I would strongly advise against conflating progress (ongoing notifications about a streaming async operation) and promises (one-and-done async operations). The promise implementer community in general, and the Promises/A+ organization in particular, has found this to be an extremely problematic line of reasoning, and one that does not mesh well with the existing semantics or syntax of promises. It has led to increased confusion from our users and a muddling of the underlying conceptual model, especially e.g. as it pertains to error handling. The fact that any progress additions to the promise API have no place in a shallow coroutine-based syntax enhancement of promises (e.g. C#’s await, or ES6’s yield) further strengthens our view.

And in http://lists.w3.org/Archives/Public/www-dom/2013JulSep/0122.html

All this applies from the other side to observable completion values. It is downright impossible to do this in a sound way and describe how completion values compose the same way iterators do (or the other way around).

Thanks @gaearon for digging this up in the lists in the issue we've had in bluebird by the way.

zenparsing commented 8 years ago

@benjamingr

I'm not sure what you consider the mistake to be. Would you have disallowed the "return" keyword inside of generator functions?

I perfectly fine with the accepted wisdom saying "don't use it", but I think equivalency is important if this is going to be a standard library thing.

benjamingr commented 8 years ago

I'm not sure what you consider the mistake to be. Would you have disallowed the "return" keyword inside of generator functions?

No, I would have allowed it, and for return 5 have emitted two IterationResults, a {done: false, value: 5} and a {done: true, value: undefined}.

That is what other languages practically do.

zenparsing commented 8 years ago

No, I would have allowed it, and for return 5 have emitted two IterationResults

Maybe that's better, or maybe not, but that's not what we have, and I don't see a killer argument for making observable break from that paradigm. Given an iterator that ends with { done: true, value: 5 } would should an observer see, complete(5) or next(5)?

benjamingr commented 8 years ago

The observer should see .next(5) and then complete(), probably. I'm hoping @kriskowal weighs in though.

zenparsing commented 8 years ago

The observer should see .next(5) and then complete(), probably

I agree that makes a lot of sense on a local level, however I believe that it creates a degree of disharmony at a global language level.

benjamingr commented 8 years ago

Only if we're claiming duality. Are we?

benlesh commented 8 years ago

I think there's a slight disconnect here WRT iterables/observables.

Because you have to pull from next() with an iterator, it needs to be able to tell you everything about the state of the iterator at that moment: The next value, whether or not it's done, and of course it can throw an error right then too.

In other words, if you didn't have it throw, IteratorResult very well could have been written like this:

interface AlternateRealityIteratorResult<T> {
  value?: T,
  done: boolean,
  error?: any
}

The whole { value: 'something', done: true } thing is a result of needing to pull a value from an iterator. The "doneness" is unrelated to the value, other than by virtue of the fact they can only be communicated by the same "pull".

With Observables, all of those things can be pushed, asap, through the observer. Which means the observer can tell you as soon as your "done". And you don't need to check a boolean value. It also means you can disconnect the last value from the "doneness" of the thing, since everything is pushed.

Given that both next(value) and complete() can even be triggered in the same job, synchronously, it's really more than adequate for communicating the Iterable equivalent.

benlesh commented 8 years ago

To take it a step further, I realize that iterating over an iterable with for..of or the like generally results in dropping the value provided by return in a generator. I think this was probably a mistake (in my know-nothing opinion) that was implemented to accommodate an idiosyncrasy of generators more so than it was to accommodate iterables/iterators. If generators had a way to exit that didn't return it probably wouldn't have been necessary.

Anyhow, that's almost orthogonal to this discussion. Other than how it relates to Observable consumption.

kriskowal commented 8 years ago

I envision the generator side of an observer having this API:

generator.next(value);
generator.return(value);
generator.throw(error);

The optional value given to return (which I believe you are currently calling complete) does not participate in the stream. That is, return(value) is not equivalent to next(value); complete().

The completion value would in most cases be ignored by composition operators in the same way the return value of an iterator is ignored by a for loop. However, if a for loop were an expression, the return value of the iterator would be appropriate. So with observers, an operator like map() would return a new observer that would forward the completion value unaltered. A reducer would drop the completion value as it has no place. Let me know if you need more specifics.

benlesh commented 8 years ago

@kriskowal But in a generator, passing a value to return(value) is practically useless. All it does is return { done: true, value: 'same thing you just passed' } and if your generator has a finally block, it jumps to it, AFAICT.

benlesh commented 8 years ago

(I'm 1000% willing to admit I probably just don't know the use case for generator.return(value))

kriskowal commented 8 years ago

@blesh A use-case for the done value for iterators was not obvious in the Python community for many years. It turns out to be necessary for the async decorator to work properly. I expect that there is an analogous use for the return value on an observer, if not just the ability to pump an async iterator into an observer and the observer back into an async iterator without losing information.

benlesh commented 8 years ago

It turns out to be necessary for the async decorator to work properly.

I'm interested in this. I'm not familiar with the async decorator in Python, or why a completion value matters there.

All I know is with observables, the completion value is awkward at best. At worst, it will be used as some sort of weird anti-pattern promise replacement.

kriskowal commented 8 years ago

https://github.com/kriskowal/q/blob/v2/q.js#L549-L598

On Mon, Apr 25, 2016 at 8:29 PM Ben Lesh notifications@github.com wrote:

It turns out to be necessary for the async decorator to work properly.

I'm interested in this. I'm not familiar with the async decorator in Python, or why a completion value matters there.

All I know is with observables, the completion value is awkward at best. At worst, it will be used as some sort of weird anti-pattern promise replacement.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/zenparsing/es-observable/issues/87#issuecomment-214471595

benlesh commented 8 years ago

@kriskowal I can see where it's used, I cannot see where the completion value is "necessary", though. Can you elaborate?

kriskowal commented 8 years ago

@blesh The purpose of using the async decorator is to use a generator to divide a task into multiple events and eventually resolve a promise. The promise that an async function returns is resolved by the return value of the generator function, as communicated on the done iteration.

Promise.async(function *() {
    yield Promise.delay(100);
    return 10;
})().then(ten => console.log(ten))
benjamingr commented 8 years ago

@blesh kris is just talking about async functions, as in async/await, and how you need the return value to implement them as generators (see my comment in #77 about the same thing).

The use case for .return is cancellation, namely, it means "I want to cancel this iterator" or "I want to cancel this generator" - it runs finally blocks but nothing else - it models cancellation with disinterest semantics, roughly the same as the proposed native promise cancellation and very similar to how bluebird does cancellation.

benlesh commented 8 years ago

I see. Thanks for that context.

benlesh commented 8 years ago

I have complete accepting an argument for consistency with the iteration model; it should be possible to push the results of an iterator to an observer without loss.

I'm still stewing on this. I was really close to adding this feature in RxJS 5, but my spidey-sense was tingling.

All values arrive in one channel

iterator sends all values on one channel... next(). Since this is pull based, the completion is also received via result.done via the same channel. However this is a design detail. completion status could be checked just as easily via some other method or getter property. The fact is that all values are communicated via next(), and no other channel.

Iterator design in JavaScript might be off by a hair

In a designs like .NET's IEnumerator or even Java's Iterator, the consumer would probe the iterator to see if it actually had the next value prior to getting it. That's not possible in JavaScript's iterator design, which means we get this weird behavior with the last IteratorResult value being sometimes useful, sometimes not. Couple that with the behavior of function returns in generators, and you're left with this weird thing.

I don't think Observable should suffer because of Iterator's design.

My current case for removing complete value argument:

  1. All iterator values arrive in one channel.
  2. All observer values should arrive in one channel.
  3. If converting an iterable to an observable: The iterator result { value: 'foo', done: true } can be read and converted as observer.next('foo'); observer.complete();
  4. In a better design, iterator might look like Iterator<T> { next(): T, complete(): boolean }, which is the "dual" of Observer<T> { next (value: T): void, complete(): void }.
benlesh commented 8 years ago

BTW: I hope my previous post isn't too insulting to the TC39 or anyone involved in iterator design. It works, it has some efficiencies, but I think there were some unforeseen quirks when you include generator return behavior in the design.

zenparsing commented 8 years ago

Sorry, but the argument to complete stays (if any of this design stays : )

benlesh commented 8 years ago

@zenparsing ¯_(ツ)_/¯ #yolo