AlexKMarshall / regMan

CRM to manage registrations to summer camps
MIT License
2 stars 0 forks source link

Resolve missing dependencies in useEffect hooks #6

Open AlexKMarshall opened 4 years ago

AlexKMarshall commented 4 years ago

The useEffect hooks have got lots of issues with missing dependencies.

I expect most of these are used for fetching server-side data, so this probably would be resolved by refactoring to react-query

I think it would be useful to do an audit of all the useState hooks too and see what is actually being stored. Of that, we should see which of them are concerned with purely UI state (contents of forms, modals being open, etc), and which of them are concerned with server side state. The server side state should almost certainly go into react-query. Then with what's left, we should see whether it makes sense to put that into redux or similar, or if it makes sense to just keep that as simple component state (or maybe some context).

./src/components/ParticipantDetails/PersonalDetails/PersonalDetails.js
  Line 11:6:  React Hook useEffect has a missing dependency: 'setDisplayEdit'. Either include it or remove the dependency array. If 'setDisplayEdit' changes too often, find the parent component that defines it and wrap that definition in useCallback  react-hooks/exhaustive-deps

./src/components/Dashboard/GroupsDisplay/GroupsDisplay.js
  Line 143:6:  React Hook useEffect has a missing dependency: 'generateData'. Either include it or remove the dependency array  react-hooks/exhaustive-deps
  Line 147:6:  React Hook useEffect has a missing dependency: 'chart'. Either include it or remove the dependency array         react-hooks/exhaustive-deps

./src/components/ParticipantDetails/PaymentsDetails/PaymentsDetails.js
  Line 20:6:  React Hook useEffect has a missing dependency: 'setDisplayEdit'. Either include it or remove the dependency array. If 'setDisplayEdit' changes too often, find the parent component that defines it and wrap that definition in useCallback  react-hooks/exhaustive-deps

./src/components/Dashboard/ListParticipants/ParticipantList/ParticipantList.js
  Line 23:6:  React Hook useEffect has a missing dependency: 'search'. Either include it or remove the dependency array. You can also replace multiple useState variables with useReducer if 'setSearchedParticipants' needs the current value of 'search'  react-hooks/exhaustive-deps

./src/components/Dashboard/Dashboard.js
  Line 31:6:  React Hook useEffect has a missing dependency: 'getAccessTokenSilently'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

./src/components/ParticipantDetails/ParticipantDetails.js
  Line 46:5:  React Hook useEffect has missing dependencies: 'courseStarts', 'getAccessTokenSilently', and 'id'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

./src/components/ParticipantDetails/HealthDetails/HealthDetails.js
  Line 9:6:  React Hook useEffect has a missing dependency: 'setDisplayEdit'. Either include it or remove the dependency array. If 'setDisplayEdit' changes too often, find the parent component that defines it and wrap that definition in useCallback  react-hooks/exhaustive-deps

./src/components/ParticipantDetails/ParticipantDetails.css (./node_modules/css-loader/dist/cjs.js??ref--6-oneOf-3-1!./node_modules/postcss-loader/src??postcss!./src/components/ParticipantDetails/ParticipantDetails.css)
AlexKMarshall commented 4 years ago

These errors break the build in production mode, so I'll need to fix them now

AlexKMarshall commented 4 years ago

Looks like some of these dependencies are missing because they were getting into an infinite loop, and useCallback should have been used.

See the description of this issue here https://youtu.be/-Ls48dd-vJE?t=341

Essentially if useEffect relies on a function that is declared within the component, but not within the useEffect callback itself, then useEffect will run every render, as that function will be redeclared every render.

To stop this, the function needs to be wrapped in a useCallback hook.

AlexKMarshall commented 4 years ago

This is too big of a job for now, there are circular references in the GroupDisplay.js file because the state is not being managed properly.

State is created from props, which shouldn't be done, and derived values are also getting stored in state. It needs a complete rewrite.

This probably shouldn't be done until the server side state is put into a proper library, so parking this.