0

i have a function that use in useEffect and click event, then throw warning "React Hook useEffect has a missing dependency", how should i remove the warning?

// location is react router location
const Component = ({ location }) => {
  const [data, setData] = useState(null);

  const fetchData = () => {
    const { id } = parseLocation(location);
    fetchDataFromServer(id).then(data => setData(data));
  }

  useEffect(() => {
    fetchData();
  }, [location]);

  return (
    <div>
      {data}
      <button onClick={fetchData)}>reload</button>
    </div>
  );
}

then i try this, but the warning still exist

// location is react router location
const Component = ({ location }) => {
  const [data, setData] = useState(null);

  const fetchData = (l) => {
    // l is location
    const { id } = parseLocation(l);
    fetchDataFromServer(id).then(data => setData(data));
  }

  useEffect(() => {
    fetchData(location);
  }, [location]);

  return (
    <div>
      {data}
      <button onClick={() => fetchData(location)}>reload</button>
    </div>
  );
}
5
  • 2
    You can put // eslint-disable-next-line above }, [location]); Commented Jul 22, 2019 at 14:56
  • Seems like you could add fetchData to the dependency array also Commented Jul 22, 2019 at 14:56
  • 1
    @ArnoldGandarillas When someone has a linter error, I wouldn't suggest disabling the linter error unless you explain why it's safe to ignore the issue. In this case, I don't think that's the best choice. Commented Jul 22, 2019 at 15:32
  • 1
    @SeanMC That's going to cause the useEffect to run every render - I think it'll actually cause an infinite loop, as each render will trigger fetchData, which triggers setState which causes a new render. Commented Jul 22, 2019 at 15:41
  • I highly recommend reading Dan Abramov's Complete Guide to useEffect; I tried to give the abridged version in my answer. Commented Jul 22, 2019 at 15:57

1 Answer 1

1

The point of the exhaustive-deps rule is to prevent hooks from reading stale props or state. The issue is that since fetchData is defined within the component, as far as the linter is concerned, it could be accessing stale props or state (via closure).

One solution is to pull fetchData out of the component and pass it everything it needs: (it's already being passed the location):

const fetchData = (l, setData) => {
  // l is location
  const { id } = parseLocation(l);
  fetchDataFromServer(id).then(data => setData(data));
}

const Component = ({ location }) => {
  const [data, setData] = useState(null);

  useEffect(() => {
    fetchData(location, setData);
  }, [location]);

  return (
    <div>
      {data}
      <button onClick={() => fetchData(location, setData)}>reload</button>
    </div>
  );
}

Since fetchData isn't defined outside the component, the linter knows that it won't access state or props, so it isn't an issue for stale data.


To be clear, though, your original solution is correct, from a runtime perspective, since fetchData doesn't read state or props - but the linter doesn't know that.

You could simply disable the linter, but it'd be easy to accidentally introduce bugs later on (if fetchData is ever modified) that way. It's better to have the linter rule verifying correctness, even if it means some slight restructuring of code.


Alternative solution

An alternative solution which leverages closure instead of passing the location into fetchData:

const Component = ({ location }) => {
  const [data, setData] = useState(null);

  const fetchData = useCallback(() => {
    // Uses location from closure
    const { id } = parseLocation(location);
    fetchDataFromServer(id).then(data => setData(data));

  // Ensure that location isn't stale
  }, [location]);

  useEffect(() => {
    fetchData();

  // Ensure that fetchData isn't stale
  }, [fetchData]);

  return (
    <div>
      {data}
      <button onClick={fetchData}>reload</button>
    </div>
  );
}

This approach lets you avoid passing location into fetchData every time you call it. However, with this approach it's important to make sure to avoid stale state and props.

If you omitted the [fetchData] dep to useEffect, the effect would only run once, and new data wouldn't be fetched when location changed.

But if you have fetchData in the deps for useEffect but don't wrap fetchData in useCallback, the fetchData function is a new function every render, which would cause the useEffect to run every render (which would be bad).

By wrapping in useCallback, the fetchData function is only a new function whenever the location changes, which causes the useEffect to run at the appropriate point.

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

2 Comments

It works, but need to pass setData to fetchData. const fetchData = ({ location, setData }) => { //... }
Ah, good catch. Fixed. Personally, I'd probably use the useCallback form, myself, in that case.

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.