Skip to content

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

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.

Sign up for GitHub

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

Open
nanasess wants to merge 4 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4991990
husky+lint-staged で `eslint --fix` をコミット時に実行する
nanasess Feb 6, 2025
7bbb0b7
Merge branch 'master' into use-husky
nanasess Apr 16, 2025
ad16588
Merge branch 'master' into use-husky
nanasess May 7, 2026
929b57d
chore: lint-staged を v17.0.2 に更新
nanasess May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .husky/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
npx lint-staged -q || node -e ''
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026 •
edited
Loading

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

@nanasess nanasess May 7, 2026

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

@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.

@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.

Loading
Loading