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

Normalize actions/events signatures. #190

Closed
jorgebucaran opened this issue Apr 19, 2017 · 5 comments
Closed

Normalize actions/events signatures. #190

jorgebucaran opened this issue Apr 19, 2017 · 5 comments

Comments

@jorgebucaran
Copy link
Owner

jorgebucaran commented Apr 19, 2017

tl;dr

Make all functions in the API follow the same signature:

(state, actions, data, emit)


Now

  • view(state, actions)
  • actions.myAction(state, data, actions, emit)
  • events.loaded(state, actions, emit)
  • events.action(state, actions, data, emit)
  • events.update(state, actions, data, emit)
  • events.render(state, actions, data, emit)

After

  • view(state, actions)
  • actions.myAction(state, actions, data, emit)
  • events.loaded(state, actions, emit)
  • events.action(state, actions, data, emit)
  • events.update(state, actions, data, emit)
  • events.render(state, actions, data, emit)

Tentative: Pass the options (the same from app(options)) object to loaded as data, so it can match the proposed signature too. 🤔

@FlorianWendelborn
Copy link

FlorianWendelborn commented Apr 19, 2017

events.loaded(state, actions, emit) — @jbucaran

Maybe it's better to just use state, actions, _, emit here, where _ is (right now) always undefined? It streamlines everything and if we ever decide that we need data we don't have to break the API.

It's also not unheard of. JSON.stringify({}, null, '\t') is similarly used. Of course null doesn't have to be null here, but I've seen this a lot.

@jorgebucaran
Copy link
Owner Author

@dodekeract Good point!

@jorgebucaran jorgebucaran mentioned this issue Apr 19, 2017
@jorgebucaran
Copy link
Owner Author

jorgebucaran commented Apr 21, 2017

This is a breaking change, as you'll need to modify all your actions signature from:

(state, data, actions)

to

(state, actions, data)

@selfup @pedroborges @cdeutmeyer @lukejacksonn Are we okay with this?

@lukejacksonn
Copy link
Contributor

The difference between (s,a,d) and (s,d,a) has always niggled me and it would certainly mean one less thing for users to remember.. for these reasons I am all for it.

I also support @dodekeract's statement of using state, actions, _, emit for loaded. Just to keep things super consistent. Your proposal @jbucaran of passing options as the data for that function is also not a bad idea at all 👍

@jorgebucaran
Copy link
Owner Author

c29970c 🎉

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