Nicer createActions

[azproduction]

I noticed a few issues (imo) with actions.

1) Declaration and signature of action are separated

In the one hand It gives a nice overview of all existing actions, but in the same time one should scroll back and forth to find action signature.

var Actions = Reflux.createActions([
    'doStuff',
    // assume here is a 30 line list of actions
]);

/**
 * @param {object} data
 * @param {string} data.name
 */
Actions.doStuff.preEmit = function (data) {

};

2) Most IDEs loose autocomplete action name as they expect explicitly defined methods or something that looks like Object.create

3) User can access private preEmit and shouldEmit methods as they are defined on action 4) It is good to join preEmit and shouldEmit in order to keep familiar pattern of DOM even handlers

My proposal is better to illustrate with a code:

var actionSet = createActions({
  // (1) IDE understands that `a` is member of actionSet along with its signature
  /**
   * @param {object} data
   * @param {string} data.name
   * @param {string} data.value
   */ 
  a: function actionMiddleware(data) {
    // (2) Case: validate data
    if (validate(data)) {
      throw new TypeError();
    }

    // (3) Case: forward action
    if (forwardAction) {
      this.cancelAction();
      actionSet.b({
        name: data.name
      });
      return;
    }

    // (4) Case: async action or action delay
    if (wait) {
      this.cancelAction();
      delayPromise(1000).then(actionSet.a);
      return;
    }

    // (5) Case: action debounce/throttle or cancel
    if (doNotDispatchAction) {
      this.cancelAction();
      return;
    }
  },
  // (6) Hookless actions
  /**
   * @param {object} data
   * @param {string} data.name
   */ 
  b: function (data) {}
});

• cancelAction works in the same way as DOMEvent.preventDefault

[Janekk]

For better IDE support you can define your actions like that:

var actions = {

  init: {

    startGame: Reflux.createAction(),

    signIn: Reflux.createAction()

  }

}

It's just a little bit more verbose ;)

[azproduction]

True, but it separates preEmit and shouldEmit.

[spoike]

The idea has been put in PR #116

I'm open to the middleware approach, though might as well have a connect/express approach to it. Something like this:

var myAction = Reflux.createAction();

myAction.use(function(args, next) {
    if (args[0] === true) {
        next();
    }
});
myAction.use(function(args, next) {
    console.log(args[1]);
});
// and myAction.use(fn, [fn, [...]]);
// to make the ordering apparent

myAction(false, 'dude');
myAction(true, 'dude where is my car?');
// Outputs: "dude where is my car?"

That way you can reuse validation and transformations.

[azproduction]

I thought of middleware while composing this issue, I like middleware approach. But in the same time interface of middleware function should be neat.

Personally I don't like (args, next) format because:

• args: one have to manually name all arguments or have unnamed variables

function(args, next) {
    console.log(args[0]); // what does `args[0]` mean?
}

function(args, next) {
   // to much to write
   var someMessage = args[0];
   console.log(someMessage);
}

And that is why I am voting for natural function arguments.

• next: in case of "natural function arguments" next should be the last argument. And that is also not very neat: action middleware should know exact number of arguments or each time calculate the last argument.

In my opinion, we should think of utilising this of middleware function as, for instance, koa does. Anyway this keyword is private for every action middleware and can't be redefined from user-land like actions.myAction.call(myThis, true);.

myAction.use(function(arg1, arg2) {
    // Variant 1
    if (args1 !== true) {
        this.cancelAction(); // prevent all middlewares and do not emit action
    }
    // Variant 2
    if (args1 === true) {
        this.next(); // I do not like it personally, because .next() could be forgotten
    }
    // Variant 3 (jQuery-DOM-inspired)
    if (args1 !== true) {
        this.preventDefault(); // do not emit action
        this.stopPropagation(); // skip all middlewares
        // Helper for preventDefault+stopPropagation
        return false;
    }
});

Personally I like this.cancelAction(); and return false; as helper.

[maxguzenski]

I believe that use next() as argument is better because connect/primus and other libraries use it in this way. And it is a clean way to async code.

// or Reflux.ActionMethods.use(...) // for all actions created
// or Actions.use(...) for a group of actions
// or Actions.load.use(..) for a single action
myAction.use(function(packet, next) {
  packet.arguments; //original arguments sent to this action, can be overwritten
  this.actionName(); // or maybe packet.actionName()
  next(false); // stop propagation 
  next(); // middleware ok, keep going
});

1 - Use a object "packet" instead of arguments directly, in future packet can has more data/info. 2 - This approach can replace preEmit and shouldEmit.

But, if you really dont like 'next', reflux can use middleware like that:

// only one parameter, it is a sync middleware
// returning a false value (diff from undefined) it abort, otherwise it keep going
.use(function(packet) { ...});

// 2 arguments, it is a async code and need call next() or next(false)
.use(function(packet, next) { ...});