Bug: Unexpected warning in react-hooks/exhaustive-deps using optional chaining operator

[jcestibariz]

When the optional chaining operator (?.) is used inside a hook and the variable is already listed in the dependencies eslint-plugin-react-hooks reports an unexpected "missing dependency" warning. Replacing ?. with . produces the expected behaviour: the warning is no longer reported.

I would expect that the optional chaining operator (?.) be handled as normal property access (.) in the context of this validation.

React version: 16.13.1
eslint-plugin-react-hooks version: 4.0.2

Steps To Reproduce

Notice that foo is both referenced inside the code in useEffect and is listed as a dependency.

import React, {useEffect} from 'react';

const getFoo = () => undefined;
const doSomethingWith = (foo) => {};

export default () => {
  const foo = getFoo();

  useEffect(() => {
    if (foo?.bar) {
      doSomethingWith(foo);
    }
  }, [foo]);

  return null;
};

package.json

{
  "name": "react-hooks-bug",
  "version": "1.0.0",
  "description": "",
  "license": "ISC",
  "scripts": {
    "test": "eslint test.js"
  },
  "dependencies": {
    "babel-eslint": "^10.1.0",
    "eslint": "^7.1.0",
    "eslint-plugin-react-hooks": "^4.0.2"
  },
  "eslintConfig": {
    "parser": "babel-eslint",
    "parserOptions": {
      "sourceType": "module"
    },
    "plugins": [
      "react-hooks"
    ],
    "rules": {
      "react-hooks/exhaustive-deps": "warn"
    }
  }
}

The current behavior

The following warning is reported:

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

The expected behavior

No warning should be reported.

Additional notes

With the following code, which only replaces ?. with ., eslint-plugin-react-hooks behaves as expected, that is no warning is reported. (Of course this fails at runtime when foo is undefined).

import React, {useEffect} from 'react';

const getFoo = () => undefined;
const doSomethingWith = (foo) => {};

export default () => {
  const foo = getFoo();

  useEffect(() => {
    if (foo.bar) {
      doSomethingWith(foo);
    }
  }, [foo]);

  return null;
};

[Mostafa-Mohammadi]

what is bar ?

[jcestibariz]

This is not real code it's just a synthetic example to reproduce this particular bug. In this example bar would be a possible property of foo.

[Mostafa-Mohammadi]

This is not real code it's just a synthetic example to reproduce this particular bug. In this example bar would be a possible property of foo.

By using the ?. operator instead of just ., JavaScript knows to implicitly check to be sure foo.bar is not null or undefined before attempting to access bar. If foo is null or undefined, the expression automatically short-circuits, returning undefined.

just put your foo.bar in deps Array useEffect

[deluksic]

I'm hitting this too with latest exhaustive-deps

[tbergquist-godaddy]

From looking at this change, it looks like you need to change your code to

export default () => {
  const foo = getFoo();

  useEffect(() => {
    if (foo?.bar) {
      doSomethingWith(foo);
    }
  }, [foo?.bar]);

  return null;
};

And it should work.

[jcestibariz]

@tbergq If I make that change it will miss changes to foo.

The point that I'm trying to make is that the optional chaining operator (?.) should be handled exactly as normal property access (.) in the context of this validation.

[tbergquist-godaddy]

@tbergq If I make that change it will miss changes to foo.

Sorry, yes, you are right, I missed the doSomethingWith(foo);

The point that I'm trying to make is that the optional chaining operator (?.) should be handled exactly as normal property access (.) in the context of this validation.

Yeah, that is probably right, I see if you write this without optional chaining it would look like

export default () => {
  const foo = getFoo();

  React.useEffect(() => {
    if (foo && foo.bar) {
      doSomethingWith(foo);
    }
  }, [foo]);

  return null;
};

and that does not produce an eslint error.

[belliAndrea]

Got the same problem here.

[yanneves]

This also happens in cases where we may access a prototype method on the dependency, such as

React.useEffect(() => {
  if (arr?.includes('value')) {
    // ...
  }
}, [arr])

[gaearon]

Happy to take a fix!

[yanneves]

@gaearon addressed in #19008 - first time digging into eslint plugin code, comments welcome 👀

[jcestibariz]

@yanneves Looks like that would work, do you mind adding a test case for this specific issue too? Something like:

{
  code: normalizeIndent`
    function MyComponent(props) {
      useEffect(() => {
        console.log(props?.foo);
        console.log(props);
      }, [props]);
    }
  `
}

[gaearon]

eslint-plugin-react-hooks@4.0.4

[yanneves]

@jcestibariz just added that to the test suite and it passes, not sure whether it's worth submitting a new PR for it, the underlying logic is identical to this test

{
  code: normalizeIndent`
    function MyComponent(props) {
      useEffect(() => {
        console.log(props.foo);
        console.log(props.foo?.bar);
      }, [props.foo]);
    }
  `,
}

[jcestibariz]

@yanneves You're right it's basically the same check. Thank you for checking.

If it ever happens again I'll know pretty soon after :)

[wojtekmaj]

With 4.0.4, I'm still getting an error at

const computedArray = useMemo(
  () => veryLongArray?.map(costlyComputation), 
  [veryLongArray]
);

Exact error:

4:2 warning  React Hook useMemo has an unnecessary dependency: 'veryLongArray'. Either exclude it or remove the dependency array  react-hooks/exhaustive-deps

[belliAndrea]

me too

[gabrieledarrigo]

Yeah, we are getting the same bug as well.
Any hints on how to solve the problem? : )
Thank you!

Update:

This was just fixed in #19062 😹
Thank you so much!

[gaearon]

Optional chaining problems should be fixed in eslint-plugin-react-hooks@4.0.6. If you experience some problem with optional chaining after 4.0.6, please file a new issue. Thanks.