-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Logic in actions mix of concerns #7
Comments
Maybe this particular example isn't good. We usually use By the time action is plain object and reaches a reducer, it's just a description of what happened. We found it useful to also be able to express intention when dispatching, even if such “async action” transforms into side effects and produces one or more actions, potentially asynchronously, later. You need to put your side effects somewhere. We suggest to dispatch “async actions” which are handled by “middleware” and turn into proper “actions” at some point. You may do side effects differently—it's up to you. But we find this mechanism is convenient. Does this clarify anything, or do you still disagree? Please propose how to express http://gaearon.github.io/redux/docs/recipes/ReducingBoilerplate.html#async-action-creators in a clean manner without the concept of “async actions” and a helper like |
I'm not sure what you mean by “component receiving action”. Components can't receive actions. They only receive the current state. Reducers receive actions. |
Disregarding the “component receives action” misconception, what you describe is convenient to implement with function addTodoWithoutCheck(text) {
return {
type: 'ADD_TODO',
text
};
}
export function warnTooManyTodos() {
return {
type: 'WARN_USER',
message: 'Too many todos.'
};
};
}
export function addTodo(text) {
return function (dispatch, getState) {
return (getState().todos.length === 3) ?
dispatch(warnTooManyTodos()) :
dispatch(addTodoWithoutCheck(text));
}
}
} Yes, this is domain logic, but it's not “mixed” with actions. It is interpreted by middleware (in this case, You could move it somewhere and call it something else, but it's just too convenient to be able to “dispatch” these things, whether you call them “async actions”, “instructions”, or something else. |
Finally, yes, you should strive to put your logic into reducers whenever possible. But sometimes it's just too much pain, or involves side effects. That's when you'd use |
Thanks for the quick response time @gaearon. I think the conceptual problem I'm having was clarified by this:
If I were the architect, I would move these side effects into some other contraption that is not middleware. Middleware (to me) seems best suited to orthogonal devices such as logging, error handling, and other interceptions, and should not be responsible for any domain logic or feature-related side effects. Given that you have only one opportunity when initializing a redux app to create a custom dispatcher via In order to take advantage of the event-sourced architecture of redux we need simple object actions pumped through our reducers. Generating these actions via middleware seems convoluted for reasons listed above. Perhaps there can be a second mechanism for handling a logic-laden (or async) action before it ultimately dispatches a synchronous, simple action object. For sake of the example let's call this the domainer. Imagine you have a view component trigger Now that I have typed this all out and waded through my own thoughts, I think what I'm missing in Redux is a dedicated state machine. Reducers provide simplicity from (state, action) -> newState, but in reality those switch statements are just begging for a dedicated state machine implementation. The domain logic hidden behind switch statements and if/then blocks could be much more explicitly stated in the form of This is all crazy talk and hypotheticals, but I appreciate your feedback. I think I should stop trying to impose my design thoughts onto a differently laid out architecture. Feel free to close this as not really an issue, more of a discussion leading nowhere. |
That's an interesting discussion, and I'd appreciate if you could whip up a proof of concept of your proposed system for side effects and created an issue in the main Redux repo. We're not dogmatic, it's just that this is the best we have at the moment. |
Related: reduxjs/redux#351 But at this point, indeed, discussion without a proof of concept goes nowhere. |
You might be interested in Redux Saga project which moves side effects to generator-driven "sagas": reduxjs/redux#1139 |
After reading through the conceptual ideas behind redux I have one particular issue with regards to actions. It makes sense to me to use action creators as, if for no other reason, a means to maintain consistency. The problem I have, however, is when you start talking about middleware thunks applied to actions. This seems like a gross mixing of concerns, particularly if you see actions in a more familiar/typical role in an evented or messaging architecture. Should it not be that whomever receives the actions enforces the domain logic of (per the example here) "don't allow more than 3 todos"?
This may be nitpicking, but I am of the belief that mixing "something happened" events with "should we send this event?" is a crossing of concerns and will run you into trouble down the road. Perhaps one component really wants to know each and every time a new todo is added (maybe for displaying a "too many todos" error) while a second component will receive the action, but follows its own logic to disregard that action because it reads that there are too many todos already. This argument extends to the numerous other examples of decorating actions with domain logic. Maybe I'm missing the point, or thinking about this in the wrong paradigm, but it seems very limiting to have your actions responsible for performing tasks like fetching, updating, emitting more actions, etc. versus simply firing an action and letting some other components deal with that event.
It just overall seems like a bad idea to offer this action creation decoration as an example of thunk middleware.
The text was updated successfully, but these errors were encountered: