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

Reflux.createActions needs more work? #14

Closed
dashed opened this issue Jul 29, 2014 · 7 comments
Closed

Reflux.createActions needs more work? #14

dashed opened this issue Jul 29, 2014 · 7 comments
Labels

Comments

@dashed
Copy link
Contributor

dashed commented Jul 29, 2014

PR at #13 is a baby step; which shares EventEmitter among group of actions.

This allows the possibility of certain features:

1. Should we bother adding/removing actions to/from context/namespace/group of actions?

May need to use Object.defineProperty for custom getter/setters to register/unregister to the namespace's EventEmitter. Since react.js supports IE8, Object.defineProperty will need to be polyfilled.

  1. Assign hook functions when calling actions. Hook functions get executed before/after each emit.

    Inspired by mount lifecycle of react.js.

    Proposed example:

    var Actions = Reflux.createActions([
      "statusUpdate", 
      "statusEdited",
      "statusAdded"
    ]);
    
    // assign hook functions
    
    // executed prior to emit
    Actions.statusUpdate.preEmit = function() {};
    
    // test whether emit should occur. Executes after preEmit.
    Actions.statusUpdate.shouldEmit = function() {};
    
    // Alternative syntactic sugar -- only assign preEmit functions
    
    var hooks = {};
    hooks.statusUpdate = funnction() {};
    hooks.statusEdited = funnction() {};
    
    // Map based on keys from hooks.
    var Actions = Reflux.createActions(hooks);
    
    // add/remove/change hook functions
    Actions.statusUpdate.shouldEmit = funnction() {};
@spoike
Copy link
Member

spoike commented Jul 29, 2014

1: Could you provide an example of how you mean or how our API should look like for this? I have some trouble visualizing this.

2: I like the hooks, we should totally have that.

@dashed
Copy link
Contributor Author

dashed commented Jul 29, 2014

For (1):

var statusUpdate = Reflux.createAction('statusUpdate'),
    statusEdited = Reflux.createAction('statusEdited');

// statusEdited unregisters its own EventEmitter, and registers to 
// statusUpdate's EventEmitter.

// Any and all actions beyond the first parameter is merged to the context of 
// the action within the first parameter.
Reflux.combineActions(statusUpdate, statusEdited, ...);

// support arrays?
Reflux.combineActions([statusUpdate, statusEdited]);


var Actions = Reflux.createActions([
  "statusEdited",
  "statusAdded"
]);


// add statusUpdate (and others) to a context/namespace group
Reflux.combineActions(Actions, statusUpdate, ...);

// ideally, parameters to Reflux.combineActions(...) may be any combination of 
// single actions or any context/namespace groups

@spoike
Copy link
Member

spoike commented Jul 29, 2014

I'm a bit confused to why a user wants to combine actions. What will happen when one action is invoked, will other actions in that group be invoked as well? By original design, they're supposed to be independent emitters and if component wants to invoke many actions they should invoke them seperate from each other and let the deferring do the rest.

I feel this looks a lot like waitFor solution in Flux, that there might be a better way to do it (for which stores can listen to each other instead, the "aggregate stores"). Basically I want to know what the actual end user use case is for doing this?

If it is only for grouping actions together then just create actions as usual and put them in different object literals that will serve as namespace:

var Group1 = {
    doFoo: Reflux.createAction(),
    doBar: Reflux.createAction()
};
var Group2 = {
    doBaz: Reflux.createAction()
};

If you need to unregister actions from a "namespace" that a component uses then we do actually have the ListenerMixin in place that does this for us.

Is there something I'm missing with your proposal?

@dashed
Copy link
Contributor Author

dashed commented Jul 29, 2014

Hmmm, I refactored a bit, and it doesn't seem to make sense to dynamically add/remove actions within a namespace.

My original goal was to have Reflux.createActions() to use only one shared EventEmitter.

@dashed
Copy link
Contributor Author

dashed commented Jul 29, 2014

@spoike I think we can scrap the idea of dynamically combining/splitting actions. Edited the first post.

@spoike
Copy link
Member

spoike commented Jul 29, 2014

@dashed Ok, cool.

@spoike
Copy link
Member

spoike commented Jul 29, 2014

Closing this since I created an ticket #16 for the second part of this issue.

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

No branches or pull requests

2 participants