Closed myrjola closed 5 years ago
Thank you very much for the contribution! I especially appreciate the added tests and documentation.
However, I'm not convinced this is something that's good to support because it creates couplings between different parts of a codebase that aren't obvious. Here are some potential concerns if I had code like the above:
At a later time, a different developer (or the same developer months later with a hazy memory) creates a new action related to user accounts, say setUsername
. This developer might add this new action as a case to userAccountReducer
, and they would have no idea that doing so would also cause setting the username to reset the user data to the initial state, which is probably not what they want. The only way the second developer could know that this would happen is if any time they add or remove an action to any reducer, they search the full codebase to see if getActionCreatorsUsedInCases()
is called on that reducer anywhere.
In the same scenario, even if the developer realizes that changing the userAccountReducer
behavior will implicitly change the userDataReducer
behavior as well, there's no convenient way to make the change they want (to have data reset on signIn
or signOut
, but not on setUsername
) by extending the existing code. They will need to delete the call to getActionCreatorsUsedInCases()
and explicitly specify the actions they want. Whenever possible, I would prefer convenience methods that extend gracefully as requirements change.
Suppose my program has a bug when the sign out button is pressed. To investigate, I do a search of my codebase for references to the signOut
action and find that it's only referenced once, in the definition of userAccountReducer
. It would be very reasonable for me to assume that the bug is therefore in that file, which could be incorrect if the action is implicitly referenced elsewhere bygetActionCreatorsUsedInCases()
.
I'm open to hearing your thoughts. Does that make sense?
Thanks for your thorough comments. I agree with all your points and will abandon this PR.
In my case, I was worried that the developer would forget to add the newly created action to the reducer that should reset its state when the new action is dispatched. At first sight, it seemed easiest to just extract all the actions used in a reducer with something like the proposed getActionCreatorsUsedInCases
.
After reconsidering my options, I realized that I can move the userDataReducer
out from Redux into a React component's internal state. Thus, that components just needs to listen to changes to the user account state to know when the internal state should be reset.
Returns an array of the action creators that are used in the reducer chain. This list is useful, e.g., when you want to reset the state in another reducer when any of the actions affecting the state in the current reducer are dispatched.