Bug: eslint(react-hooks/exhaustive-deps) wrong deps with optional chained function calls

[AviVahl]

React version: 16.13.1

Steps To Reproduce

Every time optional chaining is used with a function call, the function itself is detected by the rule.

// users is optional...
const someContext = {
    users: Math.random() > 0.5 ? [{name: 'test'}] : undefined,
};

const filteredUsers = useMemo(() => {
    return someContext.users?.filter((u) => u.name.startsWith('a'));
}, [someContext.users]);

The current behavior

v4.0.1 and above of eslint-plugin-react-hooks tells me to change the dep to someContext.users?.filter.

The expected behavior

Allow me to keep using someContext.users. I can imagine scenarios where I would want the looked up symbol, but that's probably less common than depending on the uniqueness of the instance.

Observations

Probably regressed due to #18820, but I'm not sure.

[DavidHenri008]

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

[lynxtaa]

I have similar problem

const userEditable = useMemo(
  () =>
    !formValues?.nodeIds ||
    formValues.nodeIds.every(({ id }) => id),
  [formValues?.nodeIds],
)

Error: React Hook useMemo has a missing dependency: 'formValues.nodeIds'. Either include it or remove the dependency array react-hooks/exhaustive-deps. Autofix rewrites it as:

const userEditable = useMemo(
  () =>
    !formValues?.nodeIds ||
    formValues.nodeIds.every(({ id }) => id),
  [formValues.nodeIds, formValues?.nodeIds],
)

It will throw in runtime because formValues may be undefined

[ziimakc]

It also complains about data?.user.theme

warning React Hook useEffect has a missing dependency: 'data.user.theme'. Either include it or remove the dependency array react-hooks/exhaustive-deps

[AviVahl]

Guys, every time it complains about the looked up field which is not a function call, the new behavior is probably more correct.

In the examples about, if the memoized value depends on formValues?.nodeIds (@lynxtaa example) or on data?.user.theme (@ReTWi example), the lint rule has no way of knowing if formValues or data are immutable, so it compares the inner value directly (nodeIds and theme).

@lynxtaa the expression could have been just formValues?.nodeIds?.every(({id}) => id), and then the dep should have been: formValues?.nodeIds, but that's where the bug occurs in the lint (current version suggests formValues?.nodeIds?.every).

[lynxtaa]

@lynxtaa the expression could have been just formValues?.nodeIds?.every(({id}) => id), and then the dep should have been: formValues?.nodeIds, but that's where the bug occurs in the lint (current version suggests formValues?.nodeIds?.every).

In case of !formValues?.nodeIds || formValues.nodeIds.every(({ id }) => id) the result will be true when formValues is undefined. So just formValues?.nodeIds?.every(({id}) => id) is not a valid substitution

[AviVahl]

Oh my bad. formValues?.nodeIds?.every(({id}) => id) ?? true

[yanneves]

Relates to #18985, should now be resolved in [email protected]

[AviVahl]

thanks for the fix!

[AviVahl]

Re-opening. Bug still occurs:

import React, { useMemo } from 'react';

export const Comp = ({ text }) => {
  const sliced = useMemo(() => text?.slice(0, 5), [text]);
  // ...
};

Plugin suggests changing text to text?.slice.

[dmarkow]

Still happening here too. The fix seems to only work for useEffect. useMemo and useCallback are still triggering the same warnings:

useEffect(() => {
  console.log(a?.b);
}, [a]) // No errors

var c = useMemo(() => {
  return a?.b;
}, [a]) // React Hook useMemo has an unnecessary dependency: 'a'. Either exclude it or remove the dependency array

If I remove [a] in the useMemo version, it then tells me I need to add a?.b as a dependency.

[yanneves]

Interesting, it's not obvious from the plugin code that useMemo or useCallback would be treated differently when matching their dependencies - I'll see if I can apply it to these also, and anywhere else it might not be working

[gaearon]

Happy to keep taking fixes :-)

[vadzim]

still happening in [email protected]

[AviVahl]

Will probably be fixed by #19062 (which is waiting for a merge/release).

[AviVahl]

Should be fixed by [email protected]

[gaearon]

Remaining optional chaining problems should be fixed in [email protected]. If you experience some problem with optional chaining after 4.0.6, please file a new issue. Thanks.