-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Add default handler in createReducer #156
Comments
Currently, there is no such feature yet, we would need to extend the API to handle this case. Proposed API: const loadingReducer = createReducer(initialState)
.handleAction([action1, action2, ...], (state, action) => {
return true;
})
.defaultHandler((state, action) => {
return false;
}) Recipes:
const rootReducer = combineReducers({
session,
organization,
organizationAccount,
});
export type RootState = StateType<typeof rootReducer>;
export default createReducer(undefined as RootState)
.handleAction(signOut, (state, action) => {
return rootReducer(undefined, action);
})
.defaultHandler((state, action) => {
return rootReducer(state, action);
})
}; |
@issuehunt has funded $80.00 to this issue.
|
Does the following scenario make sense?
So you either define the reducers for all the actions or for some of them + the default one. |
@vladimiry |
The benefit is that defining the reducers for all the actions would be enforced by TypeScript. But I understand that would be an opinionated thing as not all the dev teams prefer this way, probably the minority. |
@vladimiry I agree that might be useful in some cases but it seems too opinionated for a default. To get enforced exhaustive checking I would propose to create another issue as I don't want to expand the scope of this feature. Proposed solution could be a new API like |
@piotrwitek |
@McTano I understand your point although you could very easily fill the gaps by extending RootAction type and also this library is heavily opinionated about type-safety as a number one goal. Secondly, the reason for a compilation error is that action in default case would have a type of remaining unhandled actions (all actions - handled actions). |
I agree that the correct type for the action in the default case should be the union of the remaining unhandled actions, and in some cases that would be never. But, and correct me if I'm wrong, I don't think that would cause a compilation error on its own. In the case that the defaultHandler takes a function of type Then the natural way to do a visual check for exhaustiveness is to look at the inferred type of the action argument to the handler function. If it's never, you've covered all the cases. EDIT: |
@piotrwitek what would it take now to implement the defaultHandler in the way you've described? Ironically, I want to achieve the exact example you've provided. |
I've gotten around this recently (at least the sign out problem) by implementing the root reducer pattern that Dan Abramov suggested here. I'm not sure if I'm missing something, but the following seems to work for me:
The clearOnSignOutReducer could handle any number of actions I guess. But I suppose, to the questions point, you'd have to hoist the particular default action up to the top level, rather than have it handled by the local reducer. |
@chrispaynter, unless I'm not mistaken, this requires all your reducers to accept |
Ah right yes, got you @marinintim. One way I've gotten around this in the past is to export the default states of all reducers. Bit more effort (and you have to remember to add new reducers), but did the job (a few years ago!). After a while, new reducers don't get added so much anyway. Could get you out of the woods at least until something like the above is added to the library.
|
Just started work on it. Will open a PR soon. |
How do I create a default action handler. For example loadingReducer should set
loading
bool to true only if it receives specific actions, otherwise I want to make it falseIssueHunt Summary
Backers (Total: $80.00)
Submitted pull Requests
Become a backer now!
Or submit a pull request to get the deposits!
Tips
The text was updated successfully, but these errors were encountered: