“Flux Standard Action” has 3700+ stars on GitHub and used by “redux-promise”, “redux-actions” and other libraries.
Several weeks ago one of my developers tried to switch to FSA approach in one of our projects. After some time of discussion, it was decided that FSA is not what we need.
FSA has good ideas behind it. The main idea is to have a convention about Flux actions shape. It increases compatibility between different libraries and allows to create more consistent flux/redux ecosystem.
I like the idea of having “type” and “payload” fields. But there are some concerns related to error handling.
Everything is about conventions
One of the key ideas of the FSA is “errors as a first-class concept”. FSA says that using LOAD_SUCCESS and LOAD_FAILURE is less than ideal. But in my opinion, both approaches are just conventions. You can have a convention like having an “error” property in an action
or you can have another convention — action status as part of the action type
I call it “traditional approach” in this post
In both cases, you have errors as a first-class concept (on a convention level). If you want more explicit errors then you can use something like ${ACTION_NAME}:${STATUS} instead of ${ACTION_NAME}_${STATUS}. So you will have a special syntax for separating action status.
Let’s look closer at the FSA approach of error handling. It has substantial drawbacks in my opinion.
Double dispatch problem
First of all, it is the double dispatch. We have the first dispatch based on the “type” field and the second dispatch based on the “error” field. It is really painful.
For example, we can have actions that are not going to produce errors. For example, often you do not expect any concrete error for fetching operations (yes, we can have errors while fetching, but almost usually it’s general “Server error”). So, for fetch operations you can omit the error handling and write something like that:
But what if in future you will have an error in your ‘LOAD_ALL_TODOS’ action. In this case, your action will go the wrong way (for the success path) which can cause unpredictable behaviour.
So, you see that and decide that the best way is to always check the error flag for every action.
And you rewrite your code to:
So, we should always have an additional if statement. And now it looks like this annoying best practice about using obj.hasOwnProperty(key) in for in loops. That inconvenient. You will have this if statements everywhere for every action, even for a button click. You are never going to have an error in this button click action but you must check for error for the consistency with the rest of the code. Otherwise, someday someone will forget this if check in another action (I believe that this will happen in any case).
Moreover, we’ve introduced a new bug occasionally. If we have an error in any of the supported action types, the reducer will return “undefined” (as it will not match the default state). So, we change the code to:
But why do I need all of this? FSA actions are compared to promises but why I handle errors like in continuation style?
Compare the above code to this one:
FSA action handling has higher cyclomatic complexity than the traditional approach.
Progress handling problem
The second issue with FSA errors approach. Is that we think that action can be only in two states (Why at all we are talking about the state of the actions???). FSA is compared to a Promise in the docs. But the problem is that any promise has more than two states. As they represent async values, they can be in an unfulfilled state. Promises represent a result of the async execution. In FSA we think about actions as a result of execution but it’s always in the final state — success or failure, we do not have an unfulfilled state.
In a traditional approach you can have ‘LOAD_ALL_TODOS:SUCCESS’, ‘LOAD_ALL_TODOS:FAILURE’, ‘LOAD_ALL_TODOS:INPROGRESS’ (or ‘LOAD_ALL_TODOS:REQUEST’ ).
I cannot do this with FSA. Of course, we can define another action like “LOAD_ALL_TODOS_INPROGRESS” but in this case:
- We switch back to the traditional approach
- It does not make any sense to have an error flag for “LOAD_ALL_TODOS_INPROGRESS”.
- It contradicts with the idea of having one action type for representing loading of all todo items.
Handling all errors in the global sequence of actions
With FSA it is easy to track all errors in your app.
But you can do the same with the traditional approach:
Flux actions are not promises
I believe that it is incorrect to think that every action is a result (with an error or not) of execution of some operation (that we run). For example, you can have actions coming from UI events like buttons clicks (it is unusual to get a failed button click). We can have streams of actions from a server (though WebSocket connection) containing notifications. Notifications can be about failure but actions itself should not have error:true. And don’t say, that everything is the result of execution of some operation, it will be too general :)
“Flux actions can be thought of as an asynchronous sequence of values”. Yes, absolutely. But we do not need an additional level of asynchrony at the action level. We do not want to make every action represent an asynchronous operation, we don’t want to make every action the result of execution of run operations. So, it is ok to have actions without additional statuses like :SUCCESS, :FAILURE or :INPROGRESS. We do not need :SUCCESS for a button click.
One more thing that I do not like is that action shape depends not only on the action type but also on the flags inside the action (“error” flag) which is not a real problem but IMHO increases variability and complexity.
I like the ideas behind “FSA” but I believe that we should move forward towards “FSA v2” :).
Originally published at blog.webbylab.com.
