React: Bug: eslint-react-hooks asking (exhaustive-deps) asking for the full object instead of the property that is being used on useEffect

Created on 15 May 2020  路  11Comments  路  Source: facebook/react

React version:

"react": "^16.13.1",
"react-dom": "^16.13.1",

devDependencies
"eslint": "^7.0.0",
"eslint-import-resolver-alias": "^1.1.2",
"eslint-module-utils": "^2.6.0",
"eslint-plugin-import": "^2.20.2",
"eslint-plugin-react": "^7.19.0",
"eslint-plugin-react-hooks": "^4.0.0",

Steps To Reproduce

This code:

function MyComponent() { 

  const lastPathname_ref = useRef(null);
  const location = {pathname: ""};

  useEffect(() => {
      lastPathname_ref.current = location.pathname;
  },[]);
}

Link to code example:

https://codesandbox.io/s/solitary-currying-htsp3

The current behavior

This very same code is asking for different variables in the dependency array:

On CodeSandbox, it's asking for location.pathname, which is correct and is the intended behavior of my useEffect.

image

On my local environment (using the versions I've mentioned above), it's asking for the full location object. Which I think it's not correct, and that is not what I need.

image

I don't know what versions the CodeSandbox environment is using.

The expected behavior

It should be asking for the location.pathname property, instead of the full location object.

NOTE

This issue seems to be different than https://github.com/facebook/react/issues/16265 , because that one is caused when you call a method straight from the object, without destructuring it first. This issue is happening when using a property, not a method.

Unconfirmed

Most helpful comment

I also have this behavior since upgrading to "eslint-plugin-react-hooks": "4.0.2", from version 4.0.0.

In my case:

useEffect(() => {
    const newSelectedVersion = globalPackage?.version || globalPackage?.versions[0].version;
    setSelectedVersion(newSelectedVersion);
  }, [globalPackage]);

Produce the following warning message:

React Hook useEffect has missing dependencies: 'globalPackage?.version' and 'globalPackage?.versions'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

All 11 comments

I also have this behavior since upgrading to "eslint-plugin-react-hooks": "4.0.2", from version 4.0.0.

In my case:

useEffect(() => {
    const newSelectedVersion = globalPackage?.version || globalPackage?.versions[0].version;
    setSelectedVersion(newSelectedVersion);
  }, [globalPackage]);

Produce the following warning message:

React Hook useEffect has missing dependencies: 'globalPackage?.version' and 'globalPackage?.versions'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

We have the same as @DavidHenri008 - at this point, unsure if this is intended, so actually swapped the missing deps over to warnings and ignoring them for the time being :(

It actually is quite problematic in places, for instance have something like this -

useEffect((): void => {
  all?.accountId.eq(accountId) && setValue(all.lockedBalance);
}, [accountId, all]);

It expect me to add all?.accountId.eq to the deps array, however that makes other linting rules complain since .eq is a function -

Avoid referencing unbound methods which may cause unintentional scoping of this. eslint@typescript-eslint/unbound-method

The only option atm is to remove all ?., but have a bundle of them. Or remove them where they refer to functions and follow the suggested approach to not include the parent.

We are have the same issues, both the original poster and the optional chaining when upgrading to version 4 of the eslint-plugin-react-hooks package. We are holding off until we can take a closer look. Here is an example of the warning being thrown:

  useEffect(() => {
    if (response?.data) {
      dispatch({ type: SUGGESTION_SUCCESS, items: response.data })
    }
  }, [response])

it's saying there is a missing dependency for response?.data. If we however change it up to:

  useEffect(() => {
    if (response && response.data) {
      dispatch({ type: SUGGESTION_SUCCESS, items: response.data })
    }
}, [response]

it works fine, but that would be a substantial change for us. For what it's worth we aren't using CRA so I can't reproduce this on CodeSandbox 馃槥 .

Looks like it's somehow related to optional chaining

I've found a similar issue that's been affecting our codebase. When you assign an object's property using mutation it also asks you to pass in the entire props object needlessly.

// Requires entire props object
const myObj = React.useMemo(() => {
    const returning = { id: 0 };
    returning.id = props.id;
    return returning;
}, [props]);
// Lets you pass in the correct dependencies
const myObj = React.useMemo(() => {
    const returning = { id: props.id };
    return returning;
}, [props.id]);

Me too!

  useEffect((): void => {
    setInitialAccountIdFound(false);
    data?.accounts?.forEach((account: Account): void => {
      if (account.isDefaultAccount && defaultAccountId !== account.id) {
        setDefaultAccountId(account.id);
      }
      if (initialAccountId === account.id) {
        setInitialAccountIdFound(true);
      }
    });
  }, [data, defaultAccountId, initialAccountId]);

gives

89:6 warning React Hook useEffect has a missing dependency: 'data?.accounts?.forEach'. Either include it or remove the dependency array react-hooks/exhaustive-deps

after updating eslint-plugin-react-hooks to 4.0.2

I've found another similar issue, not sure if it's related. If you use optional chaining in a default parameter, and use the full object later, it requires both the full object and the field you're using.

const someFunc = React.useCallback(({ someParam = Data?.id}) => {
    console.log(Data);
}, [Data, Data?.id]);

Keep getting this:

  useEffect(() => {
    if (props.match.params.docPeriod !== lastDocPeriod_ref.current) {
      lastDocPeriod_ref.current = props.match.params.docPeriod;
      setDoc(null);
    }
  },[props.match.params.docPeriod]); // ESLINT ERROR

  // NOTE: lastDocPeriod_ref IS A useRef()

It's asking for props.match.params instead of props.match.params.docPeriod.

image

Same issues on our projects as well, related to optional chaining.

I have the same issue as the original poster (no optional chaining, effect requires full object instead of property) after upgrading to 4.0.0 as well.

I'm getting the expected behavior (location.pathname) with [email protected]. I'm closing this ticket, but if it happens again with 4.0.8+, please file another!

Was this page helpful?
0 / 5 - 0 ratings