bitovi / react-pmo

https://bitovi.github.io/react-pmo/
0 stars 1 forks source link

Document decisions for the React PMO app. #4

Closed rlmcneary2 closed 2 years ago

rlmcneary2 commented 5 years ago

This will be a "perfect" React app that will be used to drive the Bitovi Academy React course.

Why?

  1. App.js is in the src directory because that is where CRA creates it (as opposed to the components directory.
  2. Restaurant data is fetched and cached because it is not expected to change during a user's session.
  3. Data to be shared among components (like restaurants) is centralized in ~appContext~ a RestaurantProvider component paired with a useRestaurants hook to reduce prop drilling.
  4. Remote data that is used by a component and its (immediate?) children is fetched by the component in a useEffect hook (or if appropriate a Provider/Hook pair like above).
  5. Context includes

Discuss

  1. Create contexts by domain? Is it OK to have a single appContext? Keep appContext but split up if it gets "too big?" Keep appContext and create hooks by domain?
  2. Service or models? Are they the same thing just different names? Right now services wrap fetch requests and do some error and data modification.
  3. (5/30) If a Component needs to update Context it will do so using a function returned by the associated Hook? The Hook may return an object with multiple properties including data and functions.

React Best Practices

These are Bitovi recommendations when working with React.

Todo

  1. Remove Service Worker from index.js
  2. Create a useRestaurants Hook that makes use of ~appContext~RestaurantProvider Context.
  3. Order Components.

Terms

christopherjbaker commented 5 years ago

Even though App is just a component, I like to keep it at the top level because it is used differently. I also like to make my components folder only those that are reused, and put page-type components into a pages folder.

I dislike the centralized state that Redux encourages, so I would prefer to separate it (in this case, probably by data type), but if the decision is to go with centralized data, then so be it. I would probably call it something like "state" (like redux does, I think), and expose an actual API instead of just a raw context.

BigAB commented 5 years ago

Discuss

  1. Create contexts by domain? Is it OK to have a single appContext? Keep appContext but split up if it gets "too big?" Keep appContext and create hooks by domain?

I like the idea of Provider/hook pairs, where Provider components fetch and store data and expose it via hooks, isolating the context from the components. If it is simple enough, I think fetching data right in the component with useEffect() could be fine as well.

  1. Service or models? Are they the same thing just different names? Right now services wrap fetch requests and do some error and data modoification.

I don't think services and models are the same thing myself. I like the way the services are just classes/instances/singletons that get data from "somewhere' and I think if the Providers and Components consume services that seems pretty good to me.

RE: App outside of the components dir

I don't see the point of having it outside, anything you might want to do in "App" that is special could just be done in the index.js? (...other than CRA does that, which is actually a pretty strong argument for me to just roll with it)

Main question: Why have both index.js and App.js?

rlmcneary2 commented 5 years ago

Context, Hooks, State I guess I'm in the minority - I don't dislike redux. I do find it annoying to create initially. I like having a single state object: it's easy save and restore the state object when the application closes and resumes, there is a mechanism for updating state where domains overlap (i.e. the same action can be consumed by more than one reducer to affect state), separating state management from the view layer (I have worked on applications where state is updated outside of React so it requires a non-React way to interact with it).

I still love Hooks and Context, though I would tend to try and move most of the Hook logic into separate modules because I find Hook code clutters up a function that is supposed to be rendering the view.

I'm good with domain-specific Contexts and accessing them in a Component through Hooks.

index.js index.js and App.js is how CRA works. I suppose it's a way of indicating that non-React app startup behavior goes in index.js and React code goes in App.js. I tried not to modify what CRA produced too much.

BigAB commented 5 years ago

I find Hook code clutters up a function that is supposed to be rendering the view.

Yeah I totally agree, which is why I eventually landed on this idea:

I am starting to believe all non-trivial stateful components should look like this

const StatefulComponent = props => {
 const state = useCustomHook(props);
 return <StatelessComponent {...state} />;
};
christopherjbaker commented 5 years ago

@BigAB I agree with the sentiment, but I think that is too far. I will happily put either the component structure or the hook structure in that. Either make a "container component" that has all the hook logic and returns a single component, or make an external custom hook that is consumed by the full component.

rlmcneary2 commented 5 years ago

I like the container component pattern. It will certainly be familiar to anyone who has used Redux 😉

rlmcneary2 commented 5 years ago

I just added a "Best Practices" section for things that Bitovi recommends for React. OK with that premise? Anyone have suggestions for how to move things from "Discuss" into "Best Practices?"