Discussion: useEffect deps array

[darklight9811]

What is it

Since the hooks launch, I've been working with various teams and developers, and all of them, I repeat, ALL of them, seniors and juniors. Have been using useEffect "wrong" (considering what the react team intended). They all use the dependency array as an update array. There is no reason to keep hammering us, telling us that it is a dependency array if no one uses that way.

What I propose

Change linters and documentation to reflect the array as an update array, not as a dependency. Maybe add a third argument that should run as an actual dependency array, that does not trigger the code inside.

[bvaughn]

What do you mean by an "update array" and how is that different from the concept of a dependency array?

[darklight9811]

@bvaughn, update array is when the hook should be called, the problem is, that the dependency array runs everytime a value changes in useEffect, and that is not what people are looking for when using it. I mean with update array, is an array only for useEffect, telling when it should run the code inside.

[bvaughn]

There is no reason to keep hammering us, telling us that it is a dependency array if no one uses that way.

The lint rule is not there to correct misconceptions or be pedantic. It's to prevent people from accidentally using hooks in ways they don't intend to. For example, if a variable is referenced inside of an effect but is not specified in the second array parameter, the behavior of the effect (at least under certain conditions) is likely to be wrong.

You can find discussions of some of these scenarios on past GitHub issues if you're curious!

[bvaughn]

update array is when the hook should be called, the problem is, that the dependency array runs everytime a value changes in useEffect, and that is not what people are looking for when using it. I mean with update array, is an array only for useEffect, telling when it should run the code inside.

I don't think there's any difference between these two terms.

I think the difference is that you're arguing it should be possible for some things referenced inside of the effect to change without the effect being triggered. If you really want this behavior, you can use a (mutable) ref to accomplish it. In many cases though, trying to trick or bypass the dependencies array is a pattern that will cause bugs. (You'll end up potentially referencing a stale value.)

[darklight9811]

@bvaughn, but linters work as a guideline for developers, mainly in big projects. If a variable is referenced inside useEffect, it does not mean (for all the developers I met) that it should be updated, but instead that it just references it inside.

If you really want this behavior, you can use a (mutable) ref to accomplish it.

I can use that, but all the devs use "useEffect" wrongly, react as any language, needs to adapt based on the way people speak it.

trick or bypass the dependencies array is a pattern that will cause bugs.

That is why I suggest using 3 parameters for useEffect, the callback, the update array and the dependency array.

[bvaughn]

I think there are problems with what you're proposing that maybe you haven't considered. Perhaps you could share an actual code example that highlights the problem you're talking about?

This general topic has been discussed in length on some other issues that you might be interested in reading. For example, #16956.

[darklight9811]

Code examples:

useEffect(() => {
fetchTenant();
}, [selectedTenant]);

This would trigger an lint error, since selectedTenant (an id) is not present inside of the fetchTenant (that is a redux action).

const { open } = useContext(modalContext);

useEffect() => {
if (props.startOpen) open(props.id);
}, [props.id, props.startOpen]);

I don't want the open method to trigger an update, since it would fall into an endless loop, since the modalContext would update the current displayed modal id, and that would update the context, and trigger a loop inside of useEffect. But at the same time, I would want it to update in case the provider changes.

There are tons of examples, if you wish.

[bvaughn]

useEffect(() => {
  fetchTenant();
}, [selectedTenant]);

This could should trigger an error, right? The dependencies array can be a little confusing or awkward to think about, but it's how you tell React when it needs to re-run an effect. Some effects should run every time (no dependencies array) but many should only run under certain conditions.

In the code example above, I think you're trying to express that you want fetchTenant to be called any time selectedTenant changes. Conceptually, that makes sense. The way this code is written though, changes to the fetchTenant function itself would not be picked up by the effect. That could cause some pretty confusing bugs.

Maybe it wouldn't in your specific situation– if fetchTenant was a static function that never changed. The lint rule tries to account for this by not requiring you specify things like ES imports (that are known to be static).

But imagine that fetchTenant is a prop that does change. Now the effect above would be broken in certain situations (if fetchTenant changed but selectedTenant did not).

const { open } = useContext(modalContext);

useEffect() => {
  if (props.startOpen) {
    open(props.id);
  }
}, [props.id, props.startOpen]);

I don't know if I have enough context to understand this one. (Why would modal context update the props.id? Those don't seem connected here.)

There are tons of examples, if you wish.

No, but thank you 😄 I would suggest you read some of the other issues where this topic has been discussed though, like the one I mentioned above (#16956). I don't think there's a lot of value in rehashing those discussions here in a separate thread.

[darklight9811]

About the first example, see? That is just what I'm saying, it should update the props, but do not run.

About the second example, little matters the concept, you should know that you can't preview every situation, but prepare for them, every application has it's own needs and it may be necessary in that cases.

As I said, it doesn't matter what the react team says, if its not harmonized with the way people talk react, it will be a really big factor for it's downfall.

[bvaughn]

I don't think I understand what you're saying about the first example then, I'm sorry. 😅

It seems like this discussion isn't really new or different from the one on #16956 though, so I think it's probably best we close this issue and redirect conversation there. There's already been a lot of thoughtful conversation there that we wouldn't want to duplicate here. (It's helpful for people looking for this topic to find discussion in one place rather than spread out.)

If you'd like to add additional concerns to that thread after reading the earlier discussion, that would be fine! 😄 Best to keep it all in one place.

[darklight9811]

Look, I've been a react evangelizer for a long time. I've been pushing issues like this under the carpet for other developers. But ignoring them only make it worse. I've seen many people abandon and digust react for this. And now, I become one of them. Sadly.

[bvaughn]

To be clear, @darklight9811, I don't think I agree with the conclusion you're coming to, but I am not pushing your concerns under the rug. With any large project like this, it's important to organize discussions and keep them from forking off into many separate threads that are difficult to find and piece together later. I'm just re-routing this chat to a large discussion thread about the same topic that already existed. :)