Sorry, for missing this until now. But, you are absolutely correct in changing it. At the end of the day, it does not actually change anything, because, even though one should not mutate state, a new state will be returned. So, they both accomplish the same thing, but not mutating is better.
Mutation is discouraged because it generally breaks time-travel debugging, and React Redux’s connect function:
For time traveling, the Redux DevTools expect that replaying recorded actions would output a state value, but not change anything else. Side effects like mutation or asynchronous behavior will cause time travel to alter behavior between steps, breaking the application.
For React Redux, connect checks to see if the props returned from a mapStateToProps function have changed in order to determine if a component needs to update. To improve performance, connect takes some shortcuts that rely on the state being immutable, and uses shallow reference equality checks to detect changes. This means that changes made to objects and arrays by direct mutation will not be detected, and components will not re-render.
Other side effects like generating unique IDs or timestamps in a reducer also make the code unpredictable and harder to debug and test.
Well, now you have peeked my interest in the subject. From what I understand (very little), altering the state within the reducer should not cause anything to break, as the input to the dispatcher is measured against the output of the reducer. So, anything inbetween is moot?
I agree that it should be changed for clarity, but I don’t think that it’s mutating state. Correct me if I’m wrong, but since it is a primitive, it is being passed in by value and a local instance of that primitive is created. Then that value is returned and put onto the state. I think that that is what is being changed. I think that both solutions do the exact same thing and neither mutates state, I do agree that state += 1 is unnecessarily confusing.
That was my initial assumption as well. No real mutation is taking place because of the value type being a primitive. I just don’t know enough about how Redux, it’s tooling, or middleware works behind the scene to fully say that reassigning the state can’t cause any issues.
But the idea behind making this change was just to make sure we are not teaching anything wrong or showing anti-patterns. And as you said, the reassignment seems unnecessary, if nothing else.
Yeah, my understanding of redux is that both will work fine because you are not mutating the state because you are returning a whole new state. The fact that it is a primitive means that it is impossible to mutate. But yeah, the += is confusing because it makes it seem like you’re mutating state.