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

Improve defining action types #93

Closed
jhollingworth opened this issue Jan 30, 2015 · 3 comments
Closed

Improve defining action types #93

jhollingworth opened this issue Jan 30, 2015 · 3 comments

Comments

@jhollingworth
Copy link
Contributor

Our current approach to defining action types has a number of drawbacks:

  1. We can't support ES6 function declarations (e.g. { foo() { })
  2. Feedback I've had is that invokable constants is a little odd (I agree)
  3. Action creators become really long (e.g. updateCriteria: SomeConstants.UPDATE_CRITERIA(function (profileId, criteria) { ...)
  4. We mess with function contexts

Function annotations solve this problem perfectly. Unfortunately it won't be in any spec anytime soon. So we need an alternative approach.

The two contenders right now:

Have a type hash

  • Pros
    • Simple, easy to understand, terse
    • Precedent: We already do this with stores
  • Cons
    • Action type is not co-located with action function
    • Cannot support properties
    • Messing with context
var UserActionCreators = Marty.createActionCreators({
  types: {
    addUser: Constants.ADD_USER
  },
  addUser: function(user) {
    var user = UserUtils.createUser(name);
    this.dispatch(user);
    return UserHttpAPI.createUser(user);
  }
})

Invoke constant inside of creator

  • Pros
    • Don't mess with context
    • Don't have to proxy action creator functions
    • Can have multiple action types to a function
  • Cons
    • More complicated, need understand JS well to use this
    • 1 extra line of code
    • Cannot enforce everything has an action type
var UserActionCreators = Marty.createActionCreators({
  addUser: function(name) {
    return Constants.ADD_USER(function(dispatch) {
      var user = UserUtils.createUser(name);
      dispatch(user);
      return UserHttpAPI.createUser(user);
    }, this);
  }
})
@jhollingworth jhollingworth changed the title Document type hash Document action creator type hash Jan 30, 2015
@jhollingworth jhollingworth changed the title Document action creator type hash Action creator type hash Jan 30, 2015
@jhollingworth jhollingworth changed the title Action creator type hash Improve defining action types Jan 30, 2015
@therealcisse
Copy link
Contributor

Invoke constant inside of creator.

+1

The intent of dispatch is really clear here and:

Can have multiple action types to a function.

This could be a cool feature.

@oliverwoodings
Copy link
Contributor

I prefer option 1, mainly because you can then do cool stuff like this:

var UserActionCreators = new ActionCreators({
  types: {
    addUser: Constants.ADD_USER
  }
});

And have Marty automatically create the addUser method for you (with it auto dispatching). I think it also looks much cleaner and will be easier for people to understand. Trying to get across that 'you have to return an instance of a constant' is not going to be easy.

@jhollingworth
Copy link
Contributor Author

In Marty v0.9

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

No branches or pull requests

3 participants