Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eslint-plugin-react-hooks suggests adding a dependency on a function that always changes #15488

Open
sophiebits opened this issue Apr 24, 2019 · 6 comments

Comments

@sophiebits
Copy link
Collaborator

If you write:

import React, {useEffect} from 'react';
const Foo = ({ orgId }) => {
  const fetchOrg = () => {
    alert(orgId);
  };
  useEffect(() => {
    fetchOrg();
  }, [orgId]);
  return <div />;
};

then you get the error:

React Hook useEffect has a missing dependency: 'fetchOrg'. Either include it or remove the dependency array

But if you follow that advice and add fetchOrg to the dep array, you get:

The 'fetchOrg' function makes the dependencies of useEffect Hook (at line 6) change on every render. Move it inside the useEffect callback. Alternatively, wrap the 'fetchOrg' definition into its own useCallback() Hook

Ideally it could suggest the second solution immediately, instead of suggesting a remediation that it will immediately warn about.

@kik-o
Copy link

kik-o commented Apr 24, 2019

Currently working on this enhancement 🏗️

@ipoppo
Copy link

ipoppo commented Apr 29, 2019

can I add request? I want eslint to warn about possible dependencies error but not fix with --fix. Right now it is always fix my code regardless.

@jacobrask
Copy link

I think it's useful in most cases to keep exhaustive-deps auto fixable, but you can use eslint --quiet to only report errors and not warnings, so eslint --quiet --fix will not fix issues reported by "react-hooks/exhaustive-deps": "warn". Eslint also has a --fix-type flag you might play around with.

@gaearon
Copy link
Collaborator

gaearon commented Apr 30, 2019

can I add request? I want eslint to warn about possible dependencies error but not fix with --fix. Right now it is always fix my code regardless.

It would be great if you could request this in ESLint itself. We could add something like this but it seems like a more generic issue that would be better addressed in the tool. Even better — it would be nice to signal ESLint that the fix is a suggestion but should not be applied automatically. Then IDEs could offer it behind a click, but it would still require manual inspection.

@justDanielMata
Copy link

The second one is also not hepfull, if you move it into useCallback it will ask you to provide dependencies for it anyways, and some funcitions (like fetching a list of objects) don't take any parameters, an empty array fails, i'm not sure what the idiomatic solution is here?

@sgehrman
Copy link

I'm also in need of this fix. This breaks code and should be turned off until it's 'fixed'

@necolas necolas added React Core Team Opened by a member of the React Core Team Partner and removed React Core Team Opened by a member of the React Core Team labels Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants