1

I know the how useEffect's dependencies works. However in my scenario, I should not watch the change of a prop value I use it as a condition to process flow, but I will get a react-hooks/exhaustive-deps warning if I don't put it in the dependencies array.

My scenario is that if foo and fetchValue change, I want to re-run the whole fetch. Although I use bar as a condition to decide is the fechValue called, but in my business logic, the change of bar should not make re-run the block.

const Component = ({ foo, bar, fetchValue }) => {
  useEffect = (
    () => {
      if(foo) {
        if (bar) {
          fetchValue();
        }
      }
    },
    [foo, fetchValue] // will get a warning `react-hooks/exhaustive-deps`
  )

  return <div />

}
Improve this question
1
  • this does not sound like a valid business case, more like asking for hard time debugging race conditions in the future - would you please describe more why this is needed? Commented May 21, 2019 at 9:37

3 Answers 3

Reset to default
4

The ESLint rule is there to provide safety with the useEffect hook.

If you're absolutely sure the value is not a dependency of useEffect, you can add a comment to ignore the ESLint rule: Disabling Rules with Inline Comments.

I would suggest adding // eslint-disable-next-line instead of file-wide eslint-disable.

Here's more context about this by Dan Abramov:

is there any way I can disable this rule specifically for places where the spread operator is used?

You can always // eslint-disable-next-line react-hooks/exhaustive-deps if you think you know what you're doing.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Sure. I know how to disable the warning. I just want to discuss is some great implement to avoid the warning and do it in the right way.
The tool is there to catch bugs with common use cases, e.g. external variables used inside the useEffect, which does some optimisations to prevent calls on each re-render. What you're describing is an edge case, which the tool doesn't support, therefore you should know what you're doing and disable the rule. I have provided a comment by Dan Abramov explaining exactly this.
1

The reason you're supposed to put all variables that you use in your useEffects 'deps' array is not doing so can create weird issues due to stale data.

For example, given your use case, suppose your component props are initially {foo: false, bar: false } (assume fetchValue is some fixed function), and then they change to {foo: true, bar: true}. In this case, your component will call fetchValue as expected.

But as another example, suppose your props change from {foo: false, bar: false} to {foo: true, bar: false} and then change to {foo: true, bar: true}. In this case, fetchValue has not fired, despite that the props are the same as they were at the end of the previous example.

Maybe this isn't "wrong", per se, but it's certainly weird and unintuitive: ideally your component should behave consistently based on its props and it shouldn't matter what order the props changed.

So, yes, you can always eslint-ignore the deps array to allow it to be incomplete, but I'd suggest looking for another solution, personally.

It's hard to be specific without more information about the use case, but perhaps the call to fetchValue can be memoized so that it doesn't do anything if foo hasn't changed? Or perhaps foo and bar could be combined into a single prop so they change together?

Improve this answer

7 Comments

"suppose your props change from {foo: false, bar: false} to {foo: true, bar: false}". It would fire there, and from what I understand, that is what @WendellLiu wants. And only that because "and then change to {foo: true, bar: true}" doesn't need to fire it.
@Robbeoli useEffect will fire there, but fetchValue will not be fired, because it's wrapped in if(bar) and bar is still false.
Oh yeah, that is true, to me this use does feel intuitive thought. I see bar as some state that determines if a it is ok to fetchValue, and foo as the actual trigger that needs to be true and checks if the bar state is true.
@Retsam What if bar is a state inside this Component? Do I need to add it in the dependency array to get the latest value inside useEffect?
@NiyasNazar Yes - the only things that you don't need to include in the deps array are things defined outside the component (e.g. imports or module level variables) and things guaranteed to be constant by React (state setters, dispatch functions, and ref objects).
|
-3

Logic like that happens quite often with if statements in 'useEffect'. First of all, there is no need to add dependencies at all, you could just leave it empty.

Now there are some things you could do, either simply ignore the warning, because your code looks fine. Or reconstruct your code where bar is an argument of fetchValue(). So you're code would be:

const Component = ({ foo, bar, fetchValue }) => {
  useEffect = (
    () => {
      if(foo) {
        fetchValue(bar);        
      }
    },
    [foo, fetchValue] 
  )

  return <div />

}

And start your fetchValue(baz) declaration with if(baz){ //your code}.

Kind of cumbersome though.

You need to ask yourself, why and where do you expect foo or bar to change? Inside the component itself? Shouldn't foo and/or bar be states then with foo and or bar as defaultValues?

const Component = ({ foo, bar, fetchValue }) => {

  const [fooState, setFooState] = useState(foo);
  const [barState, setBarState] = useState(bar);

  useEffect = (
    () => {
      fooState && barState && fetchValue(); 
    //does the same as your code, I just prefer short circuits for these kind of things.
    },
    [fooState, fetchValue()] 
  )

  return <div />

}
Improve this answer

6 Comments

Refactoring fetchValue doesn't solve anything because the code is still using the bar value inside the useEffect. I support the second part, if the value is irrelevant inside the useEffect, the code could probably be refactored to remove it's usage inside the useEffect.
Both of your examples will still raise the same linter warning, and moving props into state is not a generally good idea. (It means that the component will completely ignore changes to those props, which is often not desired)
Maybe I did something weird then, because when I stumbled upon this warning on a similar use case, and later had to change my prop to a state for a custom delay, the warning went away: useEffect(() => { if (toggleComponents) { const getElem = document.getElementById(inputId); const getValue = getElem.value ? getElem.value : getChecked(inputId); setValue(getValue); } setToggleComponents(editMode); }, [editMode]); No warnings here.
@Retsam you're making a weird argument. You say the prop changed doesn't affect the useEffect, but somehow it would be a problem with the state? This is getting very hypothetical, can you please provide a more concrete example on which we can expand?
@MarkoGrešak The second code snippet is a concrete example. By copying foo into fooState on initial render, all future values of foo would be ignored: useState only reads its default value once. (This is a completely separate issue from useEffect)
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.