-
-
Notifications
You must be signed in to change notification settings - Fork 10
build(deps-dev): Migrate husky and lint-staged to lefthook (#2988) #2995
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
base: staging
Are you sure you want to change the base?
Conversation
WalkthroughReplaces Husky+lint-staged with Lefthook: removes lint-staged invocation and Husky wiring, adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (git commit)
participant Git as Git
participant LH as Lefthook (pre-commit)
participant Format as Prettier
participant Lint as ESLint
Dev->>Git: git commit (staged files)
Git->>LH: invoke pre-commit hooks
par Parallel jobs
LH->>Format: run format job (prettier --write on staged globs)
LH->>Lint: run lint job (prettier --check + eslint on staged globs)
end
Format-->>LH: format results (files may be modified)
Lint-->>LH: lint results (success/fail)
LH->>Git: return combined exit status (pass → commit / fail → abort)
Note right of LH: Lefthook orchestrates parallel jobs and aggregates results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 (4)
.husky/_/pre-commit (2)
65-67: Consider adding an error exit when lefthook is not found.When lefthook cannot be found, the script prints a message but doesn't exit with an error code. This allows commits to proceed without running the pre-commit checks.
If this is intentional for graceful degradation, consider adding a comment explaining the design choice. Otherwise, add
exit 1after the echo to enforce the hook.Proposed fix to exit with error
else echo "Can't find lefthook in PATH" + exit 1 fi
19-21: Hardcoded Docker path is covered by extensive fallbacks.Lines 19-21 contain a version-specific path
/usr/src/app/node_modules/.pnpm/[email protected]/..., but this is just one fallback in a chain of 15+ strategies. More robust fallbacks follow on lines 23-67 using dynamic architecture detection (lefthook-${osArch}-${cpuArch}) without version pinning. The script will continue to function on lefthook upgrades as it will attempt multiple location strategies before failing.If targeting Docker environments specifically, consider that the hardcoded path will only work until lefthook is upgraded in that Docker image. For long-term maintenance, ensure the Docker image uses a generic fallback path or regenerates these wrapper scripts after updating lefthook.
.husky/_/prepare-commit-msg (1)
11-69: Duplicate code with.husky/_/pre-commit.The
call_lefthookfunction is identical in both wrapper scripts. If these scripts are manually maintained (not auto-generated by lefthook), consider extracting the shared logic to reduce duplication.However, if these are auto-generated by
lefthook install, this duplication is expected and acceptable.docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md (1)
130-134: Plan mentions deleting.husky/but wrapper scripts still exist.The plan states "
.husky/ディレクトリ削除" (delete .husky/ directory), but the implementation keeps.husky/_/pre-commitand.husky/_/prepare-commit-msgas wrapper scripts. This is likely the correct approach for Lefthook integration, but the plan document should be updated to reflect the actual implementation for accuracy.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.husky/_/pre-commit.husky/_/prepare-commit-msg.husky/pre-commitCONTRIBUTING.mddocs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.mdlefthook.ymlpackage.json
💤 Files with no reviewable changes (1)
- .husky/pre-commit
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md
139-139: Spaces inside code span elements
(MD038, no-space-in-code)
🔇 Additional comments (5)
lefthook.yml (1)
1-12: Configuration looks good for the migration goals.The Lefthook configuration properly leverages parallel execution and staged-file scoping to achieve the performance improvements documented in the plan. A few observations:
- The separation of
format(write) andlint(check) jobs with parallel execution is a good approach.- The glob patterns correctly target the project's file types, with
.mdappropriately excluded from linting.- Using
pnpm execensures the local toolchain is used consistently.CONTRIBUTING.md (1)
272-299: Documentation is clear and comprehensive.The new Git Hooks section effectively covers:
- Pre-commit hook behavior (format and lint)
- How to bypass hooks when needed (
LEFTHOOK=0)- Migration steps for existing users transitioning from husky
The migration instructions at lines 294-297 are particularly helpful for ensuring smooth team transitions.
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md (2)
214-246: Excellent performance results and lessons learned.The documented 97.7% reduction in pre-commit hook execution time (30s → 0.7s) demonstrates the migration's success. The lessons learned section provides valuable insights for future reference:
- Direct command invocation vs npm scripts
- Parallel execution benefits
- Git config cleanup requirements
This documentation will be helpful for team members and future maintenance.
139-139: Fix spacing in code span per static analysis.Per markdownlint, remove the space inside the code span element.
Proposed fix
-- [ ] テスト commit で動作確認 **1h** +- [ ] テストcommitで動作確認 **1h**Note: The static analysis hint points to "Spaces inside code span elements" - if there are inline code spans with internal spaces elsewhere, those should also be checked.
Likely an incorrect or invalid review comment.
package.json (1)
46-46: Lefthook 2.0.12 is the latest stable version and correctly specified.The migration from husky/lint-staged to lefthook is properly reflected in the dependencies, with the dependency pinned to the current latest release. The removal of the
preparescript and lint-staged configuration aligns with the migration plan.
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md
Outdated
Show resolved
Hide resolved
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: 1
♻️ Duplicate comments (1)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md (1)
96-96: Typo: Chinese character used instead of Japanese.The text uses "设定" (Chinese simplified) instead of "設定" (Japanese). This was previously flagged but remains unfixed.
🔎 Proposed fix
-**結論:** `exclude` は削除し、设定ファイルをシンプルに保つ +**結論:** `exclude` は削除し、設定ファイルをシンプルに保つ
🧹 Nitpick comments (1)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md (1)
196-196: Consider using a proper heading for structural clarity.The text "実装開始可能" uses emphasis (bold) rather than a heading. While this may be intentional for emphasis, using a heading would improve document structure and address the markdownlint hint.
🔎 Optional formatting adjustment
-**実装開始可能** +### 実装開始可能
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.husky/pre-commitCONTRIBUTING.mddocs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.mdlefthook.ymlpackage.json
💤 Files with no reviewable changes (1)
- .husky/pre-commit
🚧 Files skipped from review as they are similar to previous changes (2)
- CONTRIBUTING.md
- lefthook.yml
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md
196-196: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview
🔇 Additional comments (3)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md (2)
214-223: Excellent performance improvements achieved!The migration successfully achieved the goal of sub-5-second execution time, with a 97.7% reduction from ~30s to ~0.7s. The performance metrics are internally consistent and well-documented, clearly demonstrating the value of this migration.
224-249: Comprehensive and valuable lessons documented.The implementation lessons provide excellent insights, particularly the clarification that direct command invocation (not npm scripts) is required for staged-files-only processing. These lessons will be valuable for the team and future maintenance.
package.json (1)
46-46: No action required. The lefthook version 2.0.12 specified is the latest stable version available on npm and is not deprecated.
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md
198-198: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (3)
docs/dev-notes/2025-12-22/migrate-from-husky-and-lint-staged-to-lefthook/plan.md (3)
157-172: Configuration implementation aligns with lessons learned.The
lefthook.ymlconfiguration correctly uses direct command invocation (pnpm exec prettier --write {staged_files}) rather than npm scripts, which aligns with the documented lesson (lines 232–234) that npm scripts cannot process staged files selectively. The use ofpnpm execpreserves the npm context while enabling stage-file-only processing. Well done on addressing the configuration consistency feedback from the earlier review.
216-248: Performance metrics and lessons learned demonstrate thorough validation.The post-implementation report is well-structured with concrete measurements (97.7% reduction), clear lessons learned, and actionable insights for future decisions. The emphasis on direct command invocation, parallel execution benefits, and Git config cleanup provides valuable context for maintenance and troubleshooting.
59-114: Q&A section effectively addresses implementation concerns.The Q&A addresses practical considerations (npm scripts, Prettier separation, gitignore handling, lefthook config scope, optional package.json commands) with clear rationale. This reduces ambiguity during implementation and helps maintainers understand the migration decisions.
close #2988
Summary by CodeRabbit
Refactor
Chores
Documentation
Impact
✏️ Tip: You can customize this high-level summary in your review settings.