Skip to content
Advertisement

React – Adding props.something as a dependency of useEffect

I have this useEffect code here:

useEffect(() => {
    if (status === "completed" && !error) 
      props.onAddedComment();
  }, [status,error,props.onAddedComment]);

But I get this warning in the terminal: React Hook useEffect has a missing dependency: ‘props’. Either include it or remove the dependency array. However, ‘props’ will change when any prop changes, so the preferred fix is to destructure the ‘props’ object outside of the useEffect call and refer to those specific props inside useEffect

Why do I need to use destructuring if I’m passing props.onAddedComment and not the whole props object? would it still refer to the whole props even if I’m adding the .onAddedComment?

I have the same question with using params, in this code:

useEffect(() => {
    sendRequest(params.quoteId);
  }, [params.quoteId, sendRequest]);

I didn’t get this warning here, so why?

In short, my question is if I should always use destructuring even if I’m adding .something after the props, and why isn’t it warning me with the params.

Thanks!

Advertisement

Answer

Thanks to Andrius for finding this. See here.

When you invoke a function as part of an object, it’s possible that the behavior function also depends on which object is being used, even if the function itself doesn’t change. Here’s a minimal example of why

useEffect(() => {
  obj.fn();
}, [obj.fn]);

could be a problem:

const Child = ({ obj }) => {
  React.useEffect(() => {
    obj.fn();
  }, [obj.fn]);
  return null;
};
const App = () => {
  console.log('App rendering');
  const [count, setCount] = React.useState(0);
  // This fn is stable across renders
  const [fn] = React.useState(() => function() { console.log('count is', this.count); });
  React.useEffect(() => {
    setTimeout(() => {
      console.log('Calling setCount; expecting obj.fn to be called again');
      setCount(count + 1);
    }, 1000);
  }, []);
  return <Child obj={{ count, fn }} />
};

ReactDOM.render(<App />, document.querySelector('.react'));
<script crossorigin src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>
<div class='react'></div>

The point of exhaustive-deps is to get you to call the effect callback when anything inside the callback changes. Because it’s theoretically possible for a change to an object to produce a change in the logic executed if the object has a method, the object itself should be added to the dependency array.

This does not produce an error:

useEffect(() => {
  sendRequest(params.quoteId);
}, [params.quoteId, sendRequest]);

because quoteId is not a function you’re invoking; the this of params does not matter, in contrast to my above snippet and your original code, in which case a this of props could theoretically matter.

If you had done instead

useEffect(() => {
  sendRequest(params.getQuoteId());
}, [params.getQuoteId, sendRequest]);

That would have produced the warning, because now the invocation of getQuoteId depends on what params is.

Removing the function from the object and putting the function into a standalone identifier also removes the warning, because calling a function as a standalone identifier instead of as part of an object removes the possible dependency the function has on the object – the this inside the function no longer references the object, but undefined.

One way to think of it is that when you invoke a function as part of an object, the object itself is passed as a hidden additional parameter to the function, as the this inside the function.

This:

useEffect(() => {
  obj.fn();
}, [obj.fn]);

is like doing

const { obj } = fn;
useEffect(() => {
  fn.call(obj)
}, [fn]);

which is clearly missing obj as a dependency – even if the fn‘s implementation does not consider its this at all.

Advertisement