refactor: extract state to contexts to reduce prop drilling#800
refactor: extract state to contexts to reduce prop drilling#800lucaengelhard wants to merge 22 commits intoCyberTimon:mainfrom
Conversation
…and SelectedImage to context
|
@CyberTimon does this fit what you imagined for reducing prop drilling? the handlers can now also largely be removed from the App.tsx component. And maybe then the components themselves could also be extracted so that the main component isn't that long. Maybe for a first step the extracted contexts and methods can be merged and the modified components come later. that way the different functions can be refactored step by step |
|
@CyberTimon no stress, but what is your usual timeframe and routine for reviews and feedback? so that i know how long to wait :) I would love to contribute more! |
|
Hey @lucaengelhard, thanks for the contribution and sorry for the delayed response! I appreciate the effort you've put into this. I'm in a bit of a tough spot with this one. The reason I can develop this project as fast as I do is because I heavily rely on language models during development. These models need access to most of the code at once to understand the relationships and context between different parts of the application. Splitting almost every function into its own separate file would actually kill a lot of my development speed and make the project much harder for me to maintain going forward. The current structure, even if it's "cluttered" by traditional standards, works well with how I build and iterate on this codebase. But I definitely do see the value in reducing prop drilling (that's why I also put it in the readme todo section) and I'm not against improvements in that direction. I just need to find a balance that doesn't fragment the codebase into 80+ changed files where context is spread across dozens of small files. Could we maybe find a middle ground? For example, extracting just a few key contexts without going as far as splitting every piece of state and every hook into its own file? Something that cleans things up a bit but keeps enough code co-located that the overall structure remains easy to work with? Thanks so much!! |
|
Hey @CyberTimon yeah I understand that llms need all the context and thats easier to access in a single file. But at some point I think it will get super unmaintainable and its already hard to get into developing and contributing because its super unclear which part of the code affects what and what state depends on what. So maybe this is a more foundational question than just prop drilling, because I think a general refactor that makes a lot of components more generic and makes the control and information flow clearer would be really beneficial for accessibility for other contributors and probably also for yourself in the future. Idk what the right answer is tbh, because I'm not that into language models so I can't give you an answer on what would be best for that workflow, but I think that overreliance on them could hurt the project in the long run as it gets way to cluttered to maintain |
|
At a point, maintainability does have to be prioritised over development speed |
|
Agents can easily handle a context instead of the current single file structure too now! Especially with a solid AGENTS.MD and/or some internal docs. |
This. I would love to contribute to the project but in it's current state it is almost impossible to work with. I started multiple tries to implement new things or refactor code but always ran into the wall of convoluted code that isnt meant for humans to read and modify. I would be willing to contribute to a restructuring of the codebase but don't want to start that sisyphusean task without a clear decision from @CyberTimon |
|
Same, there's a lot of things I've avoided because I'm waiting for this to be finished up and merged. Really should be a priority before too much other stuff gets done. |
+1 |
|
Hey @lucaengelhard, I've been thinking about this a lot recently and reading through the feedback from you all. Since RapidRAW has now reached a stage where most of the core features are implemented and working reliably, I am definitely on board with moving forward with some refactoring. Let's avoid putting every single function or tiny hook into its own isolated file. Instead, I'd prefer we group related contexts, states, and methods together logically so the codebase doesn't fragment too heavily. Also, reviewing 80 changed files in one go is pretty hard... It would be much easier for me to review and merge if we break the massive refactor down into smaller PRs that target individual groups or specific features one at a time. What do you think about this? |
|
@CyberTimon we will definitely need one larger PR to setup the context architecture however you prefer, React Context, Zustand, etc. along with some of the core logic moved there. Then from there if you can add some guidance or docs about how the context structure is set up we could move it over gradually and make sure any new features use that method :D |
|
Started with the backend here: 3151fd7 |
|
What should we use for the frontend? My experience with all these librarys is very limited... |
|
Zustand is my personal rec, lightweight and pretty minimal. Native react context is not as great for things that are being written/read as often as this app does (lots of re-renders).
Might look something like this: |
|
Hey @CyberTimon I'm really happy you decided the App is ready for some refactoring and I agree that one big PR is really difficult to review and I'm even unsure if my approach was the right one. But if @Flohhhhh is experienced in zustand and can maybe start some refactors that would be a great start! But maybe it would be an idea to first analyze and maybe draw up the structure of the app to see how it is layed out and maybe where code duplication can be reduced and modularity can be increased. idk what the right approach for that would be. |
|
Slowly moving into the right direction: df78568 Thanks for all the inputs! I'm now going to seperate the hooks... |
|
Part 2: e74e77a |
|
Part 3: 0e8029b |
|
App.tsx lost over 3k lines this evening :). Will continue in the next days. |
Description
Currently the App.tsx file of the frontend is very cluttered and instantiates a lot of useStates, useEffects and functions that are used globally, but mainly within subcomponents. This PR extracts all of them out into contexts and hooks so that they can be used globally without having to pass them as args.
Type of Change
Changes Made
Screenshots/Videos
Testing
Test Configuration:
Checklist
Additional Notes
AI Disclaimer:
Please state the involvement of AI in this PR: