2

Assume you are using React and you are writing a custom hook useSomething that returns the identical same thing each time it is invoked for the same component.

const something = useSomething()

// useSomething() at time X  === useSomething() at time Y

If you now use this something value inside of a useEffect(() => ...) and you do not pass something as a dependency to the array of the second argument of useEffect then the linter will warn you:

React Hook useEffect has a missing dependency: 'something'. Either include it or remove the dependency array. (react-hooks/exhaustive-deps)

Of course ESLint cannot know that something will always stay identical (per component), but adding not-changing things like something to the dependency array of useEffect each time they are used is really annoying. Just deactivating react-hooks/exhaustive-deps does also not seem to be a good solution (nor using // eslint-disable-next-line react-hooks/exhaustive-deps).

Is there a better solution than to add things like that unnecessarily to the dependency array of useEffect just to make the Linter happy?

Please find a simple demo here: https://codesandbox.io/s/sad-kowalevski-yfxcn [Edit: Please be aware that the problem is about the general pattern described above and not about this stupid little demo - the purpose of this demo is just to show the ESLint warning, nothing else]

[Edit] Please find an additional demo here: https://codesandbox.io/s/vibrant-tree-0cyn1

Natasha
  • 516
  • 7
  • 24
  • 1
    It might be annoying to specify all dependencies even though they might be constant, but it's good practice. After all they might be constant **for now**, it's kind of future-proofing – apokryfos Sep 21 '19 at 10:15
  • @apokryfos Thanks for your answer. Yeah, in the comment of Dan that I have linked in my above answer, Dan says the same. So your comment is the correct answer to my question. – Natasha Sep 21 '19 at 10:26
  • Nevertheless, I still think it's annoying and also confusing and also I'm not really sure whether that would be still considered a "good practice" if there had never been a react-hooks ESLint plugin. If a certain function behaves completely different regarding its return value sometimes in future then it's nothing special that this will cause trouble, there's no need to prevent that in advance. Anyway, but let's just say I am wrong with my last comment ... ;-) – Natasha Sep 21 '19 at 10:28
  • 1
    [A complete guide to useEffect](https://overreacted.io/a-complete-guide-to-useeffect) also argues for specifying all dependencies and offers better patterns for more complex cases when you want to minimise the number of times the `useEffect` function is called, such as using [the functional updater form](https://reactjs.org/docs/hooks-reference.html#functional-updates) of `setState` or decoupling updates from actions with `useReducer` instead of `useEffect`. It is a 50 min read, but I think it will give you the background to be confident that the ESLint warning is correct. – Jeremy Larter Sep 21 '19 at 12:53
  • 1
    Thank you very much, Jeremy, for that tip. Great article. – Natasha Sep 21 '19 at 14:19

2 Answers2

5

Here

https://github.com/facebook/react/issues/14920#issuecomment-471070149

for example you can read this:

If it truly is constant then specifying it in deps doesn't hurt. Such as the case where a setState function inside a custom Hook gets returned to your component, and then you call it from an effect. The lint rule isn't smart enough to understand indirection like this. But on the other hand, anyone can wrap that callback later before returning, and possibly reference another prop or state inside it. Then it won’t be constant! And if you fail to handle those changes, you’ll have nasty stale prop/state bugs. So specifying it is a better default.

So maybe just adding that never-changing values to the dependency array of useEffect may yet be the best solution. Nevertheless I hoped there would be something like a ESLint react-hooks configuration possibility to define a list of hook names which whose return values should be considered as static.

Natasha
  • 516
  • 7
  • 24
0

The example is a little contrived but I suspect you may wish to create a new useEffect block without this dependency.

If the store is not changing though I'd question why you'd wish to console log it time. If you wish to log it only on change then you'd add someStore to your dependency array. It really depends on what you're trying to achieve and your seperation of concerns.

I'd argue that if someStore is used as part of whatever logic is handled in this effect then it does belong in your dependency array.

You could also alternatively move const something = useSomething() into your effect and extract it as a custom hook link 

useEffect(() => {
    console.log("Current state (may change)", someState);

  }, [someState]); 

useEffect(() => {
    console.log("Current store (will never change)", someStore);
  });
Damian Green
  • 6,895
  • 2
  • 31
  • 43
  • Thanks for your response. But of course my real world use case is way more complex that this simple little stupid demo that I've provided. I really want this `someStore` to be used inside of the same `useEffect` block. – Natasha Sep 21 '19 at 08:01
  • why does it need to be in the same block? – Damian Green Sep 21 '19 at 08:02
  • Replace that `useSomething` example with `const [state, setState] = useStateObject(...)` which behave similar to `useState` just that the state is always an object and the state update passed into `setState` will bet merge with the previous state (similar as it is done by `Component.prototype.setState`). If you use `setState` inside of an `useEffect` block ESLint will NOT warn if `setState` is created by `useState` but will warn you if `setState` has been created by `useStateObject`. Like said it's about the pattern not about my demo whose only purpose is to show the warning. – Natasha Sep 21 '19 at 08:11
  • BTW: I've added an additional demo to the question above - maybe that demo describes better what's the problem. – Natasha Sep 21 '19 at 08:51