Skip to content

chore: add Husky pre-commit hooks for code quality#5

Merged
JoniUzan merged 3 commits intomainfrom
chore/add-husky-pre-commit-hooks
Oct 10, 2025
Merged

chore: add Husky pre-commit hooks for code quality#5
JoniUzan merged 3 commits intomainfrom
chore/add-husky-pre-commit-hooks

Conversation

@JoniUzan
Copy link
Copy Markdown
Owner

Summary

Set up Husky with comprehensive pre-commit hooks that enforce code quality standards by running checks on the entire codebase before each commit.

Why Full Codebase Checks?

Running checks on the entire codebase (instead of just staged files) ensures:

  • Migration safety: Catches breaking changes that affect other files
  • Consistency: Ensures all code meets quality standards, not just new changes
  • Refactoring confidence: Detects issues across the whole project
  • Team alignment: Everyone commits code that passes all quality gates

Pre-commit Hook Checks

The hook runs three comprehensive checks on every commit:

1. 💅 Prettier Formatting

pnpm format

Formats all TypeScript, TSX, and Markdown files with Prettier + Tailwind plugin

2. 📝 ESLint Linting

pnpm lint

Runs ESLint across all packages:

  • @poly/ui
  • @poly/api
  • @poly/storybook
  • web

3. 🔎 TypeScript Type Checking

pnpm --filter @poly/ui check-types
pnpm --filter @poly/api check-types
pnpm --filter web typecheck

Ensures type safety across all TypeScript packages

Changes

Configuration:

  • Add husky@^9.1.7 to pnpm catalog
  • Add "prepare": "husky" script to package.json
  • Create .husky/pre-commit hook with format + lint + type-check

Files:

  • pnpm-workspace.yaml - Added Husky to catalog
  • package.json - Added catalog reference and prepare script
  • .husky/pre-commit - Pre-commit hook script
  • pnpm-lock.yaml - Updated dependencies

Benefits

Prevents bad commits: Catches formatting, linting, and type errors before they're committed
Enforces standards: All code must meet quality checks
Saves CI time: Issues caught locally instead of in CI/CD
Migration-safe: Validates entire codebase, not just changed files
Fast with Turbo: Cached results make repeat checks quick

Testing

Tested successfully:

  • Pre-commit hook runs automatically on commit
  • All checks pass (format, lint, type-check)
  • Hook blocks commit if any check fails
  • No Husky deprecation warnings

Performance Note

With Turbo caching, subsequent runs are very fast (< 1 second when cached). First run takes ~10-15 seconds for full codebase validation.

🤖 Generated with Claude Code

Set up Husky with comprehensive pre-commit checks to enforce code quality
standards across the entire codebase before commits.

**Changes:**
- Add husky@^9.1.7 to pnpm catalog for centralized version management
- Initialize Husky with prepare script in package.json
- Create pre-commit hook that runs on entire codebase:
  - Prettier formatting check
  - ESLint on all packages
  - TypeScript type checking on @poly/ui, @poly/api, and web

**Pre-commit checks:**
✅ Format entire codebase with Prettier
✅ Lint all packages with ESLint
✅ Type check all TypeScript packages

**Benefits:**
- Prevents committing unformatted, unlinted, or type-unsafe code
- Ensures consistent code quality across migrations and refactors
- Catches issues before they reach CI/CD pipeline
- All checks run on full codebase for comprehensive validation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Oct 10, 2025

PR Review: Husky Pre-commit Hooks

Overall, this is a well-implemented PR that adds valuable quality gates to the development workflow. The approach is sound and aligns with the repository's quality standards.

✅ Strengths

  1. Clear Documentation: Excellent PR description with rationale for full codebase checks
  2. Proper Configuration: Uses pnpm catalog for dependency management (consistent with project style)
  3. Comprehensive Checks: Covers formatting, linting, and type-checking across all packages
  4. Good Error Messages: Clear, actionable feedback when checks fail
  5. Correct Setup: Uses modern Husky v9 with the prepare script approach

🔍 Code Quality Observations

Pre-commit Script (.husky/pre-commit:1-34)

  • Clean, readable bash script with proper error handling
  • Good use of emojis for visual feedback (consistent with the repo style based on CLAUDE.md)
  • Sequential checks allow early exit on first failure

⚠️ Potential Issues & Recommendations

1. Missing Shebang Line

The pre-commit hook is missing a shebang. While it works (Git runs hooks with sh), it's best practice to add:

#!/bin/sh

at the very top of .husky/pre-commit:1

2. Performance Consideration

Running full codebase checks on every commit can be slow, especially as the project grows. Consider:

Option A (Recommended for DX): Add a bypass mechanism for quick commits:

# Allow HUSKY_SKIP=1 git commit to bypass
[ -n "$HUSKY_SKIP" ] && exit 0

Option B: Use lint-staged for incremental checks:

# Only check staged files
npx lint-staged

The PR description mentions 10-15 seconds for first run. This is acceptable now, but consider adding documentation about the bypass option for urgent hotfixes.

3. Type Check Script Inconsistency

Different packages use different script names:

  • @poly/ui and @poly/api: check-types
  • web: typecheck

Recommendation: Standardize to one name (e.g., typecheck) across all packages for consistency.

4. Missing Database Generation

The Turbo config shows that check-types depends on db:generate. Consider adding:

echo "🔧 Generating Prisma Client..."
pnpm --filter @poly/database db:generate

before type checks, or document that developers must run pnpm dev first to generate Prisma types.

5. Storybook Package Not Type-Checked

The pre-commit hook checks @poly/ui, @poly/api, and web, but not @poly/storybook. If Storybook has TypeScript, it should be included.

🔒 Security

✅ No security concerns. The hook only runs local development commands with no external dependencies or network calls.

🧪 Test Coverage

The PR description mentions manual testing, which is appropriate for tooling changes. Consider adding a note in CLAUDE.md or README about how to test that hooks are working.

📚 Documentation Suggestions

  1. Add a section to CLAUDE.md about:

    • How to bypass hooks in emergencies: HUSKY_SKIP=1 git commit -m "..."
    • How to run hooks manually: .husky/pre-commit
    • What to do if hooks fail
  2. Consider adding a troubleshooting section for common issues (e.g., "Prisma Client not generated")

🎯 Summary

Verdict: ✅ Approve with minor suggestions

This PR improves code quality and prevents issues from reaching CI. The implementation is solid, but the recommendations above would make it more developer-friendly and consistent with the existing architecture.

Priority Fixes (optional but recommended):

  1. Add shebang to pre-commit script
  2. Document bypass mechanism for emergency commits
  3. Ensure Prisma Client generation happens before type checks

Nice to Have:

  • Standardize type-check script names
  • Include Storybook in type checks if applicable
  • Add hooks documentation to CLAUDE.md

Great work! 🚀

Address review feedback to make the pre-commit hooks more robust, consistent, and developer-friendly.

**Key Improvements:**

1. **Add shebang** - Added #!/bin/sh to pre-commit script for better portability

2. **Standardize typecheck script** - Renamed check-types → typecheck across all packages
   - Updated @poly/ui, @poly/api package.json scripts
   - Added root-level "pnpm typecheck" command that runs Turbo typecheck
   - Updated turbo.json to include typecheck task with db:generate dependency
   - Simplified pre-commit hook to use single "pnpm typecheck" command

3. **Add Prisma Client generation** - Hook now generates Prisma Client before type checks
   - Ensures type safety with latest schema changes
   - Prevents "Prisma Client not generated" errors

4. **Include Storybook type checking** - Now checks all 4 packages: @poly/ui, @poly/api, storybook, web

5. **Comprehensive documentation** - Added Git Hooks section to CLAUDE.md with:
   - What checks run and why
   - How to bypass hooks (emergency only)
   - How to run hooks manually
   - Troubleshooting guide

**Benefits:**
- Consistent script naming across monorepo
- Single command for type checking entire codebase
- Better error messages and documentation
- Includes Storybook in quality checks

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Oct 10, 2025

Pull Request Review: Husky Pre-commit Hooks

✅ Overall Assessment

This is a well-implemented PR that adds comprehensive pre-commit hooks to enforce code quality. The implementation is solid, documentation is thorough, and the approach aligns well with the project's needs.

🎯 Strengths

  1. Excellent Documentation: The CLAUDE.md updates are comprehensive and include troubleshooting steps
  2. Consistent Naming: Great catch on renaming check-typestypecheck across all packages for consistency
  3. Complete Implementation: All necessary files are updated (packages, turbo.json, workspace config)
  4. Proper Dependencies: Husky is correctly added to the pnpm catalog and workspace
  5. Clear Error Messages: The pre-commit hook provides helpful feedback when checks fail
  6. Turbo Integration: Correctly leverages Turbo's caching for performance

🔍 Code Quality

Pre-commit Hook (.husky/pre-commit)

  • ✅ Proper error handling with exit codes
  • ✅ Clear progress indicators
  • ✅ Logical order of checks (format → lint → generate → typecheck)

Configuration Changes

  • ✅ turbo.json properly updated with dependency graph
  • ✅ All package.json files consistently updated
  • ✅ No breaking changes to existing scripts

⚠️ Considerations & Recommendations

1. Hook Execution Order (Minor)

Consider running pnpm format as an auto-fix rather than a blocker:

echo "💅 Running Prettier..."
pnpm format
echo "✓ Code formatted"

Rationale: Prettier can auto-fix most formatting issues, so failing the commit seems harsh. Most teams auto-format and continue.

Alternative approach: Keep it as-is but ensure your team knows about --no-verify for edge cases.

2. Performance Consideration (Minor)

The hook runs db:generate on every commit. While necessary for type safety, consider:

  • Current approach: ✅ Safe, ensures Prisma Client is always up-to-date
  • Alternative: Only run if schema files changed (but adds complexity)

Verdict: Keep current approach - Turbo caching makes this fast when schema hasn't changed.

3. Missing Test Hook (Question)

The PR mentions pnpm test in CLAUDE.md, but tests aren't run in the pre-commit hook. Was this intentional?

Recommendation:

  • If tests are fast (< 10s): Add pnpm test to pre-commit
  • If tests are slow: Keep them for CI only (current approach seems fine)

4. Documentation Clarity (Nitpick)

In CLAUDE.md line 25-30, the bypass instructions are good, but consider adding a note about when it's acceptable:

### Bypassing Hooks (Emergency Only)

⚠️ **Only bypass hooks for**:
- Hotfix commits during production incidents
- WIP commits on feature branches (then fix before PR)

If you need to bypass hooks in an emergency:

🐛 Potential Issues

None Found

No bugs, security issues, or blocking problems identified.

🧪 Test Coverage

  • ✅ Manual testing documented in PR description
  • ⚠️ No automated tests for the hook itself (acceptable for this type of change)

🔒 Security

  • ✅ No security concerns
  • ✅ Husky v9 is the latest major version
  • ✅ No credentials or sensitive data in hook

📊 Performance

  • ✅ Turbo caching properly configured
  • ✅ Expected ~1s for cached runs, 10-15s for full checks (reasonable)
  • ✅ Dependency graph in turbo.json ensures efficient execution

🎨 Style & Conventions

  • ✅ Follows project conventions from CLAUDE.md
  • ✅ Consistent command naming across packages
  • ✅ Proper use of pnpm workspace syntax

📝 Final Recommendation

APPROVE

This PR is ready to merge. The implementation is solid, well-documented, and follows best practices. The minor considerations above are suggestions for potential future improvements, not blockers.

Optional Pre-merge Actions:

  1. Consider auto-fixing format instead of blocking (team preference)
  2. Clarify test strategy (pre-commit vs CI only)

Post-merge:

  • Monitor first few commits to ensure hooks don't cause friction
  • Gather team feedback on whether pnpm format should block or auto-fix

Great work! This will significantly improve code quality consistency across the team. 🚀

Reviewed by Claude Code

Change pre-commit hook to automatically format files with Prettier and
re-stage them, rather than blocking the commit on formatting issues.

**Changes:**
- Prettier now auto-formats files during pre-commit
- Formatted files are automatically re-staged with `git add -u`
- Commit proceeds seamlessly without manual formatting intervention

**Benefits:**
- Better developer experience - no interruption for formatting
- Files are always properly formatted before commit
- Developers don't need to manually run format and re-commit
- Lint and type checks still block commits (as they should)

**Behavior:**
- Auto-fix: Formatting (non-blocking)
- Block: Lint, Prisma generation, type checks (blocking)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Oct 10, 2025

Code Review: Husky Pre-commit Hooks Setup

Overall Assessment

This is a well-implemented PR that adds essential code quality gates. The implementation is clean, well-documented, and follows best practices.

Strengths

  • Excellent documentation updates to CLAUDE.md with troubleshooting
  • Consistent naming: renamed check-types to typecheck across all packages
  • Proper hook structure with clear error messages
  • Correct Husky v9 setup using modern prepare script

Critical Issue: Git State Problem

Location: .husky/pre-commit:8

Using git add -u only re-stages modified tracked files, but won't stage new files. Consider either:

  1. Using git diff --name-only --cached | xargs git add
  2. Or safer: Use pnpm format --check instead of auto-fixing

This avoids complexity and makes developers explicitly format.

Medium Issues

  1. Performance: Full codebase checks may slow down with growth (already documented well)
  2. Documentation: Missing initial setup instructions for new developers
  3. Error messages: Could be more actionable (include fix commands)

Minor Suggestions

  • Add execution timing to show performance impact
  • Consider progress indicators for long-running checks

Security & Testing

  • No security concerns
  • Test coverage N/A for infrastructure changes

Code Quality Score: 8.5/10

  • Documentation: 10/10
  • Implementation: 8/10 (git add -u concern)
  • Consistency: 10/10
  • Error Handling: 7/10

Approval: Ready to merge after fixing git add -u issue

Great work on improving the development workflow!

@JoniUzan JoniUzan merged commit 191c3bc into main Oct 10, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant