React: eslint-plugin-react-hooks on vscode with formatOnSave enabled strange insertion on useEffect skipping array

[sartios]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

On VSCode with formatOnSave enabled, the eslint-plugin-react-hooks appends inside useEffect's skipping array the function I'm invoking in useEffect.

Initial

1. useEffect(() => { fetchData() }, [aProp])
2. const fetchData = async () => { ... }

After saving

useEffect(() => { fetchData() }, [aProp, fetchData])

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

What is the expected behavior?

It shouldn't append the function inside the skipping array of useEffect.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

[gaearon]

We can add an option to disable the autofix. This plugin is designed so that you actually read the messages it emits. It is incorrect to omit the dependencies so your original code is wrong. However, you also need to read the linter messages because it tells you how to fix it.

[gaearon]

Also running ESLint isn’t “formatting”. Can you please list the plugins you’re using?

[sartios]

The ESLint warning is about exhaustive-deps. I'd like to ask whether the optional array should contain the dependencies which if change then the effect should be re-invoked. If that's the case I'm wondering about the initial example, how the fetchData function will change as it's an internal implementation of the same component. However, when it comes to a property passed down it's apparently.

I'm using "react-hooks" only and have installed "eslint-plugin-react-hooks": "^1.5.0".

[gaearon]

how the fetchData function will change as it's an internal implementation of the same component

See "My function value is constant" section in https://github.com/facebook/react/issues/14920#issuecomment-471070149.

[zebralight]

We can add an option to disable the autofix. This plugin is designed so that you actually read the messages it emits. It _is_ incorrect to omit the dependencies so your original code is wrong. However, you also need to read the linter messages because it tells you how to fix it.

I was wondering what makes the omission of the dependencies that are auto-inserted incorrect. I'm facing a similar issue where the plugin with vs code inserts certain props to where an infinite loop is triggered.

[brybrophy]

Agree with everyone here. This plugin should not assume the developer's intent, especially for useEffect.

From the React docs...

If you want to run an effect and clean it up only once (on mount and unmount), you can pass an empty array ([]) as a second argument. This tells React that your effect doesn’t depend on any values from props or state, so it never needs to re-run. This isn’t handled as a special case — it follows directly from how the dependencies array always works.

This plugin has no way of knowing if the developer wants to trigger the hook only once, or if they left dependencies out by mistake. Excluding the useEffect hook, I _could_ see a case for auto-correcting in other hooks.

[gaearon]

@brybrophy

Omitting dependencies is not the correct way to "run something once". The correct way is to restructure your code so that it wouldn't need those dependencies.

The difference is a bit subtle. But if you have dependencies suggested by linter, and you're trying to remove them, your code has a bug.

Please read this:

• 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
• https://overreacted.io/a-complete-guide-to-useeffect/#dont-lie-to-react-about-dependencies

[gaearon]

Regarding the autofix, I hope https://github.com/eslint/rfcs/pull/30 will let us solve the problem without compromising on IDE suggestions.

[Mikado68]

@brybrophy I had the same problem as you did, and at first I was agree with you: why should ESLINT disallow something describe in the React documentation.
But @gaearon 's comment didn't make me want to "hack" ESLint.

I finally found a solution on StackOverflow. Create a new hook:
const useMountEffect = (fun) => useEffect(fun, [])

[gaearon]

Let's consolidate and track this in https://github.com/facebook/react/issues/16313 instead.

[gaearon]

This is resolved now.
https://github.com/facebook/react/issues/16313#issuecomment-587149109