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

Recommend that Action constants be named in the past tense #384

Closed
wmertens opened this issue Aug 1, 2015 · 55 comments
Closed

Recommend that Action constants be named in the past tense #384

wmertens opened this issue Aug 1, 2015 · 55 comments

Comments

@wmertens
Copy link
Contributor

wmertens commented Aug 1, 2015

Since #377 was closed (for valid ecosystem reasons), how about recommending in the documentation that instead of INCREASE_COUNT, it is better to write COUNT_INCREASED?

That way one will be less tempted to put side effects in reducers.

@gaearon
Copy link
Contributor

gaearon commented Aug 1, 2015

This sounds sensible to me!

@imevro
Copy link
Contributor

imevro commented Aug 1, 2015

I agree 👍

@ioss
Copy link

ioss commented Aug 1, 2015

I am not convinced (yet), but open to be convinced.
For me Actions are an intend, the reducer might choose not to increment, so COUNT_INCREASED feels kind of strange then.
Also { type: 'COUNT_INCREASED', data: 2 } has a totally different meaning ("increased to 2") to { type: 'INCREASE_COUNT', data: 2 } ("increase by 2").

@merk
Copy link
Contributor

merk commented Aug 1, 2015

In the case of BUTTON_PUSHED, it makes sense - but I dont agree that INCREASE_COUNT should become COUNT_INCREASED - the count isnt increased when the AC creates that action.

@srph
Copy link

srph commented Aug 2, 2015

I wasn't immediately convinced. I'll have to think about the idea.

@wmertens
Copy link
Contributor Author

wmertens commented Aug 2, 2015

Actually, in that case the full name would be COUNT_INCREASE_REQUESTED,
which is a bit long indeed but still conveys the actual intent.

COUNT_INC_REQD?

On Sun, Aug 2, 2015, 04:12 Kier Borromeo notifications@github.com wrote:

I wasn't immediately convinced. I'll have to think about the idea.


Reply to this email directly or view it on GitHub
#384 (comment).

Wout.
(typed on mobile, excuse terseness)

@wmertens
Copy link
Contributor Author

wmertens commented Aug 2, 2015

I think that the ActionCreator has the obligation to check if an action is
possible (via a thunk) before blindly sending it out.

Reducers have the obligation to always keep the returned state valid.

The action log is the truth of what happened, and the state is just a
reflection of that.

On Sun, Aug 2, 2015, 07:07 Wout Mertens wout.mertens@gmail.com wrote:

Actually, in that case the full name would be COUNT_INCREASE_REQUESTED,
which is a bit long indeed but still conveys the actual intent.

COUNT_INC_REQD?

On Sun, Aug 2, 2015, 04:12 Kier Borromeo notifications@github.com wrote:

I wasn't immediately convinced. I'll have to think about the idea.


Reply to this email directly or view it on GitHub
#384 (comment).

Wout.
(typed on mobile, excuse terseness)

Wout.
(typed on mobile, excuse terseness)

@jedrichards
Copy link

COUNT_INCREASED 👎 for the reasons already stated in the thread
COUNT_INCREASE_REQUESTED 👍 but its probably too verbose

How about a recommendation to write action types in the "imperative mood" (a la the famous Chris Beams' Git commit message style guide). So in this case INCREASE_COUNT works well because it is in the imperative mood, i.e. "written as if giving a command or instruction", and conveys the notion that creating an action just signifies a wish or intent that something should happen at a point in time.

@ghost
Copy link

ghost commented Aug 3, 2015

I tend to see ActionTypes as things that have happened in the past as well. I found flux easier to initially grok by comparing it to ddd/cqrs/es with which I was already somewhat familiar. This discussion between @abdullin and @fisherwebdev might be of interest here. Particularly the bit titled "Actions -> Events"

@vladap
Copy link

vladap commented Aug 3, 2015

Personally, I'm getting to a conclusion that we are mixing two design patterns and two models how to think about a system. Both are valid in separation and probably they can be mixed in some applications.

The one is based on EventSourcing where Action object represents what happened. Reducers then interprets the past into a state, different reducers can interpret it into different views on the past.

The second is based on write ahead log (WAL). It is used to solve all sorts of hard problems, starting with recovery of a filesystem from journal, database transactions, replication, recovering on another machine after a failure... Also known as journal, transaction log, command log...
In this case Action object represent mainly intention to change state and whatever is needed, f.e. TRANSACTION_BEGIN, TRANSACTION_COMMIT... WAL is heavily used in databases to change state. All sorts of distributed problems are solved based on it.

I think that vanilla flux was more like WAL. I think that both are valid approaches and it just might depend on type of application which one makes more sense. Possibly Redux should stay agnostic to them.

My problem with past tense is that it is sometimes tricky to name action objects in past tense. My problem with imperative mood is that it is bound to WAL design pattern. Hence I'm currently investigating an option to use nouns to describe my action objects. They seem to work for both design patterns and allows tricky names. They are similar, sometimes the same as imperative mood, INCREASE_COUNT changes to COUNT_INCREASE, VISIT is VISIT etc.

As an example we can take DOM events. Try to name 'mousemove', 'mouseup', 'mousedown' in past tense, I think it will not take long to find the same tricky actions in your domain. It allows 'contextmenu' and such.

Redux can just describe all approaches with their pros and cons and let developer to choose which model fits his app and thinking better. I believe that Redux can be agnostic and easily support both.

@vladap
Copy link

vladap commented Aug 3, 2015

Maybe both are acceptable for both design patterns - COMMENT_DELETE or COMMENT_DELETION, i.e. OBJECT_VERB or OBJECT_NOUN as far as OBJECT_VERB is not understood as imperative mood, which is not that much compared to DELETE_COMMENT.

@vladap
Copy link

vladap commented Aug 3, 2015

It is the reason why having both in docs might be confusing (while technically correct in separation):

The only way to mutate the state is to emit an action, an object describing what happened. (Three Principles)

An action is a plain object that represents an intention to change the state. (Action in Glossary)

@sergey-lapin
Copy link

@vladap I guess I see that in same way.
I think that it became obvious that action itself defined poorly enough to make a lot of confusion. We have a lack of semantics on that now.
Action - call to do something or a fact that something happened?
Answer on this question defines how smart should be reducer. This shapes whole app.
I think that there some common agreement that reducers should be dump. It is the core of redux that should be simple, synchronous and reloadable. To get dump reducers we should propagate to them actions like "call to do something" not "fact". Because facts can be interpreted differently, so reducer tends to make some interpretations which makes it automatically smarter. I prefer to call "call to do something" - instruction(long name which is bad, but I like semantics).
I think that there is a gap between instruction and fact as values, but we call them both actions now - this is the main source of confusion.
As @gaearon commented this
"There are "smart" actions that turn into "dumb" actions at some point, they often describe "what should happen" Middleware interprets smart actions and turns them into dumb ones."
Past tense incline that an action is a fact, so I think this is actually a bad idea. I believe that reducer should get instructions. One other thing that I want to mention is that this all connected with other question "where business logic should live in my app?". User clicks somewhere (fact) which interprietated as array of instructions. (fact)=>(List[instructions]) - this is what actionCreator actually do now. And it is not clear to me how middleware correlates with actionCreator. I think middleware is more low level and should not make any decisions. Just some isomorphic stuff like unboxing promises and thunks. P.S. Maybe intent is better as it shorter as @ioss said but again instruction is better in semantics.

@ioss
Copy link

ioss commented Aug 3, 2015

I like the idea to use nouns! This - apart from easy to name - makes them timeless, they are fact and intent/instruction at the same time.

While reducers might be pretty dumb, they still - in my opinion - could contain business logic, like: "count should never become negative".
If they were totally dumb, they should be removed and replaced by a generic one.

While this is a pretty academic discussion, as long as we do not have a action-type-is-a-noun-validator ;) +1 for nouns

@sergey-lapin
Copy link

Couter example it too simple and does not represent this use case. Consider that we sync some database. And we want to show progress bar of that process.

export function syncDb() {
    return (dispatch, getState) => {
        const {db:{client}} = getState().toJS();

        dispatch(createAction(STATUS_SYNC_DB)('syncing'));
        client.sync((progress)=> {
            dispatch(createAction(PROGRESS_SYNC_DB)(progress))
        }).then(()=> {
            dispatch(createAction(STATUS_SYNC_DB)('complete'));
        })
    };

The code is not complete but it shows the idea. Cause we have async it is even imposible to do this in reducer, so it forced to be dump here, but user might be tempted to make some action like SYNC_STARTED in other cases and write some business logic in reducer.
Reducers are dump but we still need them and we can reuse them for syncronization with some other source, because they are dump.

INIT_DB: (state, action) => state.merge(action.payload),
PROGRESS_SYNC_DB: (state, action) => state.set('dbSyncProgress', action.payload),
STATUS_SYNC_DB: (state, action) => state.set('dbSyncMode', action.payload)

I use immutablejs but some other fellow may preffer plain js and remove all immutable js from reducers, and app still would work. And it can be easily done because all business logic located in action creator. @ioss that what I mean by dump reducer. So there is separation of concerns.

@sergey-lapin
Copy link

One more thing is that it would be awesome to dispatch both instructions and facts. But facts should be dispatched in way that they preceded action creator execution as this make possible to generate tests for action creators which I think is the most interesting part for testing.

@ioss
Copy link

ioss commented Aug 3, 2015

@Lapanoid: I am against the totally dumb reducers.
Your reducers are a good example for being so dumb they could be replaced by something like:
(state, action) => state.set(action.type, action.payload)

This means: If the action.payload is never processed or decided upon by the reducers (because they should not have business logic), the action creators might as well just pass mergeable payload, thus becoming kind of it's own reducer and the "real" reducers could be removed.

Also if they are that dumb, why have hotreloading for reducers then? They would hardly ever change.
The demonstration video by dan, shows pretty dumb reducers, still he changes the amount of the increment, which makes them contain "business logic", at a place where I think it should be.

@gaearon
Copy link
Contributor

gaearon commented Aug 3, 2015

These are different use cases.

For state that "lives" on server, reducers are dumb because there is little business logic. In fact it makes sense to generate reducers from a schema.

For state that "lives" on client (e.g. complex post editor), business logic is described in reducers.

@vladap
Copy link

vladap commented Aug 3, 2015

@Lapanoid Compared to my understanding I think you have facts shifted one layer up towards an user.

dispatch(createAction(STATUS_SYNC_DB)('syncing'));
dispatch(createAction(PROGRESS_SYNC_DB)(progress))
dispatch(createAction(STATUS_SYNC_DB)('complete'));

These are facts (actions as object) which happened in your AC logic. Set of facts describe execution of a logic in AC.

User clicks somewhere ("fact") which interprietated as array of "instructions". (fact)=>(List) - this is what actionCreator actually do now.

When user clicks it is not yet a fact by default. When he clicks he executes action as a function (ActionCreator) and some logic which results in facts (actions as an object). Usually synchronous operations are translated 1:1 to actions as facts and whole logic is done in a reducer. Async wrecks havoc on this and forces us to move all async code with all its dependencies to ActionCreators which can result in artificial business logic separation, especially in cases when logic needs to move back and forth between action creator and a reducer.

Reducers based on facts are loose model. Reducers can do whatever makes sense based on facts. They can be smart, they can be dumb (just reducing AC results to state).

You can use action as object to describe instructions but it is the other model of thinking which is more coupled (but it can be preferred).

I would be fine having ACTION AS FUNCTION (ActionCreator) and ACTION AS OBJECT.

@ioss
Copy link

ioss commented Aug 3, 2015

@gaearon: agreed. I didn't mean that there is no place for dumb reducers, just that I don't see that all should be logic free.

@vladap
Copy link

vladap commented Aug 3, 2015

My action objects (facts) are arrogant, they have no intentions, they just say - "this was done" and you, dear reducer, do your business. I don't care. I won't tell you what you should do, it is your business. My action objects might care if reducer kindly asks to better describe what happened but generally he is hesitant to change (to preserve existing kind of API).

Having 100 reducers it might be harder to find everything what happens on a given fact though. Which is the case with all event driven programming and highly separated architectures.

@sergey-lapin
Copy link

@vladap

When he clicks he executes action as a function (ActionCreator) and some logic which results in facts > (actions as an object).

This is a current design, but I think it is wrong actually. If user iteraction generated some immutable by default and only then AC would triggered by that we could test AC execution. This could be done in core transparently I think, pushing user to make additional action on every AC would be brutal.
In real world when user click - pufff - fact happend. Even if redux does not think so.

My action objects might care if reducer kindly asks to better describe

How it can be done?

@ioss

Also if they are that dumb, why have hotreloading for reducers then?

AC is also hot-realoded - so this is not an argument

still he changes the amount of the increment, which makes them contain "business logic", at a place where I think it should be.

they receive INCREMENT_COUNTER, they don't make any decisions, just make operation. So there is no business logic here. They are dump, but you still need them even in such simple case. They know how to maintain state on actions.

@ioss
Copy link

ioss commented Aug 3, 2015

I think the problem is the word "Action" (in Action and ActionCreator, and that ACs are often kind of used as DomEvent handlers).
If I read Action, my mind connects them with the events in the view (or other events, like from a backend) and I always have my mind to take a step back. I probably only have to get used to it, to get it into my subconscious. :)

At the moment i am thinking about going: DomEvent -> ViewActionCreator ("left" part of ActionCreator with VL) -> ViewAction/ExternalAction -> (optionally) middleware/store with ViewActionHandler ("right" part of ActionCreator with BL and possible sideeffects) -> (Store)Action -> reducers.
This way it may be possible to connect ViewActions to their StoreAction, which could give me the opportunity to replay View/ExternalActions "visually" in the devtool, but apply StoreActions to the state, without replaying sideeffects.
Not sure, just an idea...

@sergey-lapin
Copy link

Thing is that business logic(BL) in general case is not synchronous, so AC is better place for it anyway. Putting BL sometimes in AC and sometimes in reducers is confusing(maybe it is confusing just for me but anyway = ))

@jedrichards
Copy link

In terms of event naming style, in my opinion, the past tense style works best for the sort of intermediate ChildView to ParentView events you often write when building Backbone apps, e.g. CLOSE_BUTTON_CLICKED or DROPDOWN_CHANGED. A controller of some sort might then abstract these events away from the details of the UI implementation and re-dispatch them as more generic/reusable app domain events (or Commands) in the imperative mood such as CLOSE_MODAL or SET_SELECTED_ITEM respectively. In React/Redux we skip a few steps of view to view to controller communication and dispatch app domain actions right from the component. So I find myself thinking of Redux actions as somewhat analogous to commands being dispatched into a global event bus that's accessible to every view, which is partly why I feel the imperative mood fits best with action type naming.

As an aside, this transformation from DOM event into app domain action that often occurs in a click or change handler implicitly leaks some business logic into the view layer imo. I'm not sure how I feel about that, but as long as its naturally confined to smart/connected components it seems OK since they will be much more tightly coupled to the state layer anyway.

We know that once an action strikes the reducer's surface it must be dumb, compromised of only a type and an optional payload. I'm comfortable with action creators and middleware containing business logic, but it seems helpful to stipulate that such business logic strictly only relates to the transformation of a smart action into a dumb action, nothing else. Other business logic would most likely take place on the server, or alternatively the reducer.

Fwiw I would support a re-name of Actions to Commands since conceptually it seems to fit quite closely with the command pattern. For me, an action gives the impression that the how of the operation has already been pre-determined which doesn't really fit well with the core business logic happening elsewhere (server or reducer). In reality in Redux all we've done is transformed a DOM event into one of the available app-domain command types, supplied it with any necessary data and forgotten about it ... in other words we issue a command.

If anything should be called an "action" it would the description of the event that takes place when a dumb command hits the reducer at a specific point in time. So it's actions that we replay during time travel not commands and their potential side effects. The reducer acts on a command, hence performing a concrete action, but this would be internal implementation terminology abstracted away from library users, who only have to worry about issuing commands ... i.e. intents that something should happen.

From Wikipedia on the Command pattern:

the command pattern is a behavioral design pattern in which an object is used to encapsulate all information needed to perform an action or trigger an event at a later time.

Anyhow, sorry if renaming to Commands has been discussed before and decided against, feel free to ignore. Also sorry if I'm taking this off-topic feel free to point me in the right direction. Just my 2 cents worth :-)

@vladap
Copy link

vladap commented Aug 3, 2015

My action objects might care if reducer kindly asks to better describe
How it can be done?

It was just a metaphor:) Trying to convey that action objects should be well thought-out as it can be hard to change them once defined and some reducers are already hooked to them.

@sergey-lapin
Copy link

@jedrichards haha, now we have fact, action, instruction, intent and command =) It is funny how people see same thing differently I see that dump action/command/fact actually became smarter in some way as action/intent/instruction prescript what should happened next and carry some info in themselfs about it. And BUTTON_CLICKED as fact does not do that, so it is dumper. A fact interpretates to some set of instructions you may say.

@sergey-lapin
Copy link

Trying to convey that action objects should be well thought-out as it can be hard to change them once defined and some reducers are already hooked to them.

Yep. Because of that I want some clarity with actions, approach to them really influences on whole app.

@vladap
Copy link

vladap commented Aug 3, 2015

Thing is that business logic(BL) in general case is not synchronous, so AC is better place for it anyway. Putting BL sometimes in AC and sometimes in reducers is confusing(maybe it is confusing just for me but anyway = ))

It is the price currently paid for writing hot-reloadable code.

AC is also hot-realoded - so this is not an argument

If they contain async code, they are not hot-replayable, you can't do it deterministically. Redux DevTools ignore them on replay and replay operates on results from async AC - on action objects only => hot-replaying code is in reducers only.

@sergey-lapin
Copy link

@vladap That makes sense, I did not know that DevTools behave this way, thanks. So they are replayable only if sync? Then we can locate BL always in AC trying to make them as sync as possible

@vladap
Copy link

vladap commented Aug 4, 2015

@jedrichards Command pattern explicitly defines which command to execute, in other words which exact behaviour to execute. It is 1:1 relation. Flux has generally 1:M (many) relation between Action and executed behaviours. Pattern with 1:M relation is called Observer pattern to my knowledge.

Renaming actions to commands wouldn't help solve the root cause of the confusion (which is terms used vs. design patterns they suggest by default). Actions are used instead of Commands in some Command pattern implementations. In our context they are practically the same thing.

When I'm told it is the Command pattern I expect that one side defines what to execute, give it to other side which blindly execute it. If turnOnRadio command is defined it would be strange to turn on lights.

The original Flux is either an overloaded Command pattern where action objects are capable to trigger more and possibly unrelated behaviours, which can be confusing.

Or it is an overloaded Command pattern where execution of a single behaviour can be spread across more stores. So to turn on a radio, one store has to plug it while the other store will waitFor till plugged and then turn it on.

Or it is an Observer pattern with events strangely named as actions objects, which is confusing. Than I expect that one side just signals something happened without knowing what will happen next. If one side signals that radio was turned on then the other side can turn on lights if wish so.

In all cases it can mean that some logic was already executed which resulted either in commands or events. Commands vs. events just define level of coupling.

@vladap
Copy link

vladap commented Aug 4, 2015

@wmertens I haven't meant to replay side-effects, it is discussed in #307.

But yes, it would be inpure to use promises in reducer which means I would be actually replaying side-effects. They can be made replayable with a custom combineReducers (as far as async code uses only Redux.state) but I think it has a little use in practice.

@ioss
Copy link

ioss commented Aug 4, 2015

@vladap could you elaborate on your last sentence? I didn't get it.

@vladap
Copy link

vladap commented Aug 4, 2015

@ioss: Forget it, it is wrong. I think, it can't be done with just combineReducers.

@adri
Copy link

adri commented Aug 4, 2015

Like vladap mentioned, it is at the moment not consistent. This should be fixed, see:

The only way to mutate the state is to emit an action, an object describing what happened. (Three Principles, Reducer)
An action is a plain object that represents an intention to change the state. (Action in Glossary)

@adri
Copy link

adri commented Aug 4, 2015

A proposal:

Actions: As some people have mentioned, an action is seen as stating an intent to do something. Currently action creators return intents intend of a user but somehow later these intents are seen as facts.

Events: In Event Sourcing events are seen as facts that happened in the past. There is already a concept of an event store (not an action store).

Events instead of actions
I think action creators should return events. Actually action creators are the intents already. By calling an action creator function, the action is executed. While an action is executed, events can be dispatched, either by returning or by returning a callback function.

It's just a simple rename but I think it would help. What do you think?

@vladap
Copy link

vladap commented Aug 4, 2015

You could write your own combineReducers and execute some async code deterministically but because all reducers would have to synchronize till next action is executed the net gain is questionable - action processing and rendering is coupled (unlike f.e. in game engines) hence execution of reducers have to be fast to avoid unresponsive UI.

This my statement doesn't make sense either. Other actions would still go through. I'm not sure why I thought there is kind of a lock-step like in games. It is good to realize because multiple execution of action creators dispatching same actions can potentially lead to race conditions and unpredictable state.

@sergey-lapin
Copy link

I think this should be closed as well as #377 (comment) for same reasons @gaearon. (however this discussion was quite enlightening, at least for me)

@wmertens
Copy link
Contributor Author

wmertens commented Aug 5, 2015

Well, l think that simply recommending something in docs is quite different
from refactoring the API.
I think we achieved consensus that action constants make most sense in the
past tense, and that commands can be imperative but they must be converted
to past tense actions by action creators or middleware.

On Tue, Aug 4, 2015, 21:25 Sergey Lapin notifications@github.com wrote:

I think this should be closed as well as #377 (comment)
#377 (comment) for
same reasons @gaearon https://github.com/gaearon. (however this
discussion was quite enlightening, at least for me)


Reply to this email directly or view it on GitHub
#384 (comment).

Wout.
(typed on mobile, excuse terseness)

@sergey-lapin
Copy link

Ultimately we start to discuss the same thing - nature of actions which is fine because it is all connected. And I don't see any concensus here. If action is intent it should not be it past tense and if it is fact it should. But there would not be any renaming for reasons from #377

@sergey-lapin
Copy link

Action actualy used in both ways now, because it can be both. Like photon is both wave and particle it depends on perspective.

@wmertens
Copy link
Contributor Author

wmertens commented Aug 5, 2015

yes but by the time it reaches the store the Intents have collapsed into
Facts :-)

On Wed, Aug 5, 2015, 10:25 Sergey Lapin notifications@github.com wrote:

Actioin actualy used in both ways now, because can be both. Like photon is
both wave and particle it dpends on perspective.


Reply to this email directly or view it on GitHub
#384 (comment).

Wout.
(typed on mobile, excuse terseness)

@sergey-lapin
Copy link

I definetelly don't think so.

@vladap
Copy link

vladap commented Aug 5, 2015

I think that Intents are generally understood as user's intents, they represent user's interaction with UI. Intents are then translated either to events/facts or actions/commands whichever pattern you decide to base your system on. I would rather avoid to use Intent term in any other context.

User intent can be showMeTop10Todos which is translated to TODOS_LOAD_BEGIN and TODOS_LOAD_SUCCESS/FAILURE. User doesn't care about any of them, they are internal events/facts or actions/commands. User has intents and generally cares about displayed results, he doesn't care what has to be done in between.

As @adri mentioned, the common term in EventSourcing is Event or DomainEvent. Event is the common term in Observer pattern as well which is more appropriate seeing TODOS_LOAD_BEGIN and TODOS_LOAD_SUCCESS/FAILURE as what happened and when there is 1:M relation.

FACT term is used in OLAP analytics where it is used for fact tables in a star schema, together with dimension tables. I would say that Fact is more general term representing whatever is analysed, it can be an event, measurements.... I'm not aware it is used directly in some programming design pattern, though I don't know them all.

@vladap
Copy link

vladap commented Aug 5, 2015

User intents are one level higher I would say. User intents are transformed to ActionCreators in React components. We can take a user intent, take the current Redux.state and/or the current local state maintained in React component and decide if we actually execute ActionCreator or not.

The flow is UserIntent (virtual term, represented by DOM event handlers) > ActionCreator > Action object.

@vladap
Copy link

vladap commented Aug 6, 2015

Maybe there should be Four Principles in docs.

State is a result of an immutable past.

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2015

This is controversial as proven by this discussion. We won't prescribe any specific naming scheme in the documentation.

We have also frozen the terminology now, so we're not going to introduce "events", "facts" or something else.

Thanks to everyone for participating ;-). Closing as non-actionable.

@gaearon gaearon closed this as completed Aug 14, 2015
@peteruithoven
Copy link
Contributor

I agree with @Lapanoid that there is very little consensus in this thread.
One of the things I'm missing is opinions on the EventSourcing vs WAL approach, see #384 (comment) and #384 (comment).

Should a smart component dispatch CLOSE_BUTTON_CLICKED (EventSourcing, Observer pattern) or CLOSE_MODAL (Command pattern) actions?
(I'm not talking about the past tense / naun / ... formulation)

Personally I like @jedrichards's points that smart components can translate events into generic/reusable app domain actions. I think this creates a better separation between ui and business logic. When there are multiple events that results in the same actions this also limits the amount of actions.

@wmertens
Copy link
Contributor Author

I think it is quite clear: there are things that happened (observer
pattern: user clicked the like button, server sent new chat message), and
things that you want to happen (command pattern: get user details from
server, show desktop notification) and they are traveling over the same
message bus, the dispatch call.

However.

The only things that make sense to store in the state are observations,
since commands have no direct influence on state. The desired change didn't
happen yet by the time the command reaches the reducers, and the result of
the command will probably be a new observation.

At most, the command can result in the observation that a request was made,
and I believe it would be best to make that explicit by letting the
actioncreator or middleware that brokers the command dispatch that
observation.

Therefore, any action that makes it to the reducers should be written in
the past tense. Anything else is at best a shortcut and at worst a source
of confusion.

On Wed, Aug 19, 2015 at 6:25 PM Peter Uithoven notifications@github.com
wrote:

I agree with @Lapanoid https://github.com/lapanoid that there is very
little consensus in this thread.
One of the things I'm missing is opinions on the EventSourcing vs WAL
approach, see #384 (comment)
#384 (comment) and #384
(comment)
#384 (comment).

Should a smart component dispatch CLOSE_BUTTON_CLICKED (EventSourcing,
Observer pattern) or CLOSE_MODAL (Command pattern) actions?
(I'm not talking about the past tense / naun / ... formulation)

Personally I like @jedrichards https://github.com/jedrichards's points
that smart components can translate events into generic/reusable app domain
actions. I think this creates a better separation between ui and business
logic. When there are multiple events that results in the same actions this
also limits the amount of actions.


Reply to this email directly or view it on GitHub
#384 (comment).

Wout.
(typed on mobile, excuse terseness)

@wmertens
Copy link
Contributor Author

As for the CLOSE_BUTTON_CLICKED vs CLOSE_MODAL, imho the actual action that
should be sent is MODAL_GOT_HIDDEN or just MODAL_HIDDEN, and the
actionCreator could be called hideModal().

On Wed, Aug 19, 2015 at 11:32 PM Wout Mertens wout.mertens@gmail.com
wrote:

I think it is quite clear: there are things that happened (observer
pattern: user clicked the like button, server sent new chat message), and
things that you want to happen (command pattern: get user details from
server, show desktop notification) and they are traveling over the same
message bus, the dispatch call.

However.

The only things that make sense to store in the state are observations,
since commands have no direct influence on state. The desired change didn't
happen yet by the time the command reaches the reducers, and the result of
the command will probably be a new observation.

At most, the command can result in the observation that a request was
made, and I believe it would be best to make that explicit by letting the
actioncreator or middleware that brokers the command dispatch that
observation.

Therefore, any action that makes it to the reducers should be written in
the past tense. Anything else is at best a shortcut and at worst a source
of confusion.

On Wed, Aug 19, 2015 at 6:25 PM Peter Uithoven notifications@github.com
wrote:

I agree with @Lapanoid https://github.com/lapanoid that there is very
little consensus in this thread.
One of the things I'm missing is opinions on the EventSourcing vs WAL
approach, see #384 (comment)
#384 (comment) and #384
(comment)
#384 (comment).

Should a smart component dispatch CLOSE_BUTTON_CLICKED (EventSourcing,
Observer pattern) or CLOSE_MODAL (Command pattern) actions?
(I'm not talking about the past tense / naun / ... formulation)

Personally I like @jedrichards https://github.com/jedrichards's points
that smart components can translate events into generic/reusable app domain
actions. I think this creates a better separation between ui and business
logic. When there are multiple events that results in the same actions this
also limits the amount of actions.


Reply to this email directly or view it on GitHub
#384 (comment).

Wout.
(typed on mobile, excuse terseness)

Wout.
(typed on mobile, excuse terseness)

@sergey-lapin
Copy link

As for me the best idea of this thread is call action nouns without any tense like ITEM_CREATION I am totally fine this this. I think tense itself is the source of confusion.

@ghost
Copy link

ghost commented Aug 20, 2015

I think tense itself is the source of confusion.

I would be more general and say that the main source of confusion is the application of a conflicting mental model.

This truly is a matter of perspective; I can argue for past tense easily because the mental model that is most comfortable for me is Event Sourcing, but I have come to understand the perspective of the other suggestions as well. I think that we should refrain from pushing for a firm stance on this because we must understand that people from different backgrounds will benefit from different explanations that fit their natural mental model. To select one suggestion over another is to make it harder for some even if easier for others. To leave this up in the air "officially", but to tailor the explanation of actions to the specific audience of a blog post, tweet or podcast will serve everyone more directly I would think.

sfentress added a commit to concord-consortium/geniblocks that referenced this issue Jun 20, 2016
reduxjs/redux#384

There is not a consensus on this issue, but it seemed right. "Actions"
could be better thought of as events, they describe a change that has
already occurred. The job of the reducer is to make the state reflect
that change.

Action creators (the functions below) are still written in the imperative.
@reduxjs reduxjs deleted a comment from mathieueveillard Jul 4, 2019
@reduxjs reduxjs locked as resolved and limited conversation to collaborators Jul 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests