-
Notifications
You must be signed in to change notification settings - Fork 0
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
WIP to switch connector to hooks #131
Conversation
This reverts commit acb6132.
ui/js/src/hooks.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file towards the end we export:
export const useAppDispatch: () => AppDispatch = useDispatch;
Is this so we get type information when using the hook?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya, with this re-mapping, all Typescript types are automatically inferred and propagated through the app.
More is mentioned in the typescript part of the migration guide here:
https://redux.js.org/usage/migrating-to-modern-redux#modernizing-react-components-with-react-redux
updateTodo, | ||
updateTodoLabels, | ||
}; | ||
const connector = connect(mapStateToProps, mapDispatchToProps); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how using the hooks simplifies all this, and avoids having to pass down these props through the hierarchy.
Although, I was curious about using connect at the root level (before these changes) instead of letting each component taking care of mapping the required fields and actions. Was it to avoid writing the connect logic multiple times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain. I think it was a best practice aiming to have a central connected "controller" component and then individual controlled or UI specific components. I know that made it easier to use tools like Storybook, etc.
I still haven't played w/ the new setup enough, but I suspect I may end up adding back passing data via props in some cases to make components easier to configure in storybook. But for now it's also been very easy to bind storybook to redux w/ an initial state.
I also haven't worked in an app w/ multiple connect setups so I'm not sure if there are any downsides of limitations.
Merge commit: Decorate storybook components individually to fix storyshots |
No description provided.