top | item 37133309

(no title)

ElKrist | 2 years ago

Like others I disagree that you have to wrap _everything_ in useCallback/useMemo.

However saying you only need those for performance reasons is wrong.

There are cases where avoiding re-rendering (thanks to useCallback) is avoiding an infinite loop.

I created a codesandbox[1] to illustrate this. Wrapping the "reset" function in a useCallback solves the infinite loop.

If your initial reaction is: "you should not create logic like this" or "you're adding unnecessary stuff" please note that this is a stripped down version of a real use case, where things make more sense.

The point is that it's hard to come up with a rule as to when to use useCallback. The best I can think about is: "if you don't have direct vision on where your function will be consumed, wrap it in useCallback". For example, when your function is defined and returned in a hook or defined in a parent component and given to children.

The point is that any of those children/consumers could have this function in a useEffect and so list it as a dependency of this useEffect.

[1]: Warning, clicking "Start" creates an infinite loop in your browser. https://codesandbox.io/s/intelligent-rgb-6nfrt3

discuss

order

pcthrowaway|2 years ago

This example doesn't resemble anything someone would actually write though.

There's no reason to reset before setting data again, and I'm not sure why you'd even consider putting the functions in the dependency array for the useEffect in usePets.

I can imagine reasons you'd want to setData in a useEffect (maybe when something else changes, or the user performs some interaction, you fetch new data and then set it), but the dependency would be the thing that indicates that action has happened, and not a reset function returned by a custom hook coupled with your data and setter function.

ElKrist|2 years ago

I tried to address your type of reaction in my "If your initial reaction is(...)" sentence, but that failed.

The point is not that this code is good or bad. It's that it is possible to write such hard to predict code.

Remember this is a stripped down version of some code running in production.

Now to address your specific points anyway: "There's no reason to reset again": in the non-stripped down version, the hooks rely on a third hook (let's call it "useToken") and needs to react accordingly to this change of token to fetch new data

"I'm not sure why you'd even consider putting the functions in the dependency array for the useEffect in usePets.": unfortunately react-hooks/exhaustive-deps is here to warn you. You can disable or ignore it but I guess you expose yourself to a real missing dependency? Genuinely very keen to hear if you use this rule in your projects and what you do in such cases (where you use a function in the useEffect but do not want to re-run the effect each time this fn changes). To me it's such a weird/unnatural thing to list functions as dependencies because almost all the time functions do not change.