-
-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert Eslint Flat config issues #842
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces a new ESLint configuration file ( Changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.eslintrc.cjs (1)
25-31
: Consider using 'error' for critical rules.All rules are set to 'warn'. Consider upgrading critical rules to 'error':
- 'no-unused-vars'
- 'eqeqeq'
- 'array-callback-return'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (4)
.eslintrc.cjs
(1 hunks)eslint.config.js
(0 hunks)package.json
(4 hunks)rollup.config.cjs
(2 hunks)
💤 Files with no reviewable changes (1)
- eslint.config.js
🧰 Additional context used
🪛 ESLint
.eslintrc.cjs
[error] 98-98: Newline required at end of file but not found.
(eol-last)
🔇 Additional comments (4)
rollup.config.cjs (2)
30-38
: LGTM! Clear plugin configuration.The explicit configuration of typescript and babel plugins with specific options improves maintainability.
54-54
: Verify the impact of removing 'react/jsx-runtime' from external dependencies.The removal of 'react/jsx-runtime' from external dependencies might increase the bundle size unnecessarily.
.eslintrc.cjs (1)
31-58
: Review disabled ESLint rules.Several important rules are turned off without clear justification:
- 'no-undef'
- 'react/prop-types'
- 'react/jsx-key'
- 'no-func-assign'
Consider documenting the rationale for disabling these rules or re-enabling them.
package.json (1)
100-104
: Verify compatibility with downgraded ESLint dependencies.The downgrade of ESLint-related packages might affect existing rules:
- @typescript-eslint/eslint-plugin: 8.0.0 → 7.0.0
- eslint: 8.57.0 → 8.56.0
✅ Verification successful
ESLint dependency downgrades are safe to proceed with.
The codebase doesn't utilize any TypeScript-specific ESLint features. The ESLint configuration uses @babel/eslint-parser and focuses on React and accessibility rules, making it compatible with the downgraded versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any TypeScript-specific lint issues that might be affected echo "Checking TypeScript files..." find . -name "*.ts" -o -name "*.tsx" | while read file; do echo "Analyzing: $file" ast-grep --pattern 'type $_ = $_' "$file" 2>/dev/null doneLength of output: 131070
Script:
#!/bin/bash # Check for ESLint configuration files and their content echo "Checking ESLint configuration..." echo "1. Looking for .eslintrc.* files:" find . -maxdepth 1 -name ".eslintrc*" -type f -exec sh -c 'echo "=== $1 ==="; cat "$1"' sh {} \; echo -e "\n2. Checking package.json for ESLint config:" if [ -f package.json ]; then echo "=== package.json ===" # Extract eslintConfig section if present cat package.json | grep -A 50 '"eslintConfig":' fi echo -e "\n3. Checking for TypeScript-specific ESLint rules in use:" if [ -f .eslintrc.js ]; then cat .eslintrc.js | grep -B1 -A3 "@typescript-eslint/" elif [ -f .eslintrc.json ]; then cat .eslintrc.json | grep -B1 -A3 "@typescript-eslint/" fiLength of output: 4157
'jsx-a11y/scope': 'warn', | ||
'jsx-a11y/tabindex-no-positive': 'warn' | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline at end of file.
ESLint requires a newline at the end of file.
};
+
📝 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.
}; | |
}; | |
🧰 Tools
🪛 ESLint
[error] 98-98: Newline required at end of file but not found.
(eol-last)
package.json
Outdated
@@ -57,9 +57,9 @@ | |||
"prebuild": "npm run test && npm run clean", | |||
"build:css": "npm run generate-tokens && npm run bundle-tokens && npm run build-css", | |||
"build:components": "npm run compile-components && npm run process-components", | |||
"build": "npm run prebuild && npm run build:css && npm run build:components", | |||
"build": "npm run prebuild && npm run build:css && NODE_OPTIONS='--max-old-space-size=4076' npm run build:components", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Memory allocation in build script overrides higher requirement
The build
script sets NODE_OPTIONS='--max-old-space-size=4076'
which overrides the 8076MB allocation needed by the compile-components
script. This environment variable takes precedence, effectively limiting the build process to 4076MB when it requires 8076MB.
- Consider increasing the memory allocation in
build
script to match the 8076MB requirement or higher
🔗 Analysis chain
Review memory allocation settings.
The build script sets --max-old-space-size=4076
while compile-components uses 8076MB. This might cause memory issues during the build process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check available system memory
free -m || echo "Memory info not available"
# Check if there are any reported OOM issues in recent commits
git log -n 10 --grep="memory\|OOM"
Length of output: 165
Script:
#!/bin/bash
# Extract and analyze package.json scripts
echo "=== Package.json Scripts ==="
cat package.json | jq -r '.scripts | to_entries | .[] | select(.value | contains("max-old-space-size")) | "\(.key): \(.value)"'
echo -e "\n=== Build Related Files ==="
fd -e js -e json -e ts -e sh build
echo -e "\n=== Memory Related Documentation ==="
rg -i "memory|heap|space.size" README.md || echo "No memory-related info in README"
Length of output: 701
Script:
#!/bin/bash
# Check the relationship between build:components and compile-components
echo "=== All Build Related Scripts ==="
cat package.json | jq -r '.scripts["build:components"] // empty, .scripts["compile-components"] // empty'
Length of output: 297
Reverting breaking eslint config changes made in #805
Summary by CodeRabbit
Configuration
Chores