cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Return Promises in progress callbacks #344

Closed eliseumds closed 10 years ago

eliseumds commented 10 years ago

Firstly I would like to explain my problem, so things will hopefully get clearer:

I have a method groupController.getAllMembers('groupA') that returns a stream (Promise) of objects like:

{ _id: 'myprofileid1' }
{ _id: 'myprofileid2' }
...

Upon receiving each of them, I would like to get additional information via another controller:

groupController.getAllMembers('groupA').progress(function(item) {
    // this new Promise resolves with the profile data
    return profileController.get(item._id);
}).progress(function(profile) {
    console.log('Full profile information:', profile);
});

The progress-spec is still underspecified so I am sorry if I am creating an issue of something already discussed.

Maybe a bit of code can help: https://gist.github.com/eliseumds/ba802f698c251ed65f0d

briancavalier commented 10 years ago

Hey @eliseumds! Yes, promise progress is a somewhat unfortunate thing. It's definitely underspecified and most libs (when.js included) implemented something that seemed reasonable at the time :)

Promises work best when they represent a single value. So, it looks a proper stream might be the right fit here--preferably one that interoperates easily with promises. That's exactly the kind of use case we have in mind with most.js.

Here's a quick example of what it might look like using most.js to take a promise for an array of "members" and mapping them to their respective profiles:

var most = require('most');

most.fromPromise(groupController.getAllMembers('groupA'))
    .flatMap(most.fromArray) // send the array items into the stream individually
    .map(function(item) {
        return profileController.get(item._id);  // map to a promise for the profile
    })
    .flatMap(most.fromPromise) // wait for promises to resolve
    .forEach(function(profile) {
        // do something with a profile
    });

Sorry if my example doesn't quite match--I wasn't sure if you're delivering all the members in a single message (ie as an array) or truly as discrete events. Either way, though, I think a stream is the right fit. Let me know if most.js looks like it would work for you. I'd be happy to answer questions about it, since the docs are currently in progress (we're actively working on them atm)

eliseumds commented 10 years ago

Hi @briancavalier.

I was already doing that with the RxJS library, but I was looking for something shorter as the system has hundreds of stream operations and everything returns to the client as stream as well.

I didn't know most but I will definitely take look at it. It might suit my needs with a smaller codebase.

briancavalier commented 10 years ago

@eliseumds ah, cool, ok. Please do let us know if you try out most.js. Your feedback would be very valuable, and very much appreciated. It's relatively young, but also being very actively developed. We'll be adding new combinators probably daily, but we also hope to keep existing APIs relatively stable (since most stream combinators have relatively well-defined signatures).

So, if you need something that's not there, let me know and we can probably add it fairly quickly!

jescalan commented 10 years ago

Hey I just wanna totally piggyback on here while you're talking about most -- I've been watching it since it started, but I'm afraid that since I don't come from a functional programming background, a lot of the terminology and theory is foreign to me. I feel like once I could understand what was going on, I would find it quite useful though. I've put nearly all of when's functionality to great use, couldn't be happier with the utilities. Is there anything you could recommend for someone like myself to be more comfortable with the foundation that most is built on?

briancavalier commented 10 years ago

@jenius no problem! This gist has been making the rounds recently, and is a really good, and basic, introduction to reactive programming with event streams.

At their most basic level, you can kind of think of discrete event streams (different from block data streams, like file streams, etc.) being to promises what lists are to plain values. I say "lists" because JavaScript Array isn't quite the right analogy: lists can be infinite, as can streams (don't worry, being infinite doesn't mean "requires infinite memory"!). Reactive programming is a much broader topic of which discrete streams are a building block. As the gist above says, learning to think in that way is a process that takes time. Once it clicks, though (like when promises click), it's amazingly powerful.

We can chat more about it on IRC if you want :) (@eliseumds we're usually hanging out in #cujojs on freenode if you want to drop in)

jescalan commented 10 years ago

@briancavalier sweet, thank you! I'll take some time with this. If these concepts improve my workflow anywhere near where what promises have done for it, it will certainly be worth it!

eliseumds commented 10 years ago

I ended up creating a Rx.Subject to bind the Promise progress event to the Observer.onNext method.

BaseController.prototype.asObservable = function(promise) {
    var subject = new Rx.Subject();

    promise.then(function(value) { subject.onCompleted(value); },
                 function(value) { subject.onError(value); },
                 function(value) { subject.onNext(value); });

    return subject;
};

myController.asObservable(promise)
    .flatMap(function(item) {
        return getProfile(item._id);
    })
    .forEach(function(profile) {
        console.log('[profile]', profile);
    }, console.error.bind(console), console.log.bind(console));

I made it explicit to avoid confusion. I still don't know how to properly create an API that tells when a method will return a Promise or a Stream.

Is there a Subject implementation planned for most.js?

briancavalier commented 10 years ago

@eliseumds Interesting idea! Looks pretty good, and I'll be interested to hear how well it works in practice.

One question: When the promise rejects, should it do both subject.onError(value) and subject.onComplete()? I'm not as familiar with RxJS's error handling policies, so really just asking for my own understanding :)

I still don't know how to properly create an API that tells when a method will return a Promise or a Stream.

Honestly, I think I might just try to avoid having a method that might return either. I'd probably pick one as the return type ... it's easy enough for the caller to convert from one to the other.

Is there a Subject implementation planned for most.js?

Absolutely, there will be something akin to Subjects, though most.js's approach will likely be slightly different in the details. I want to keep the two "ends" (producer and consumer) more separate than Subject does. I have been prototyping in a branch, but it's not ready just yet. The ability to imperatively push events into a stream is key for certain types of adapters, eg adapting DOM events into streams.

eliseumds commented 10 years ago

One question: When the promise rejects, should it do both subject.onError(value) and subject.onComplete()? I'm not as familiar with RxJS's error handling policies, so really just asking for my own understanding :)

You should only call onError, but there are ways to resume and retry:

Honestly, I think I might just try to avoid having a method that might return either. I'd probably pick one as the return type ... it's easy enough for the caller to convert from one to the other.

I agree with you. Right now, all my methods are returning promises.

Absolutely, there will be something akin to Subjects, though most.js's approach will likely be slightly different in the details. I want to keep the two "ends" (producer and consumer) more separate than Subject does. I have been prototyping in a branch, but it's not ready just yet. The ability to imperatively push events into a stream is key for certain types of adapters, eg adapting DOM events into streams.

Great! I just subscribe to most.js notifications. The code is so clean, just like when.js's. That's awesome :)

briancavalier commented 10 years ago

You should only call onError, but there are ways to resume and retry

Cool, thanks for the info and links. I'll def read up on those as I design the error handling approaches in most.js.

Great! I just subscribe to most.js notifications. The code is so clean, just like when.js's. That's awesome :)

Woot, thanks! We try to do a lot with a little. Once most.js makes a little more progress, I'd really love for you to try it out (and you, too @jenius!), and hear your feedback. I think the 3 biggest things that it needs atm are: 1) Subject-like API for push events, 2) error handling, 3) API docs and a couple simple examples. I'm hoping to get some of that done this week.

It seems like we've worked through to an approach for dealing with promise progress. I'll close this issue, but feel free to keep discussing or reopen if you need.