Suggestion: Add correct version of fetchData example in "A Complete Guide to useEffect"

[SleeplessByte]

TL;DR: There is actually no solution given to the initial "issue". I recommend repeating the Comment component with the correct usage of useEffect.

Context

In the recent article A Complete Guide to useEffect, which is amazingly well written, there is a section "Don't lie to react about dependencies":

Lying to React about dependencies has bad consequences. Intuitively, this makes sense, but I’ve seen pretty much everyone who tries useEffect with a mental model from classes try to cheat the rules. (And I did that too at first!)

function Comment({ fetchData }) {
  useEffect(() => {
    fetchData().then(...);
  }, []); // This is okay, right? Nope. 😞
}

“But I only want to run it on mount!”, you’ll say. For now, remember: if you specify deps, all values from inside your component that are used by the effect must be there. Including props, state, functions — anything in your component.

Sometimes when you do that, it causes a problem. For example, maybe you see an infinite refetching loop, or a socket is recreated too often. The solution to that problem is not to remove a dependency. We’ll look at the solutions soon.

But before we jump to solutions, let’s understand the problem better.

In the last sentence it actually points that there will be a solution to this particular problem.

The issue, as you read on, is that fetchData might change, but because there are no dependencies listed, it won't ever run again. So perhaps the solution is to "Move the function inside the effect":

function App() {
  const [data, setData] = useState({ hits: [] });

  async function fetchData() {
    const result = await axios(
      'https://hn.algolia.com/api/v1/search?query=react',
    );
    setData(result.data);
  }

  useEffect(() => {
    fetchData();
  }, []); // Is this okay?

  // ...

This is a great example because it actually uses very similar code to the initial issue. I also think it's confusing, because weren't we fetching a Comment and now it's doing a "generic search"? Also, where did the Comment component go?

Naturally this can be a disjunct example, which is fine, but all the other examples are around the Counter concept, which is very cohesive and therefore clear to follow and see the differences in implementation.

Scrolling down, this last example is broken down and shown how to use deps correctly:

const [query, setQuery] = useState('react');

useEffect(() => {
  function getFetchUrl() {
    return 'https://hn.algolia.com/api/v1/search?query=' + query;
  }

async function fetchData() {
  const result = await axios(getFetchUrl());
    setData(result.data);
  }

  fetchData();
}, [query]); // ✅ Deps are OK

And then it's moved back into a component called Example. Why it's not called Example here and why it was App before is beyond me, I think it increases cognitive complexity as it's difficult, if you don't understand these hooks well enough yet, what is important, what isn't and most importantly: what are the invariants?

After this and another variations + explanation on this, the subject seems closed until we reach "In closing".

The problem

Personally, I don't think the original question was every answered: "What is the solution to []". The answer seems to be "it depends", but I don't think it's explicitly written out. I think that using the different class names makes it harder to follow.

• Can I pass my fetch function as a prop?
• Should I use all the deps of the fetch function in deps or the function itself (and risking an infinite loop, depending on how the function is created, and more specifically where).
• Should I replace my fetch function with useCallback? Or should I use useCalback and pass that as a prop?
• Should I use observables because I see .subscribe and .unsubscribe?
• What was so wrong about the first version anyway?

These are not my questions, but questions my employees had when reading the article.

The suggestion

Repeat the original problematic code just before in-closing, and fix it with everything we've learned above. Alternatively, change Comment to Example or Article, so that there is a "fixed" version down the line.

[gaearon]

Seems like you have a good idea of fixes that should be done. Do you want to send a PR doing them? That would be much appreciated.

[gaearon]

Should I use all the deps of the fetch function in deps or the function itself (and risking an infinite loop, depending on how the function is created, and more specifically where).

Should I replace my fetch function with useCallback? Or should I use useCalback and pass that as a prop?

I feel like I utterly failed to explain anything with this article if these are the questions that people ask. I'm sorry.

[SleeplessByte]

They only asked it because the examples to them were too different. There are no clear recommendations in the article (imo). Most of it is "you could do X or you could use Y".

Personally I don't think you failed explaining what useEffect is and how it works, but I don't think the guide is complete. It doesn't provide a ruleset or guidelines to use for most cases, it only highlights common issues and how they might be resolved.

[SleeplessByte]

Seems like you have a good idea of fixes that should be done. Do you want to send a PR doing them? That would be much appreciated.

I would, but I think your writing is much better than mine. I don't have a clear overview yet what exactly is missing from this article to make it even more complete. That said, I'll talk to the specific people who have raised the questions and see if that yields a better response.

[gaearon]

Hmm I think it does provide suggestions:

If you only use some functions inside an effect, move them directly into that effect.

[...]

if a function doesn’t use anything from the component scope, you can hoist it outside the component and then freely use it inside your effects.

[...]

Alternatively, you can wrap it into the useCallback Hook:

[...]

I guess that flow gets lost if you're not reading very closely. So here's a recap.

What to do if effect uses a function

• If you can, move this function outside the component. In that case we know for sure that it doesn't use any props or state, and so it's not a dependency.

• If you can't move it outside the component, move it inside the effect that uses it. Then you can clearly see which other values it depends on, and add them to deps array instead.

• If you can't either move it outside the component or move it inside the effect, wrap it with useCallback. Then you'll need to specify dependencies for that function itself.

Repeat that for every function dependency.

[gaearon]

Would this content in FAQ help? reactjs/react.dev#1815

[SleeplessByte]

I just sent them that recap. Here are the responses:

• Ah, that makes sense; I thought wrapping in useCallback was always just a way/an alternative to do this, not a "last resort".
• Ok, but they only two questions I have left are:
  • What is that observable/subscribe/unsubscribe stuff, why is it not explained? Can I just use that?
  • So just to be clear: never use the function itself as a dependency?

In short, I think that will make it great. And yes, I think that list is a must have in the FAQ. The issue mostly with my juniors is that they're given too many options, and not enough "there are other ways, but if you do it like this, you're probably doing it the right way most of the time".

[gaearon]

What is that observable/subscribe/unsubscribe stuff, why is it not explained? Can I just use that?

I don't really understand the question. It's just a random example of a data subscription. You can use observables for them if you want but I'm not sure how that relates to my point.

So just to be clear: never use the function itself as a dependency?

It's only OK to use a function as a dependency if you wrapped it in useCallback. But it's annoying to do so when you can, avoid it.

[gaearon]

New items in FAQ:

https://reactjs.org/docs/hooks-faq.html#why-am-i-seeing-stale-props-or-state-inside-my-function
https://reactjs.org/docs/hooks-faq.html#is-it-safe-to-omit-functions-from-the-list-of-dependencies
https://reactjs.org/docs/hooks-faq.html#what-can-i-do-if-my-effect-dependencies-change-too-often

Hope this helps.

[gaearon]

OK I pushed an update that I hope will clarify most of confusion.

[SleeplessByte]

What is that observable/subscribe/unsubscribe stuff, why is it not explained? Can I just use that?

I don't really understand the question. It's just a random example of a data subscription. You can use observables for them if you want but I'm not sure how that relates to my point.

That was my response more or less ("It's just a random API call that has nothing to do with the article"). The response from one guy was "oh okay, I thought hooks added this to apis or something".

So just to be clear: never use the function itself as a dependency?

It's only OK to use a function as a dependency if you wrapped it in useCallback. But it's annoying to do so when you can, avoid it.

Yes, that's how it was interpreted <3