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

Idea / Architecture : Explicit action handlers / payload #339

Closed
jnoleau opened this issue Jun 5, 2015 · 3 comments
Closed

Idea / Architecture : Explicit action handlers / payload #339

jnoleau opened this issue Jun 5, 2015 · 3 comments

Comments

@jnoleau
Copy link

jnoleau commented Jun 5, 2015

Hi,

I am a beginner with Reflux & love it. When I tried to use it on a real app, I had some common issues solved by "hacky" code in my opinion.

Problem with Actions

Orphan handler VS Store handler

I saw a lot of Reflux project writing :

var Actions = Reflux.createActions({
    "login": {asyncResult: true},
    "welcome": {children: ["fromGoogle", "fromDirect"]}
});

// orphan handler for typical promised action
Actions.login.listen( function(arg1, arg2) {
    ServiceLayer.doSomeLogic()
     .then(this.completed);
     .catch(this.failed);
});

// orphan handler for a meta action
Actions.welcome.listen(function(args) {
  if(referrer .. ) this.fromGoogle(args);
  else this.fromDirect(args)
});

// store handlers
var WelcomeMessageStore = Reflux.createStore({
 listenables: Actions,
 onWelcomeFromGoogle: function(){
   console.log('Hello from google')
 }
 onWelcomeFromDirect: function(){
   console.log('Hi !')
 }
})

I think orphan handlers are used to write logic calls to the business layer because in Stores we don't want this code, Stores are interested by updating/triggering datas from the results of actions

// this store is useless AS a store, it triggers nothing and has no data
var LoginActionStore = Reflux.createStore({
   onLogin: function(){
      .. Actions.login.completed(..)
      .. Actions.login.failed(..)
   }
})

Moreover in this Store, the data flow is not respected because store is calling an Action. View -> Action <-> Store

Static descriptors

Object passed in createActions is finally just a static descriptor, an incomplete descriptor.

var descriptor = {
   "login": {"completed","failed"}, //login(token) ? login(username,password) ? login({username, password}) ??
   "welcome"
};
var Actions = Reflux.createActions(descriptor);

// need to find into the code a call or a listen to know the payload
Actions.login(username,password);
Actions.welcome();

I mean next code for me is equivalent, no more no less entropy and Reflux.createActions seems .. quite useless

// static implicit descriptor = comment !
/**
 * Available actions : login, loginCompleted, loginFailed, welcome
 */

// no need of createActions
Reflux.Action('login')(username,password);
Reflux.Action('welcome')();

Conclusion

To conclude I think

  • Actions need to have a declarative signature not just a declarative name
  • Actions sometimes need to act ! not just a publisher
  • Business logic calls don't reside in stores

Proposal (syntax)

All of these points can be simply solved with a single explicit handler when declaring Actions

var UserActions = Reflux.createActions({
   login: function (username, password) { //action handler can call other actions
      BusinessLayer.login(username, password)
        .then(this.loginCompleted)
        .catch(this.loginFailed)
   },

   loginCompleted: function(user) {} //empty handler with payload signature

   loginFailed: function(error) {
    Action.showError(error);
   },

   getNotifications: function (fromDate, toDate) {
    //preEmit, shouldEmit can be done here in the handler
    if (fromDate > now) {
      this.emit(now, toDate); //pre emit with customizing the payload sent to data stores
    }

    if (toDate < now) {
      this.emit(fromDate, now);
    }

    this.cancelEmit();
   }

});

//or maybe (better I think), the payload is explicitly returned by the Action

var Actions = Reflux.createActions({
  login: function(u, p) {
    return Api.promiseLogin(u, p).then...; //single paylaod
  },
  loginCompleted: function(result) {
    return [result.user, result]; //multiple
  },
  shouldNotEmitAction: function(x) {
    return undefined; //we can use arbitrary undefined to cancel emit
  },

  noReturnAction: function(x, y) {
    ..
    //if no return, we can simply return arguments as an array (i.e [x, y]) in the createActions decorator
  }
});

WaitingStore = .. {
  onLogin :function (promise) {
    .. read promise state ?
  }
}

UserStore = .. {
  onLoginCompleted: function(user, result) {
    ..
  }
}

This proposal is just a partial reflexion so maybe there are mistakes or things impossible. I want to share it to have your feedback / thought about problems & solution.

Thx.

@spoike
Copy link
Member

spoike commented Jun 5, 2015

I think using functions as definitions are fine and instead of using existing hooks and if we could get rid of the different return means different things then I'll be happy.

Basically since the functions will be bound to some action context, it could expose some helper methods. Taking example above...

var Actions = Reflux.createActions({
  login: function(u, p) {
    Api.promiseLogin(u, p)
      .then(this.actions.loginCompleted); 
        // should this.actions is a reference to the created action set?
  },
  loginCompleted: function(result) {
    this.emit(result.user, result);
      // normally let the action emit
  },
  shouldNotEmitAction: function(x) {
    // returning nothing or not doing `this.emit(...)`, will not emit anything
  }
  passThroughAction: {} // default options, just passes through the arguments
});

What do you all think?

@spoike
Copy link
Member

spoike commented Jun 6, 2015

This is also discussed in #173.

@LongLiveCHIEF
Copy link

Closing in favor of #173. (Added to roadmap for v0.3.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants