File states/effect outside worth it?
Answered
Siberian posted this in #help-forum
SiberianOP
Hey there, is there a good or bad practice to have exported just useStates and useEffects for a component that might be ~200-400 lines
i.e.
i.e.
function useThoseStates(someProp1,someProp2){
const [state,setState] = ("state")
const [state1,setState1] = ("state1")
const [state2,setState2] = ("state2")
const [state3,setState3] = ("state3")
const [state4,setState4] = ("state4")
useEffect(()=>{doSomethingWhenValuesChange},[state])
return {state,setState,state1,setState1,state2,setState2,state3,setState3,state4,setState4}
}
export default function Component(){
const {state,setState,state1,setState1,state2,setState2,state3,setState3,state4,setState4} = useThoseStates("some","thing")
///rest of the code
return <div />
}Answered by josh
basically you want to aim for only putting the minimum in state and derive everything you can
38 Replies
You might find this useful: https://react.dev/learn/reusing-logic-with-custom-hooks
also look at
useReducer@josh You might find this useful: https://react.dev/learn/reusing-logic-with-custom-hooks
SiberianOP
well I have like 8 states that goes for a single form, 2 inputs with button, state for inputs, state for its labels (changes when user enters incorrect value) etc. which is in my component that I use in different places
here is my current code
function Test(
pending: boolean,
errorLabels: { origin: boolean; destination: boolean },
setErrorLabels: Dispatch<
SetStateAction<{ origin: boolean; destination: boolean }>
>
) {
const [originLabel, setOriginLabel] = useState("Origin");
const [destinationLabel, setDestinationLabel] = useState("Destination");
const containerRef = useRef<HTMLDivElement>(null);
const [filterCountries, setFilterCountries] = useState<string>("");
const [origin, setOrigin] = useState<string>("");
const [showList, setShowList] = useState<boolean>(false);
const [destination, setDestination] = useState<string>("");
const [currentTarget, setCurrentTarget] = useState("");
const handleClickOutside = (e: any) => {
if (containerRef.current && !containerRef.current.contains(e.target)) {
setShowList(false);
}
};
useEffect(() => {
document.addEventListener("mousedown", handleClickOutside);
if (errorLabels.origin || errorLabels.destination) {
setOriginLabel(
errorLabels.origin ? "Please select origin" : "Origin"
);
setDestinationLabel(
errorLabels.destination
? "Please select destination"
: "Destination"
);
setOrigin(errorLabels.origin ? "" : origin);
setDestination(errorLabels.destination ? "" : destination);
setErrorLabels({ origin: false, destination: false });
}
if (!pending) {
setOrigin("");
setDestination("");
}
return () => {
document.removeEventListener("mousedown", handleClickOutside);
};
}, [errorLabels, pending]);
return {
originLabel,
destinationLabel,
containerRef,
filterCountries,
origin,
showList,
destination,
currentTarget,
setOrigin,
setShowList,
setFilterCountries,
setCurrentTarget,
setDestinationLabel,
setOriginLabel,
setDestination,
};
}I will read that page in 30min anyway
You can only use hooks inside another hook or component. That function is neither
If your problem is "I have too much state at once", then either split it into smaller components or use useReducer like suggested above
If your problem is "I have too much state at once", then either split it into smaller components or use useReducer like suggested above
SiberianOP
okay, I thought I could have some heavy lifting in my second function, and main stuff in that, but that was propably silly
sure, that makes sense, it's just that react has some specific rules about where you can use hooks (for good reason) https://react.dev/warnings/invalid-hook-call-warning
SiberianOP
Well Im trying to achieve my goal where all components are easily readable, 7 useStates, 3 useRefs, 1 useFormStatus, 1 useEffect and 4 functions doesnt sound too bad, but its only 2 html inputs, 1 button and 1 div with a list of elements (different component)
Looking at your code
A lot of your states can be eliminated
And just derived
It looks like you’re abusing state when just a regular variable would do
SiberianOP
its changing DOM, so went with useStates
Show list is the only state needed.
SiberianOP
and how would I change
input label on button click when form validation said its incorrect? Last time I tried any regular variable wasnt changing DOM valuesSee if you can derive it
SiberianOP
you mean object in state?
Also your use effects having dependency arrays which are prop values is a code smell
Your component will always rerender when props change anyways
Adding a use effect isnt necessary
Instead refactor them to just assign a value to variable based on your logic
Rather than updating state
@linesofcode Your component will always rerender when props change anyways
SiberianOP
not by default 😅
if the props aren't causing your component to update then they wouldn't trigger the useEffect to run either
do you know what @linesofcode means when he suggests you derive everything except showList?
SiberianOP
not really know what derive means
@josh if the props aren't causing your component to update then they wouldn't trigger the useEffect to run either
SiberianOP
well it worked with useEffect thats why it was there 😄
Also tried to pass prop directly to a label of input, it worked just fine, but it didnt reset (errorLabels or label text) on input focus
here's an example
const [age, setAge] = useState(-1) // we put the age in state because we need to get it from input and keep hold of it, and re-render the component then setAge is called
const isOver18 = age >= 18 // this is derived from age. we don't need to keep it in state because every time the component re-renders after setState is called, this assignment will as wellbasically you want to aim for only putting the minimum in state and derive everything you can
Answer
as a more concrete example from your code:
// outside useEffect
const originLabel = errorLabels.origin ? "Please select origin" : "Origin"as long as
errorLabels is being updated using setErrorLabels() in the parent component, originLabel will be updated tooif that's not happening - you might be trying to update
errorLabels by mutating the object instead of creating a copy with the updated values, eg setErrorLabels({ ...errorLabels, origin: true})SiberianOP
Works, now I need to reset errorLabels on input focus and get rid of useStates as much as possible
great, you can mark with resolved with
Right click on message -> Apps -> Mark SolutionSiberianOP
I think 5 useStates will be better than 7, but with those I might not be able to get rid
-currentTarget setting filter based on value of origin/destination depending which was focused last
-showList to render list if originRef or destinationRef was focused
-origin and destination as im using MUI and need value for that (so label is moving to the corner)
-filterCountries so it filters search by origin value or destination value or list element clicked at child component
Thanks anyway
-currentTarget setting filter based on value of origin/destination depending which was focused last
-showList to render list if originRef or destinationRef was focused
-origin and destination as im using MUI and need value for that (so label is moving to the corner)
-filterCountries so it filters search by origin value or destination value or list element clicked at child component
Thanks anyway