add must-have eslint rules #7261
Conversation
Signed-off-by: reactoholic <[email protected]>
Signed-off-by: reactoholic <[email protected]>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@coderabbitai help |
CodeRabbit Commands (Invoked using PR comments)
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.vscode/extensions.json (1)
6-7: LGTM! Good addition of the React hooks extension.The addition of
ms-vscode.vscode-react-hooksis a valuable improvement that will provide better IDE support for React hooks development, complementing the ESLint rules being added in this PR.This extension will help developers by:
- Providing IntelliSense for React hooks
- Offering quick fixes for common hooks-related issues
- Supporting the enforcement of hooks rules at the IDE level
.eslintrc.js (1)
43-43: Excellent addition of React Hooks rules!These rules are crucial for preventing bugs in React applications:
rules-of-hooks: Enforces Rules of Hooks, preventing runtime errorsexhaustive-deps: Warns about missing dependency array variables, which directly addresses the PR objectives about preventing dangerous behavioral changesThe warning level for
exhaustive-depsis a good choice as it highlights potential issues while allowing for justified exceptions.Consider documenting common patterns for handling dependencies in React hooks, especially for cases where certain dependencies need to be intentionally omitted.
Also applies to: 45-45
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.eslintrc.js(3 hunks).vscode/extensions.json(1 hunks)package.json(1 hunks)
🔇 Additional comments (3)
.eslintrc.js (2)
41-42: LGTM! Good TypeScript-first configuration
- Disabling
react/prop-typesis correct as TypeScript handles type checking - Warning on
@ts-comments helps prevent unnecessary type assertions
Line range hint 15-16: Verify ESLint plugin dependencies
The configuration uses React-specific rules. Let's ensure all necessary plugins are properly configured.
✅ Verification successful
ESLint plugin dependencies are properly configured ✅
The required ESLint plugins for React (eslint-plugin-react and eslint-plugin-react-hooks) are correctly listed in the package dependencies, ensuring proper functionality of the React-specific rules in the ESLint configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if required ESLint plugins are listed in package.json
cat package.json | jq '.dependencies + .devDependencies | keys' | grep -E 'eslint-plugin-(react|react-hooks)'
Length of output: 167
package.json (1)
191-191: LGTM! The ESLint dependencies update aligns with the PR objectives.
The changes appropriately add the necessary ESLint plugins for enforcing React and React Hooks rules, which will help catch issues with missing dependency array variables. The version ranges are appropriate for dev dependencies.
Let's verify the compatibility of these versions:
Also applies to: 194-195
✅ Verification successful
ESLint dependencies are compatible with each other
The peer dependency checks confirm that:
- [email protected] is compatible with ESLint 8.x (supports ^3 through ^9.7)
- [email protected] is compatible with ESLint 8.x (supports ^3 through ^9.0.0)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify compatibility between eslint and its plugins
# Check the peer dependencies of eslint-plugin-react and eslint-plugin-react-hooks
npm info [email protected] peerDependencies --json
npm info [email protected] peerDependencies --json
# Check for any known compatibility issues or breaking changes
npm view [email protected] changelog
npm view [email protected] changelog
npm view [email protected] changelog
Length of output: 410
reactoholic
added
production
|
As I'd like to have less dusty PRs, let's quickly discuss the suggested changes and agree on an approach. As far as I understand, we had some of these configurations in the past. Although nice to have, I don't think we have time to fix either all dependency arrays or the TS ignore comments. I'd suggest updating only the ESLINT as suggested (if it doesn't require many changes). Let's collect the other suggestions in a technical ticket and keep the PRs fresh. @ccanos , @reactoholic , let me know your thoughts. |
The TS ignores are only in 3 places, these are fast to fix, but I agree not fixing all dep arrays as this hides insanely big chance of messing up the app. I agree on adding the rules as a starter, possibly fixing the 3 TS ignores. |
This PR is entirely for test purposes. One should pull this branch locally and check how many issues the client has from TS disabling and missing dependency array variables which is absolutely not good. Dependency array variables can change entire behavour and extremely dangerous avoiding them when they must be present in the array.
Summary by CodeRabbit
New Features
Bug Fixes
Chores