Skip to content

husky+lint-staged で eslint --fix をコミット時に実行する#1168

Open
nanasess wants to merge 4 commits into
EC-CUBE:masterfrom
nanasess:use-husky
Open

husky+lint-staged で eslint --fix をコミット時に実行する#1168
nanasess wants to merge 4 commits into
EC-CUBE:masterfrom
nanasess:use-husky

Conversation

@nanasess
Copy link
Copy Markdown
Contributor

@nanasess nanasess commented Feb 6, 2025

husky+lint-staged でコミット時に *.ts ファイルを自動整形する

see EC-CUBE/ec-cube#4661

  • 基本は上記4系のPRを踏襲
  • 以下の方法は husky v5 以降使えなくなったようなので、 .husky/pre-commit を使用する
"husky": {
    "hooks": {
      "pre-commit": "lint-staged 2>&1 | echo || node -e ''"
    }
  },
  • lint-staged 2>&1 | echo が動作しないため lint-staged -q に変更

refs EC-CUBE/ec-cube#6337

Summary by CodeRabbit

リリースノート

  • その他の変更
    • 開発環境のコード品質管理ツールを設定しました。

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.30%. Comparing base (f202366) to head (929b57d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1168   +/-   ##
=======================================
  Coverage   55.30%   55.30%           
=======================================
  Files          87       87           
  Lines       11089    11089           
=======================================
  Hits         6133     6133           
  Misses       4956     4956           
Flag Coverage Δ
tests 55.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nanasess nanasess enabled auto-merge April 16, 2025 21:07
master の webpack→esbuild 移行 (npm 化) に追従:
- package.json: master ベースに husky/lint-staged を再追加
- yarn.lock 削除 (master の選択を採用)
- .husky/pre-commit: yarn → npx に変更
- package-lock.json: husky/lint-staged を反映
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Warning

Rate limit exceeded

@nanasess has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 47 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 73a0c0dc-7815-4d51-8745-7e6a43080d05

📥 Commits

Reviewing files that changed from the base of the PR and between ad16588 and 929b57d.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json
📝 Walkthrough

Walkthrough

Husky と lint-staged を設定して、Git プリコミットフック経由でステージング済みの TypeScript ファイルに対して自動的に ESLint を実行するツールチェーンを導入しました。

Changes

Pre-commit Linting Setup

Layer / File(s) Summary
Package Dependencies
package.json
devDependencieshuskylint-staged を追加し、prepare スクリプトで Husky を初期化します。
Lint Configuration
package.json
lint-staged 設定を追加して、ステージング済みの *.ts ファイルに対して eslint --fix を実行します。
Pre-commit Hook
.husky/pre-commit
npx lint-staged -q を実行するプリコミットフックを実装し、エラー時はノーオペレーション(node -e '')で処理を続行します。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰✨ ステージの守り手、Husky は走る
lint-staged の鐘が鳴り、ESLint がコードを整える
コミット前の品質チェック、自動でスイスイと
バグは逃げ出す、修正は輝く
さあ、綺麗なコードで前へ進もう 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PRのタイトルは、husky と lint-staged を使用して eslint --fix をコミット時に実行することを明確に説明しており、変更セットの主要な目的と完全に一致しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.husky/pre-commit:
- Line 1: 現在の pre-commit フック contains the fallback "npx lint-staged -q || node
-e ''" which swallows failures; remove the "|| node -e ''" fallback so that the
command "npx lint-staged -q" can fail the hook, i.e. keep only the lint-staged
invocation (or replace the fallback with an explicit non-zero exit if needed) so
ESLint/lint-staged errors cause the pre-commit to fail; update the line that
currently reads "npx lint-staged -q || node -e ''" accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 05be8981-f185-46e0-86b3-d6c0c9357ab2

📥 Commits

Reviewing files that changed from the base of the PR and between f202366 and ad16588.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • .husky/pre-commit
  • package.json

Comment thread .husky/pre-commit
@@ -0,0 +1 @@
npx lint-staged -q || node -e ''
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1) 現在のフック内容確認(リポジトリ内)
sed -n '1,40p' .husky/pre-commit

# 2) シェル評価の事実確認(false が右辺で成功に変わる)
bash -lc "false || node -e ''"; echo "exit_code=$?"
# 期待値: exit_code=0

Repository: EC-CUBE/ec-cube2

Length of output: 222


|| node -e '' により pre-commit 失敗が握りつぶされています

lint-staged / eslint が失敗してもフック全体が成功終了し、問題のある変更がそのままコミット可能になります。PR 目的(コミット時の自動整形・検証)と逆の挙動です。

修正案
-npx lint-staged -q || node -e ''
+npx lint-staged -q
📝 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.

Suggested change
npx lint-staged -q || node -e ''
npx lint-staged -q
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.husky/pre-commit at line 1, 現在の pre-commit フック contains the fallback "npx
lint-staged -q || node -e ''" which swallows failures; remove the "|| node -e
''" fallback so that the command "npx lint-staged -q" can fail the hook, i.e.
keep only the lint-staged invocation (or replace the fallback with an explicit
non-zero exit if needed) so ESLint/lint-staged errors cause the pre-commit to
fail; update the line that currently reads "npx lint-staged -q || node -e ''"
accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます。

|| node -e '' は意図的に残しています。本 PR は EC-CUBE 4系 PR #4661 および #6337 の方針を踏襲したもので、4系PRの方針に以下の記述があります。

  • 実行要件(php + node.js)を満たさない環境の場合は何も起こらないようにする。
  • コード整形に失敗した際にcommit不可となってしまうのを避けるため、整形不可のコードがcommit対象だった場合もフック成功とみなす。

本 PR の目的は「コミット時の自動整形」のみで、lint エラーで commit をブロックする意図はありません(eslint --fix が修正できないコードをコミット禁止にするのは過剰と判断)。
そのため || node -e '' は維持します。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanasess ご説明ありがとうございます。意図的な設計であることを理解しました。

EC-CUBE 4系の方針(環境要件を満たさない場合は何もしない・整形失敗時もコミットをブロックしない)を踏襲した設計であることを承知しました。この方針は理にかなっており、指摘を取り下げます。


✏️ Learnings added
Learnt from: nanasess
Repo: EC-CUBE/ec-cube2 PR: 1168
File: .husky/pre-commit:1-1
Timestamp: 2026-05-07T00:17:52.020Z
Learning: In the EC-CUBE/ec-cube2 repository, the `.husky/pre-commit` hook intentionally uses `npx lint-staged -q || node -e ''` to always exit successfully. This follows the EC-CUBE v4 policy (PRs `#4661` and `#6337`): (1) do nothing if the environment doesn't meet requirements (php + node.js not available), and (2) do not block commits when auto-formatting fails. The purpose of the hook is auto-formatting only (`eslint --fix`), not enforcing lint compliance as a gate. The `|| node -e ''` fallback must NOT be flagged as swallowing errors — it is intentional design.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

- v15 → v17 へメジャーアップデート
- v17 は Node.js 22.22.1+ 必須 / Git 2.32.0+ 必須
- husky は ^9.1.7 (最新) のまま
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