-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Prop Drilling vs. Redux #824
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
Comments
it would be great if you did some of this refactoring! when i started working on this project, redux and react were totally new to me and i did the best i could. don't feel like you have to do a ton of refactoring to make a PR—i think this can be done in small stages! |
Rather than using redux ,we have good support for context api now (with react hooks too), redux adds a lot of complexity and we can use something like https://github.com/jamiebuilds/unstated for state management |
If you are still with redux, I can also help refactor this @catarak @meiamsome can you help me get started with it? |
I'd prefer to keep redux for now. I know hooks are cool and all but I think there are certain technical updates/cleanup things that are more important for this project. |
Nature of issue?
Hello,
I was wondering why there is plenty of prop-drilling from the root-level components down, even when sub-components are connected to redux. For example IDE and Nav components are both connected, but IDE passes down a lot of the properties manually. This leads to very verbose code that can be hard to read in my opinion and means that I spend a lot of time going up and down the hierarchy following props when they just come from redux in the end. There are also some performance considerations where a change of state will re-render all the components down from the route
connect()
call to where the prop is used, rather than just the section that needs re-renderingFor the Nav item, I have made an example refactoring here: meiamsome@89f850c
I would be happy to PR some similar refactoring for other components if you think that makes sense.
The text was updated successfully, but these errors were encountered: