Feat/configure husky and lintstaged #492
Conversation
…com/caushcani/postiz-app into feat/configure-husky-and-lintstaged
|
@caushcani is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces a comprehensive linting and code quality improvement strategy across the project. The changes implement Husky and lint-staged to automatically run ESLint and Prettier on staged files before commits. Configuration files like Changes
Possibly related issues
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 6
🧹 Nitpick comments (2)
.lintstagedrc.json (1)
1-7: Consider adding additional quality checks.To enhance code quality, consider adding these checks to your lint-staged configuration:
- Type checking for TypeScript files
- Unit test execution for changed files
Example configuration:
{ "*.{js,jsx,ts,tsx}": [ "eslint", "prettier --write", "tsc --noEmit", "jest --bail --findRelatedTests" ], "*.{json,md,yml,yaml}": ["prettier --write"] }apps/cron/.lintstagedrc (1)
1-6: LGTM! Configuration follows best practices.The lint-staged configuration correctly sets up pre-commit hooks to run ESLint and Prettier on TypeScript and JavaScript files.
Consider adding configurations for other file types:
{ "*.tsx": ["eslint", "prettier --write"], "*.ts": ["eslint", "prettier --write"], "*.js": ["eslint", "prettier --write"], "*.jsx": ["eslint", "prettier --write"], + "*.json": ["prettier --write"], + "*.css": ["prettier --write"], + "*.md": ["prettier --write"] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.eslintrc.json(1 hunks).husky/pre-commit(1 hunks).lintstagedrc.json(1 hunks)apps/backend/.eslintrc.json(1 hunks)apps/backend/.lintstagedrc(1 hunks)apps/commands/.lintstagedrc(1 hunks)apps/cron/.lintstagedrc(1 hunks)apps/frontend/.eslintrc.json(1 hunks)apps/frontend/.lintstagedrc(1 hunks)apps/frontend/src/components/settings/github.component.tsx(2 hunks)apps/workers/.lintstagedrc(1 hunks)package.json(3 hunks)tsconfig.base.json(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- apps/workers/.lintstagedrc
- tsconfig.base.json
- apps/frontend/.lintstagedrc
- apps/frontend/src/components/settings/github.component.tsx
- apps/backend/.lintstagedrc
🔇 Additional comments (5)
apps/backend/.eslintrc.json (1)
8-11: Consider the impact of strict error catching configuration.
The caughtErrors: "all" setting for no-unused-vars will enforce handling of all caught errors. While this promotes better error handling, it might be too strict for some cases where error details aren't needed.
Let's check the impact on existing error handling code:
Consider allowing exceptions for specific cases:
"@typescript-eslint/no-unused-vars": [
"error",
- { "caughtErrors": "all" }
+ {
+ "caughtErrors": "all",
+ "argsIgnorePattern": "^_",
+ "caughtErrorsIgnorePattern": "^_"
+ }
]✅ Verification successful
Based on the results, I can now generate the final response:
The strict error catching configuration appears safe to implement
The codebase shows consistent and proper error handling patterns in try-catch blocks. All identified catch blocks either:
- Log the error (commands/main.ts)
- Transform errors into meaningful user messages (configuration.checker.ts)
- Have intentional error recovery logic (codes.service.ts)
The caughtErrors: "all" setting won't cause issues as there are no instances of unused error variables in catch blocks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for try-catch blocks with unused error variables
ast-grep --pattern 'try {
$$$
} catch (error) {
$$$
}'
Length of output: 2606
.eslintrc.json (2)
9-9: LGTM! Well-balanced configuration for unused variables.
The configuration allows for intentionally unused variables with underscore prefix, which is a common convention.
17-17: Good progressive approach to handling 'any' types.
Changing @typescript-eslint/no-explicit-any to warning level is a good intermediate step towards stricter type safety.
Let's check the current usage of 'any' types:
✅ Verification successful
The previous ast-grep patterns might not have worked well, so let's try with ripgrep to:
- Find explicit 'any' type annotations in TypeScript files
- Check the compiler configuration for implicit any handling
Let's try a different approach since the previous file type specifications weren't recognized. We'll:
- Search for explicit 'any' usage without file type restrictions
- Check what TypeScript files exist in the codebase
Based on the search results, I can now generate the final response since we have found extensive usage of any types throughout the codebase:
Warning level for 'any' types is appropriate given current usage patterns
The warning level (1) for @typescript-eslint/no-explicit-any is indeed a good intermediate step, as the codebase currently has significant usage of any types across multiple areas:
- Form components with error handling
- Event handlers in various UI components
- API response handling
- Generic props and state management
- Upload and file handling utilities
A strict error level would be too disruptive at this stage, while the warning level will help highlight areas for gradual improvement without breaking the build.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for explicit 'any' type usage
ast-grep --pattern '
$x: any
'
# Also check for implicit any in function parameters
ast-grep --pattern '
function $f($p) {
$$$
}
'
Length of output: 91
Script:
#!/bin/bash
# Search for explicit 'any' type usage with ripgrep
rg ": any" --type ts --type tsx -A 1
# Search for implicit any usage
rg "noImplicitAny" --type json
Length of output: 143
Script:
#!/bin/bash
# Search for explicit 'any' type usage with ripgrep, without file type specification
rg ": any"
# List TypeScript files to understand the codebase structure
fd "\.(ts|tsx)$"
Length of output: 42676
package.json (2)
31-32: LGTM! Husky prepare script is correctly configured
The prepare script is properly placed after postinstall and will set up Git hooks during package installation.
190-190: Verify peer dependencies for the linting tools
The versions of husky (^9.1.7) and lint-staged (^15.2.11) are up to date. However, let's verify their peer dependencies are satisfied.
Also applies to: 195-195
✅ Verification successful
Peer dependencies are satisfied for both packages
Looking at the package.json, all necessary peer dependencies are properly configured:
- For husky (^9.1.7): It has no peer dependencies as of this version
- For lint-staged (^15.2.11): It requires Node.js >= 18 which is satisfied by the project's environment as evidenced by the TypeScript and other modern tooling versions present in the dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check peer dependencies for husky and lint-staged
npm info [email protected] peerDependencies
npm info [email protected] peerDependencies
Length of output: 87
Script:
#!/bin/bash
# Let's check the actual package.json content to see the dependencies
cat package.json | jq '.dependencies, .devDependencies'
Length of output: 4987
| @@ -0,0 +1 @@ | |||
| npx lint-staged | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add shell configuration and error handling to the pre-commit hook.
The pre-commit hook should include proper shell configuration for reliability.
Apply this diff to improve the hook's robustness:
+#!/usr/bin/env sh
+. "$(dirname -- "$0")/_/husky.sh"
+
+# Exit on error
+set -e
+
npx lint-staged📝 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.
| npx lint-staged | |
| #!/usr/bin/env sh | |
| . "$(dirname -- "$0")/_/husky.sh" | |
| # Exit on error | |
| set -e | |
| npx lint-staged |
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "*.tsx,*.jsx":["eslint", "prettier --write"] | |||
There was a problem hiding this comment.
Fix JSON syntax and expand file type coverage.
The current configuration has several issues:
- Invalid glob pattern syntax (missing space after comma)
- Missing TypeScript (*.ts) files configuration
- Limited file type coverage
Apply this diff to fix and enhance the configuration:
- "*.tsx,*.jsx":["eslint", "prettier --write"]
+ "*.{tsx,jsx,ts}": ["eslint", "prettier --write"],
+ "*.{json,md,yml,yaml}": ["prettier --write"]📝 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.
| "*.tsx,*.jsx":["eslint", "prettier --write"] | |
| "*.{tsx,jsx,ts}": ["eslint", "prettier --write"], | |
| "*.{json,md,yml,yaml}": ["prettier --write"] |
| { | ||
| "*.tsx": ["eslint", "prettier --write"], | ||
| "*.ts": ["eslint", "prettier --write"], | ||
| "*.js": ["eslint", "prettier --write"], | ||
| "*.jsx": ["eslint", "prettier --write"] | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize configuration and expand coverage.
The current configuration can be simplified and enhanced:
- Consolidate duplicate configurations using glob patterns
- Add support for other common file types
- Remove trailing whitespace
Apply this diff to optimize the configuration:
{
- "*.tsx": ["eslint", "prettier --write"],
- "*.ts": ["eslint", "prettier --write"],
- "*.js": ["eslint", "prettier --write"],
- "*.jsx": ["eslint", "prettier --write"]
+ "*.{js,jsx,ts,tsx}": ["eslint", "prettier --write"],
+ "*.{json,md,yml,yaml}": ["prettier --write"]
}
- 📝 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.
| { | |
| "*.tsx": ["eslint", "prettier --write"], | |
| "*.ts": ["eslint", "prettier --write"], | |
| "*.js": ["eslint", "prettier --write"], | |
| "*.jsx": ["eslint", "prettier --write"] | |
| } | |
| { | |
| "*.{js,jsx,ts,tsx}": ["eslint", "prettier --write"], | |
| "*.{json,md,yml,yaml}": ["prettier --write"] | |
| } |
💡 Codebase verification
Inconsistent lint-staged configurations detected
The root .lintstagedrc.json configuration differs from the one in package.json:
.lintstagedrc.jsonruns eslint first, then prettierpackage.jsonruns prettier first, then eslint with --fix flag.lintstagedrc.jsonhandles file types separately whilepackage.jsonuses a glob pattern
This inconsistency should be resolved by:
- Using a single configuration source (either
.lintstagedrc.jsonorpackage.json) - Standardizing the order of operations (prettier vs eslint)
- Deciding on whether to use the --fix flag with eslint
🔗 Analysis chain
Verify consistent lint-staged configuration across the monorepo.
There are different lint-staged configurations in the root and apps/commands directory. This might lead to inconsistent behavior.
Run this script to check for configuration consistency:
Let me try a different approach to find the lint-staged configurations, as they might be defined in package.json files as well.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for lint-staged configurations across the monorepo
# and verify their consistency
echo "Finding all lint-staged configuration files:"
fd -e json -e js -e yaml -e yml --glob '*lintstagedrc*'
echo -e "\nComparing configurations for consistency:"
for config in $(fd -e json -e js -e yaml -e yml --glob '*lintstagedrc*'); do
echo -e "\n=== $config ==="
cat "$config"
done
Length of output: 314
Script:
#!/bin/bash
# Search for lint-staged configurations in both dedicated config files and package.json
echo "Searching for dedicated lint-staged config files:"
fd -e json -e js -e yaml -e yml --glob '*lintstagedrc*'
echo -e "\nSearching for lint-staged configurations in package.json files:"
rg -l '"lint-staged"' -g 'package.json'
echo -e "\nContent of package.json files containing lint-staged config:"
for pkg in $(rg -l '"lint-staged"' -g 'package.json'); do
echo -e "\n=== $pkg ==="
cat "$pkg" | jq '.["lint-staged"]'
done
Length of output: 775
| "@typescript-eslint/no-unused-vars": [ | ||
| "error", | ||
| { "caughtErrors": "all" } | ||
| ] |
There was a problem hiding this comment.
💡 Codebase verification
Consider aligning with root configuration for unused vars
The root ESLint configuration already has a more flexible no-unused-vars rule that ignores variables and parameters starting with underscore. The stricter setting in apps/frontend and apps/backend with caughtErrors: "all" creates inconsistency. Consider removing the override to maintain consistency with the root configuration:
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_", "varsIgnorePattern": "^_" }]apps/frontend/.eslintrc.json: Remove the stricter overrideapps/backend/.eslintrc.json: Remove the stricter override
🔗 Analysis chain
Consider a less strict setting for caught errors
While enabling @typescript-eslint/no-unused-vars is a good practice, the "caughtErrors": "all" setting will flag all unused error parameters in catch blocks. Consider using "caughtErrors": "none" if you have legitimate cases where error parameters are intentionally unused.
Let's verify ESLint configuration consistency across the monorepo:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for ESLint configs and their no-unused-vars rules
fd -e json -e js --exec sh -c 'echo "=== {} ===" && cat {} | grep -A 5 "no-unused-vars"' .eslintrc
Length of output: 1776
Script:
#!/bin/bash
# Search for ESLint config files and show their content
fd -e json -g '*eslint*' -x cat {}
Length of output: 38
Script:
#!/bin/bash
# Try finding eslint configs using ripgrep
rg --type-add 'eslint:*.{json,js}' --type eslint -l "eslintConfig|\"rules\":|extends" .
Length of output: 1112
Script:
#!/bin/bash
# Check content of all ESLint config files
for file in $(find . -name ".eslintrc.json"); do
echo "=== $file ==="
cat "$file"
echo -e "\n"
done
Length of output: 16187
| "lint-staged": { | ||
| "**/*.{js,ts,jsx,tsx}": [ | ||
| "prettier --write", | ||
| "eslint --fix" | ||
| ] |
There was a problem hiding this comment.
💡 Codebase verification
Expanding lint-staged configuration is recommended
The repository contains numerous JSON files (package.json, tsconfig files, project configurations), markdown files (README.md, CONTRIBUTING.md, etc.), and YAML files (docker-compose.dev.yaml) that would benefit from consistent formatting. The Husky pre-commit hook is properly configured to run lint-staged, making this the perfect setup to enforce consistent formatting across all these file types.
- Add prettier formatting for JSON files to maintain consistent configuration files
- Add prettier formatting for MD files to keep documentation consistently formatted
- Add prettier formatting for YAML files to ensure consistent Docker configurations
🔗 Analysis chain
Consider expanding lint-staged configuration
While the current setup handles JS/TS files well, consider adding configurations for other file types:
"lint-staged": {
"**/*.{js,ts,jsx,tsx}": [
"prettier --write",
"eslint --fix"
],
+ "**/*.{json,md,yml,yaml}": [
+ "prettier --write"
+ ]
}Let's verify if the Husky setup is complete:
Let me verify if there are any JSON, MD, YAML files in the repository that would benefit from prettier formatting.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for Husky pre-commit hook configuration
if [ -d ".husky" ]; then
echo "=== .husky/pre-commit ==="
cat .husky/pre-commit
else
echo "Warning: .husky directory not found. Run 'npx husky install' to complete setup"
fi
Length of output: 120
Script:
#!/bin/bash
# Check for presence of JSON, MD, YAML files
echo "=== JSON files ==="
fd -e json
echo -e "\n=== MD files ==="
fd -e md
echo -e "\n=== YAML/YML files ==="
fd -e yml
fd -e yaml
# Check if prettier is configured for these file types
echo -e "\n=== Prettier Config ==="
fd -e json -e js -e yaml -e yml '.prettierrc' -x cat {}
Length of output: 1513
What kind of change does this PR introduce?
Feature.
Why was this change needed?
#488
Other information:
eg: Did you discuss this change with anybody before working on it (not required, but can be a good idea for bigger changes). Any plans for the future, etc?
Yes, with @nevo-david.
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
Summary by CodeRabbit
New Features
Bug Fixes
github.component.tsxfile for better clarity.Documentation
Chores