Skip to content

Do we want to rename dispatch() and getState() before 1.0? #229

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

Closed
gaearon opened this issue Jul 7, 2015 · 60 comments
Closed

Do we want to rename dispatch() and getState() before 1.0? #229

gaearon opened this issue Jul 7, 2015 · 60 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Jul 7, 2015

Bikeshedding thread here.

As part of 1.0 we can do some breaking changes we won't be able to do after 1.0. This includes bikeshedding on names.

I wanted this library to be a functional programming trojan horse for Flux people, which I think it succeeded at. Before we reach 1.0, we are able to drop some of the Flux naming baggage and find better names for whatever it does.

We're already getting rid of dispatcher and renaming stores to reducers. Do getState and dispatch still make sense?

Traditionally dispatch was called this way because in Flux an action really is dispatched across multiple Stores. In Redux, however, functional composition means that there is only one root reducer at the top. Do we really dispatch?

I've also heard some people are confused by getState as it sounds similar to React's setState and seems to imply some connection to React. In fact, Redux has nothing to do with React.

What if instead of { getState, dispatch, subscribe } we had... { read, perform, subscribe }?
Let's bikeshed.

@goatslacker
Copy link

push, pull, subscribe?

get, set, subscribe?

read, write, subscribe?

get, apply, subscribe?

or a more explicit: value(), reduce(), subscribe

@johanneslumpe
Copy link
Contributor

I think perform is way better than dispatch, as it captures what we are doing, performing an action. I'm not 100% sure about read - it's missing something for me Read what?. That might just be because I'm so used to getState by now.

@tappleby
Copy link
Contributor

tappleby commented Jul 7, 2015

I don't mind value, apply or perform, subscribe

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

I don't want symmetry (get/set, read/write) because they are asymmetric.
One receives an action, another returns the state.

@mindjuice
Copy link
Contributor

I like this Trojan horse idea.

To me, read sounds like some action is being taken rather than just accessing some existing value.

Maybe valueOf?

dispatch -> perform sounds pretty good. Can't think of anything better.

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

perform is way too general. Performing what? dispatch may not be ideal, but it's at least somewhat specific.

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

dispatch was called this way because in Flux an action really is dispatched across multiple Stores

Even with a single reducer, in practice the same action is still being handled by many different sub-reducers. I think dispatch() still kind of fits, but I agree we should try to find something better.

@mindjuice
Copy link
Contributor

Clojure uses send to send actions to agents. (http://clojure.org/agents)

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

Is perform(action) really more general than dispatch(action)?

If we throw away the Flux association (which is misleading anyway b/c there is no dispatching under the hood) they sound equally generic to me.

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

I can perform a task, or a symphony, or a lobotomy. Dispatch is much narrower.

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

Transducers have step(), which is technically a reducer itself...

type Step<X, Y> = (acc: X, val: Y) => X

...but still conceptually related

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

I like send, but I think if we do get rid of dispatch, its replacement needs to be significantly better, since we're switching it for a term that people already understand. I'm not sure if send crosses that threshold.

Also send and dispatch are basically synonyms.

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

Could be another way: { getState, performAction, subscribe }. Too verbose?

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

I kinda think so, and I don't see what it gives us versus dispatch. It's also an imperative phrase (in the English sense, not the programming sense*) which implies that an action has yet to occur, whereas action objects are representations of things that have already happened.

*EDIT: although come to think of it, in the programming sense too
*EDIT2: that's not really fair, though, because essentially all function names are imperative :)

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

For context, here are the dictionary definitions (I just took the first entry from Google):

  • dispatch – send off to a destination or for a purpose.
  • perform – carry out, accomplish, or fulfill (an action, task, or function)
  • send – cause to go or be taken to a particular destination; arrange for the delivery of, especially by mail

dispatch still fits the closest, IMO. It has the "fire and forget" connotation.

@mindjuice
Copy link
Contributor

Do you need the get? How about just state.

const { counter } = redux.state();

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

@mindjuice That looks like a direct reference to an object, whereas getState() is clearly an accessor.

@mindjuice
Copy link
Contributor

@acdlite Sorry, I have no idea what you mean by that! Can you elaborate?

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

@mindjuice Sure :) redux.state looks like you could use it like this:

redux.state.todos // wrong

whereas getState() is clearly a function

redux.getState().todos

@mindjuice
Copy link
Contributor

I see what you mean, but did you miss the parentheses after state() in my example line? It is a function.

const { counter } = redux.state();

@mindjuice
Copy link
Contributor

I just have a slight allergy to get in front of things. Maybe from my brief Eiffel programming days. :)

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

I think it's a good idea to stick to verbs for method names in JS.
Maybe { dispatch, getState, subscribe } is the best we can do after all.

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

@mindjuice Yes I know, but in the JS world accessors usually have verbs in front of them, so it could be a source of confusion.

@gaearon Do we want to consider getAtom() again?

@mindjuice
Copy link
Contributor

I don't think you need a verb for a read-only accessor, but that's just my opinion. Either way, it won't stop me from using Redux!

@mindjuice
Copy link
Contributor

@acdlite I (almost) never use get on accessor functions. It's unnecessary clutter IMO.

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

@acdlite Might as well consider it too. :-)

Let's leave this open for a while and hear more suggestions.

@mindjuice
Copy link
Contributor

The job of a function is to return a value. If the function is named for the thing it returns, I think that's great.

@nateajohnson
Copy link

Late to the party but some thoughts from a user. How about:

getShape() or just shape()
fire()
subscribe()

-Nate

@acdlite
Copy link
Collaborator

acdlite commented Jul 7, 2015

@mindjuice I see your point, but it's nice to be able to look at a parameter, method name, variable name, etc. and know if it's a function without having to refer to documentation.

@mindjuice
Copy link
Contributor

@acdlite Perhaps mildly. I generally prefer verbosity too (which iOS dev in Objective-C encourages), but functional programming tends to prefer terseness. Anyway, like I said, it's a minor issue. I still prefer state(), but I can see how in a language like JS with no type specifications, it can be possibly confusing to someone new.

@mindjuice
Copy link
Contributor

Look at underscore's API. Pretty darned FP I think you'd agree.

The accessor methods for Arrays are things like first, last, rest, etc., not getFirst, getLast, getRest. Same for Objects with functions like keys, values, pairs, etc.

Anyway, that's all I have to say on the matter. Just give it five minutes! :-)

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

@mindjuice Underscore API is a set of utility methods. It's assumed from context that they have single word names which aren't always verbs. There is no similar context here.

@goatslacker
Copy link

getValue() and send() isn't too bad.

We dislike getState because it can be confused by setState, but getState is actually the best name for it. Same goes for dispatch.

Although since send and dispatch are synonyms and clojure uses send I would opt for send because it's shorter and gets the point across pretty well.

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

I think it's important to consider the context. dispatch (or send) will be primarily used inside components (or views).

this.props.send(increment());

feels like it assumes some kind of network thing unless you worked with a similar convention before (unlikely for JS devs).

@goatslacker
Copy link

Speaking about context, this is a flux-like library so flux-like idioms could be used. I know it's good to appeal to people not coming from flux but we must also keep in mind those who started by reading through vanilla flux and then moved onto redux. The more these concepts map to each other the easier the transition will be.

Also, are there really any better suggestions than what seems like kicking off a network thing?

@gaearon
Copy link
Contributor Author

gaearon commented Jul 7, 2015

There's the existing names which might be best after all :-)

@mindjuice
Copy link
Contributor

@goatslacker That's true to a certain extent, but what I like about Redux is that it improves some things that Flux got fundamentally "wrong" (e.g., renaming Stores to Reducers and moving who owns the state), so sticking to Flux terms may not always make sense . If dispatch isn't really doing a dispatch, then naming it that to be familiar seems wrong. Of course this thread shows that coming up with a better name is hard too!

@gyzerok
Copy link

gyzerok commented Jul 8, 2015

@gaearon, @acdlite I do not see any problem with performAction since every programmer can do import { performAction as perform }. So the API name is verbose enough and you still have an ability to shortcut it with ES6.

@gyzerok
Copy link

gyzerok commented Jul 8, 2015

For the other hand getState() is rly confusing since it points us to React state. The problem is that we talking about this Flux implementation as others. It means that we think in our heads like it is state, but its not. In real world apps this state is for the first a cache for our server db. In my experience most of the time I'm caching db data with Flux Stores, and rly rare use it to store client specific values. What do you think guys?

@aaronjensen
Copy link
Contributor

perform seems good, I'm not sure the verbosity of performAction is necessary. Another one I'll toss out for consideration is handle but I'm not a huge fan of it, it feels like it's the wrong end.

I agree that getState can be somewhat confusing, but it's not bad. I don't mind getAtom and it solves the react overlap. It may lead people to clojurescript though... 😄

@dbrans
Copy link
Contributor

dbrans commented Jul 8, 2015

dispatch seems like a strange name for a function that takes an action and produces a new application state based on that action.

How about reduce?

@clearjs
Copy link
Contributor

clearjs commented Jul 8, 2015

+1 for apply and getAtom. State usually implies mutability, so Atom is a good hint.

@clearjs
Copy link
Contributor

clearjs commented Jul 8, 2015

When dispatch is invoked, action creator has already finished execution, so an action has been performed. What is left is to apply it to the Atom.

@gaearon
Copy link
Contributor Author

gaearon commented Jul 8, 2015

I'm not a fan of apply because of the potential confusion with function apply method.

@clearjs
Copy link
Contributor

clearjs commented Jul 8, 2015

update is another alternative, but it's not perfect either.

@clearjs
Copy link
Contributor

clearjs commented Jul 8, 2015

Also, append. If we consider an analogy with Event Sourcing, this would be like appending to the event log.

@emmenko
Copy link
Contributor

emmenko commented Jul 8, 2015

I have a feeling this is slightly going off topic. If we take a step back and see what we have when we execute

dispatch(anAction())
// or
dispatch(anActionAsync())

The action method simply returns a plain object, or a function. It doesn't perfom anything yet. You might as well pass an object directly to dispatch.
Then, what dispatch does are basically 2 things:

  • calls the reducer function
  • calls all the registered listeners

So it's actually just calling functions. Something like execute could also be appropriate in that sense.

So maybe we should start from that? I don't have any good names for that but maybe it will help?

@clearjs
Copy link
Contributor

clearjs commented Jul 8, 2015

consume?

@ellbee
Copy link
Contributor

ellbee commented Jul 8, 2015

I'd keep the existing names. I think dispatch fits more than any of the proposed replacements for it. If there is concern over getState being confused with component state then I'd probably make it more verbose, like getStoreState, rather than introducing another term like getAtom, but I don't really like either alternative.

@gaearon
Copy link
Contributor Author

gaearon commented Jul 8, 2015

I agree. Let's keep the existing names.

@gaearon gaearon closed this as completed Jul 8, 2015
@vladap
Copy link

vladap commented Jul 9, 2015

@acdlite

(dispatch, state) => {...}
Do you see how that could be confusing?

Actually, I don't.
dispatch is a verb and suggests it is a complex function, probably with input parameters.

state is a noun suggesting it is a property/value/instance or getter returning a property/value/instance. It doesn't matter what it actually is if all cases are accessed uniformly.

I would expect to be able to do state.something.

It turns out that ES6 supports to access it uniformly because it provides class property getters and setters. Uniform Access Principle is embraced in FP without get/set prefixes.

Hence, instead of

class Store() {
  getState() { return this.state }
}

const s = store.getState()

it could be

class Store() {
 get state() { return this.state }

const s = store.state

if I would do const s = store.state() it doesn't compile.

I think, it might open an opportunity to have getters in store object but access them in the same way in @connect without thinking if I get a state property via a getter function from a class instance or a property from a plain object. It might be helpful to implement Lenses in store because they would behave seamlessly with getting objects properties.

@emmenko
Copy link
Contributor

emmenko commented Jul 9, 2015

It's cool that we try to use all the new cool features, but we should think about browser compatibility also.

@johanneslumpe
Copy link
Contributor

@emmenko that's only a problem if you strive for IE8 compat.

@emmenko
Copy link
Contributor

emmenko commented Jul 9, 2015

Sure, although there are other like classes etc

https://babeljs.io/docs/advanced/caveats/

Just saying we should consider this and set a minimal target. Would be cool also to run some e2e tests against different browsers (eg: saucelabs).

@acdlite
Copy link
Collaborator

acdlite commented Jul 9, 2015

Actually, I don't. dispatch is a verb and suggests it is a complex function, probably with input parameters. state is a noun suggesting it is a property/value/instance or getter returning a property/value/instance. It doesn't matter what it actually is if all cases are accessed uniformly.

@vladap This is exactly what I said above. :D

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