Skip to content
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

Joining parallel listeners to one #27

Closed
spoike opened this issue Aug 1, 2014 · 8 comments
Closed

Joining parallel listeners to one #27

spoike opened this issue Aug 1, 2014 · 8 comments

Comments

@spoike
Copy link
Member

spoike commented Aug 1, 2014

I'd like to propose an alternative to the waitFor function to something that makes more sense in a data flow pattern. As described with #18, there are occasions where you're interested in data coming in parallel. The API should be something as simple as a join function (name somewhat inspired by the parallel workflow concept):

var dataStore = Reflux.createStore({
    init: function() {
        this.listenTo(anAction, this.callback)
            .join(anotherAction, anotherStore /*, ...*/);
    },
    callback: function(anActionArgs, 
        anotherActionArgs, 
        anotherStoreArgs /* , ... */) {
            // ...
    }
});

It would be awesome if we didn't need to pull in a promises library dependency for this.

Please do let me know if we can improve this API before implementing it.

@spoike
Copy link
Member Author

spoike commented Aug 1, 2014

Alternative (no. 2) is to keep the listenTo function as it is, but give support for adding more listeners in the arguments at the same time. The last argument is always the callback. The only con with this idea is that the callback interface changes depending on how you call the listenTo function. Example:

var dataStore = Reflux.createStore({
    init: function() {
        this.listenTo(anAction, 
            anotherAction, 
            anotherStore /* , ... */, 
            this.callback);
    },
    callback: function(anActionArgs, 
        anotherActionArgs, 
        anotherStoreArgs /*, ... */) {
        // ...
    }
});

@spoike spoike changed the title Joining listeners to one Joining parallel listeners to one Aug 1, 2014
@bripkens
Copy link
Contributor

bripkens commented Aug 1, 2014

Your second alternative seems more intuitive to me. We could even get a bit closer to the ES 6 Promise interface by using something like this (referring to Promise.all):

var dataStore = Reflux.createStore({
    init: function() {
        this.listenTo(Reflux.all(
          anotherAction,
          anotherStore
        ), this.callback);
    },
    callback: function(anActionArgs, 
        anotherActionArgs, 
        anotherStoreArgs /*, ... */) {
        // ...
    }
});

This has the added benefit that event synchronization is the sole responsibility of Reflux.all. Reflux.all would create a new listenable and listenTo would not need to be changed.

@bripkens
Copy link
Contributor

bripkens commented Aug 1, 2014

I'd be glad to provide an initial implementation for this.

@spoike
Copy link
Member Author

spoike commented Aug 1, 2014

I really like the Reflux.all idea. 👍 You may go ahead @bripkens :-)

@dashed
Copy link
Contributor

dashed commented Aug 1, 2014

Isn't this a waitFor function anyways?

@spoike
Copy link
Member Author

spoike commented Aug 1, 2014

Yeah, it essentially is @dashed. We could argue if it should be in scope of the project or not, but after reading this blog post on react blog, I caved in for the need to handle data coming in parallel. It is however a different need from chaining stores (which Facebook React suggests you do with waitFor) where data comes in a sequential flow instead of parallel flow.

As @bripkens says, responsibility for the synchronization stays within Reflux.all. The only thing stores care about is that they listen to something, i.e. adher to an interface with a listen function. It has the added benefit of a simple monkey patching possibility, e.g. if anyone wants to create another listenable object.

We could provide a Store.waitFor function that does the Store.listenTo(Reflux.all(...), cb) but I find that to be more confusing API because the callback signature won't be easily reasoned about after you look at Store.listenTo's callback signature (i.e. "Why are arguments passed as is in listenTo but as arrays in waitFor?"). At least the use of Reflux.all makes it pretty much explicit why the callback signature is the way it is.

@spoike
Copy link
Member Author

spoike commented Aug 1, 2014

Thinking about it, this has the added benefit of letting you compose listenable objects that can be shared among several stores as well.

var theTide = Reflux.all(timeStore, stopWatchStore, waveStore);

// later in other stores
var clockStore = Reflux.createStore({
    init: function() {
        this.listenTo(theTide, this.theTideCallback);
    },
    theTideCallback: function() { /* ... */ }
});

var clockDebugStore = Reflux.createStore({
    init: function() {
        if (debugIsOn) {
            this.listenTo(theTide, this.debugCallback);
        }
    },
    debugCallback: function() {
        console.log(arguments);
    }
});

Reuseable waitFor FTW!

@spoike
Copy link
Member Author

spoike commented Aug 5, 2014

Is merged into #34

@spoike spoike closed this as completed Aug 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants