React: [eslint-plugin-react-hooks]: auto-fix may crash app when deps is array/object types

Created on 27 Jun 2019 · 5Comments · Source: facebook/react

Do you want to request a feature or report a bug?
bug
What is the current behavior?

before run lint, my code is

const App = () => {
  const [options, setOptions] = useState([]);
  const [includesA, setIncludesA] = useState(false);

  const optionCodes = options.map(({ value }) => value);

  useEffect(() => {
    if (optionCodes.includes('A')) {
      setIncludesA(true);
    }
  }, [optionCodes.join(',')]); // transform array to string for compare 

  return <div onClick={() => setOptions([{ value: 'A' }])}></div>;
};

after 'eslint --fix'

const App = () => {
  const [options, setOptions] = useState([]);
  const [includesA, setIncludesA] = useState(false);

  const optionCodes = options.map(({ value }) => value);

  useEffect(() => {
    if (optionCodes.includes('A')) {
      setIncludesA(true);
    }
  }, [optionCodes]); // optionCodes.join(',') was replaced ! 

  return <div onClick={() => setOptions([{ value: 'A' }])}></div>;
};

The original code works fine and after 'eslint --fix' it just crashed cuz optionCodes is an array created in render function, the effect runs every time and crash my app with error :

react-dom.development.js:55 Uncaught Invariant Violation: Maximum update depth exceeded.

I've also notice this rule fix will add other params used by effect function to deps automatically. Like

// original code
const {id} = props
useEffect(()=> {
  console.log(id)
},[])

// after fix
const {id} = props
useEffect(()=> {
  console.log(id)
}, [id]) // id has been add to deps

What is the expected behavior?

For 'eslint --fix', what we expect is 'try to fix lint error automatically and SAFELY', SAFELY means DO NOT change my code logic, run 'eslint --fix' should never change your design or crash your app .

It would be better to tell developers to fix the deps by lint message, not auto fix it in dangerous way.

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

[email protected]

Source

kpaxqin

Most helpful comment

Yeah. See https://github.com/facebook/react/issues/15204#issuecomment-476789202 for context. I guess we’ll have to disable autofix if running it without looking is so common. The original intention was to make it an IDE suggestion only but ESLint doesn’t offer a way to do that. If we disable it, we’ll lose manual IDE autocompletion too which sucks.