eslint-plugin-react-hooks introduces a bug to minimal app unless overridden
Following up on #14920 ...
[ESLint] Feedback for 'exhaustive-deps' lint rule
...I believe I have a case which triggers the eslint rule and which creates a looping bug if the auto-fix is applied. I also propose a way the linting might be applied to resolve this.
I wrote a minimal app to explore the issue, to try to figure out an AST inspection which might mitigate the problem. Happy to refactor if you tell me an alternative way to present the backend server which would be preferred to an express dependency.
The app is a 'row editor' which edits rows from a backend, having an id and title. The UI offers a list of row links. The user can navigate to edit a row from the list or create a new row. The front end user is expected to edit the title field. However, the backend also needs to make changes(it adds the id value from the 'database' when a new record is saved).
There are two lines which require an override otherwise the app enters a loop between client and server, creating runaway saves and reloads and making the UI unusable. The override lines needed are at...
It could be possible to detect that setLocalRow is called within the body, and therefore allow a dependency of localRow to be commented in the array instead of declared.
Although the minimal client app is generic, the full example is quite detailed, because the async callbacks need a server endpoint to demonstrate the issue. Additionally, the interlocking concerns of e.g. navigating to rows, auto-saving rows, creating new rows contribute to defining the problem. With fewer features supported, the problem doesn't arise.
The backend code is super-simple so hopefully doesn't confuse. If there is a specific refactoring which would help, let me know as this codebase is purely there to demonstrate the issue.
npm install then
npm run start then
http://localhost:8080 should be enough to observe the working system. Removing the eslint overrides and then auto-fixing will show the issues which arise if the eslint rule is allowed to add localRow or remoteRow to the dependencies.
What is the current behavior?
A loop between client and server if the auto-fix is applied
What is the expected behavior?
The eslint rule should not introduce a dependency which causes a loop. For example, it could instead add a commented reference to localRow in the dependencies instead, to account for the case that setLocalRow is invoked in the body.
[localRow, remoteRow, saveItem]
...could look like...
[ /localRow,/ remoteRow, saveItem]
This would allow the user to decide whether to leave the commented reference in place, or uncomment it to declare the dependency, knowing that it could create a loop. Either comment or reference would satisfy the rule.
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
I observed the issue with "eslint-plugin-react-hooks": "^2.0.1"
- 1. React dev tools doesn't appear in chrome (Ubuntu 18.04.3)
- 2. Bug: React function component unmounts on state change.
- 3. Bug: SuspenseList revealOrder="backwards" is not consistent without tail props
- 4. Bug: disableRemotePlayback not recognized
- 5. Bug: react test utils do not work together with esm module