-
Notifications
You must be signed in to change notification settings - Fork 6
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
Manage global state #286
Comments
think about it. This app was built mainly by Jr Devs (in different stages). Not by a well stablished team (usually they know what to use and how). Nevertheless, non of the Jr devs had any problems taking over, bug fixing and adding new features... At some point I've tried to introduce Redux (Only for the admin page) and it was a disaster. Jr devs could not understand it and everything became slower... the solution was to remove it... |
Yeah i think redux might be more than we need. I think its probably just doable incrementally, there are a number of pieces (like the routes) that we can just split out over time. |
I agree redux is a bit much, but some things can be cleaned up quite nicely if we maybe stored some of the values on context instead import React from "react";
import { Route } from "react-router-dom";
export default function ModeratorRoute({ children, path }) {
const state = useGlobalState();
const isModerator = state.user.isModerater; // could also have a "selector" that does this
return isModerator && <Route path>{children}</Route>;
} |
I've spent a bit of time recently looking at the main
App.js
file and how we could best break it down further so that it's not >900 lines and super difficult to understand. I think one thing that might unblock this is some form of global state management. I think for now Redux might be a little more than the app needs right now so we could try just using hooks and context, where we could set it up in a modular pattern so that if the app grows further and a redux/another state management library is needed it's fairly simple to roll it outWhat are peoples thoughts? @sebastianovide @neilfulwiler
The text was updated successfully, but these errors were encountered: