-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
mapStateToProps and mapDispatchToProps looks to me like a viewModel #321
Comments
I don't think they can be combined. Would you remap the actions to their same properties each time the state changes also? mapStateToProps and mapDispatchToProps are separate for a good reason, consider the performance: mapStateToProps is actually run several times when state changes, and mapDispatchToProps once (or way fewer anyway than mapStateToProps) it doesn't depend on the state. I won't start to fight about naming however. |
Yeah, the separation exists for performance reasons:
Please see #1 for a history of how this API came to be. The single function approach was considered before and rejected. |
Makes sense. Naming seems fine to me then. |
Add missing paren
ViewModel maps model (state) into props for rendering and can include commands. It might be benefitial to keep established terminology. It might as well simplify API into a single function. I assume that viewModels are small hence the separation into two function is a boilerplate which could be reduced:
const viewModel = (state, dispatch) {
todos: state.todos,
onTodoClick: id => dispatch({...})
}
const TodoList = connect(viewModel)(TodoList)
The text was updated successfully, but these errors were encountered: