[IMP] eslint_plugin_jsx simple config whith warn level #558
Conversation
WalkthroughThe changes in this pull request involve updates to the ESLint configuration and the Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
.eslintrc.cjsOops! Something went wrong! :( ESLint: 8.56.0 Error: Failed to load parser '@babel/eslint-parser' declared in '.eslintrc.cjs': Cannot find module '@babel/core/package.json'
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.eslintrc.cjs (1)
10-11: Consider adding stricter accessibility rulesWhile using
plugin:jsx-a11y/recommendedis a good start, consider also extendingplugin:jsx-a11y/strictfor more comprehensive accessibility coverage. This would help catch additional accessibility issues early in development.extends: [ 'plugin:react/recommended', 'eslint:recommended', 'standard', 'plugin:jsx-a11y/recommended', + 'plugin:jsx-a11y/strict' ],package.json (1)
98-98: Consider documenting ESLint configuration strategySince you're adding accessibility linting capabilities, it would be helpful to document:
- The purpose and scope of each ESLint plugin
- How rules are configured (error vs warning levels)
- Any custom rule configurations
This documentation could live in the project's README or a separate CONTRIBUTING.md file.
Would you like me to help create a template for the linting documentation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.eslintrc.cjs(2 hunks)package.json(1 hunks)
🔇 Additional comments (3)
.eslintrc.cjs (2)
21-22: LGTM!
The plugins configuration is correct and properly ordered.
Line range hint 1-67: Verify accessibility rule impact on existing components
Let's ensure these new accessibility rules won't cause unexpected issues in the existing codebase.
✅ Verification successful
ESLint accessibility rules are safe to implement
The codebase already follows good accessibility practices:
- Images have proper alt text (
AvatarPrimitiveImage.tsx) - Anchors have href attributes (
Link.tsx) - ARIA attributes are correctly used in interactive components (
Toggle.tsx,AccordionTrigger.tsx)
Setting these rules to 'warn' is a good balance as it will:
- Help catch any future accessibility issues
- Not break existing code since components already follow these practices
- Allow gradual improvements where needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential accessibility issues in existing components
# Look for img tags without alt attributes
echo "Checking for images without alt attributes..."
rg -l '<img\s+(?!.*alt=)[^>]*>' --type jsx --type tsx
# Look for anchor tags without href or invalid href
echo "Checking for potentially invalid anchors..."
rg -l '<a\s+(?!.*href=)[^>]*>' --type jsx --type tsx
# Look for potential ARIA attribute usage
echo "Checking current ARIA attribute usage..."
rg -l 'aria-[a-zA-Z]+=' --type jsx --type tsx
Length of output: 940
Script:
#!/bin/bash
# Let's try a different approach with simpler patterns
# Check for image elements
echo "Checking for image elements..."
rg -l '<img' --type js --type ts
# Check for anchor elements
echo "Checking for anchor elements..."
rg -l '<a\s' --type js --type ts
# Check for ARIA attributes with simpler pattern
echo "Checking for ARIA attributes..."
rg -l 'aria-' --type js --type ts
# Check for existing JSX files to ensure we're searching in the right place
echo "Checking for JSX/TSX files..."
fd -e jsx -e tsx
Length of output: 6228
Script:
#!/bin/bash
# Let's examine the actual usage in some key files
# Check image elements implementation
echo "Checking image implementations..."
rg -C 3 '<img' src/core/primitives/Avatar/fragments/AvatarPrimitiveImage.tsx
# Check anchor implementation
echo "Checking anchor implementations..."
rg -C 3 '<a\s' src/components/ui/Link/Link.tsx
# Check ARIA usage in components
echo "Checking ARIA implementations in Toggle..."
rg -C 3 'aria-' src/components/ui/Toggle/Toggle.tsx
echo "Checking ARIA implementations in Accordion..."
rg -C 3 'aria-' src/components/ui/Accordion/fragments/AccordionTrigger.tsx
Length of output: 1683
package.json (1)
98-98: Verify ESLint configuration for jsx-a11y
The plugin addition looks good, but let's ensure it's properly configured in the ESLint setup.
✅ Verification successful
jsx-a11y plugin is properly configured
The ESLint configuration in .eslintrc.cjs shows that the jsx-a11y plugin is correctly set up:
- It's included in the
pluginsarray - The recommended configuration is extended via
'plugin:jsx-a11y/recommended' - There are specific jsx-a11y rules configured (
jsx-a11y/alt-textandjsx-a11y/anchor-is-valid)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ESLint configuration for jsx-a11y plugin
# Check if .eslintrc exists and contains jsx-a11y configuration
echo "Checking .eslintrc files..."
fd -e json -e js -e cjs -e yaml -e yml '\.eslintrc' | while read -r config; do
echo "=== $config ==="
cat "$config"
done
# Check for any existing jsx-a11y rules in the codebase
echo -e "\nChecking for existing jsx-a11y rule usage..."
rg -l "jsx-a11y/" .
Length of output: 305
Script:
#!/bin/bash
# Let's check for ESLint config in other common locations and formats
echo "=== Checking for ESLint config in package.json ==="
jq '.eslintConfig' package.json
echo -e "\n=== Checking for .eslintrc.* files with any extension ==="
find . -maxdepth 1 -name ".eslintrc*"
echo -e "\n=== Checking for eslint-related files in config directory ==="
fd -t f "eslint" "config/"
Length of output: 593
Script:
#!/bin/bash
# Let's check the content of .eslintrc.cjs file
echo "=== Content of .eslintrc.cjs ==="
cat .eslintrc.cjs
Length of output: 2078
| 'jsx-a11y/alt-text': 'warn', | ||
| 'jsx-a11y/anchor-is-valid': 'warn', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility rule configuration
While the current configuration is a good start, consider the following improvements:
-
Some critical accessibility rules should be set to 'error' instead of 'warn' to ensure they're not overlooked:
alt-textis crucial for screen readersanchor-is-validprevents broken navigation
-
Consider adding more specific configurations for these rules:
rules: {
- 'jsx-a11y/alt-text': 'warn',
- 'jsx-a11y/anchor-is-valid': 'warn',
+ 'jsx-a11y/alt-text': ['error', {
+ elements: ['img', 'object', 'area', 'input[type="image"]'],
+ img: ['Image'],
+ object: ['Object'],
+ area: ['Area'],
+ 'input[type="image"]': ['InputImage']
+ }],
+ 'jsx-a11y/anchor-is-valid': ['error', {
+ components: ['Link'],
+ specialLink: ['to'],
+ aspects: ['noHref', 'invalidHref', 'preferButton']
+ }],
+ // Additional important a11y rules
+ 'jsx-a11y/aria-props': 'error',
+ 'jsx-a11y/aria-proptypes': 'error',
+ 'jsx-a11y/aria-role': 'error',
+ 'jsx-a11y/role-has-required-aria-props': 'error'
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'jsx-a11y/alt-text': 'warn', | |
| 'jsx-a11y/anchor-is-valid': 'warn', | |
| 'jsx-a11y/alt-text': ['error', { | |
| elements: ['img', 'object', 'area', 'input[type="image"]'], | |
| img: ['Image'], | |
| object: ['Object'], | |
| area: ['Area'], | |
| 'input[type="image"]': ['InputImage'] | |
| }], | |
| 'jsx-a11y/anchor-is-valid': ['error', { | |
| components: ['Link'], | |
| specialLink: ['to'], | |
| aspects: ['noHref', 'invalidHref', 'preferButton'] | |
| }], | |
| // Additional important a11y rules | |
| 'jsx-a11y/aria-props': 'error', | |
| 'jsx-a11y/aria-proptypes': 'error', | |
| 'jsx-a11y/aria-role': 'error', | |
| 'jsx-a11y/role-has-required-aria-props': 'error' |
| "eslint-config-love": "^62.0.0", | ||
| "eslint-config-standard": "17.1.0", | ||
| "eslint-plugin-import": "2.29.1", | ||
| "eslint-plugin-jsx-a11y": "^6.10.2", |
There was a problem hiding this comment.
Fix invalid version number for eslint-plugin-jsx-a11y
The specified version ^6.10.2 is invalid as it doesn't exist. The latest version in the 6.x series is 6.8.0.
Apply this change:
- "eslint-plugin-jsx-a11y": "^6.10.2",
+ "eslint-plugin-jsx-a11y": "^6.8.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "eslint-plugin-jsx-a11y": "^6.10.2", | |
| "eslint-plugin-jsx-a11y": "^6.8.0", |
2 participants
IMP #494
This commit is my first contribution, let me know if I can correct anything.
To test the config:
npx eslint . --ext .js,.jsx,.ts,.tsxSummary by CodeRabbit
New Features
eslint-plugin-jsx-a11y.Chores