Skip to content

eslint: no-explicit-any -> error; +husky pre-commit (lint only); config cleanups - #22

Merged
hibukki merged 7 commits into
mainfrom
eslint-tighten-precommit-hook
Jul 27, 2026
Merged

hibukki merged 7 commits into
mainfrom
eslint-tighten-precommit-hook

Conversation

@hibukki

@hibukki hibukki commented Jul 27, 2026

Copy link
Copy Markdown
Owner

👋 Claude here — three related changes to tighten the dev loop.

1. no-explicit-any: off → error

Owner decision: turn it to error.

Flipped @typescript-eslint/no-explicit-any to "error" and refreshed the surrounding comments, which framed the whole block as overrides that "ease getting into TypeScript, can be removed for stricter linting." That framing is no longer true for no-explicit-any, so it now reads "several rules below are relaxed…" and the honest framing is kept for the rules that remain relaxed (no-unused-vars as warn, the no-unsafe-* group, require-await).

Did it surface real any usages? No. The only explicit any in the repo is in src/routeTree.gen.ts (generated by the TanStack Router plugin), which starts with /* eslint-disable */ and is not linted. pnpm lint passes with zero changes to app code — no eslint-disable dodges were needed.

While in that block I also fixed an adjacent stale comment: ban-ts-comment is set to "error" but was labelled // Allow escaping the compiler (the opposite of what "error" does).

2. Pre-commit hook — lint only, never formatting

Owner: No problem to add them as deps. Running lint on commit sounds good.

Added husky + lint-staged (pnpm exec husky init wired the prepare: husky script and .husky/). .husky/pre-commit runs pnpm exec lint-staged. On staged *.{ts,tsx}, lint-staged.config.js:

  • runs eslint --max-warnings 0 on the staged files, and
  • runs tsc -b to type-check the whole project.

Owner rule: no auto-formatting (it churns files an agent didn't touch and breaks LLM prompt caches).

No formatter runs in the hook. Two implementation notes, both from the lint-staged docs:

  • The config value is a function returning an array, so lint-staged runs each command verbatim without appending filenames. This matters for tsc -b: build mode rejects file arguments, so it must run project-wide, not per-file.
  • eslint gets --no-warn-ignored. The *.{ts,tsx} glob matches by basename, so staging e.g. vite.config.ts (which eslint ignores) would otherwise emit a "file ignored" warning and fail under --max-warnings 0. This is the documented fix for ESLint flat config ≥ 8.51.0.

3. Config cleanups

  • package.json lint script: dropped the dead --ext ts,tsx flag (a no-op under ESLint flat config).
  • Replaced npm-run-all with the maintained fork npm-run-all2; dev now uses its run-p binary.
  • vite.config.ts: added server.strictPort: true so a busy 5173 fails loudly instead of silently drifting to 5174.

Verification

  • pnpm install --frozen-lockfile — up to date
  • pnpm lint — pass
  • pnpm build — pass (pre-existing chunk-size warning only)
  • pnpm test — 5 passed
  • Hook fires on a real commit and runs eslint + tsc, not prettier. Verified with two throwaway staged probes (not committed): an explicit-any file was blocked by eslint; an eslint-clean file with a type error was blocked by tsc -b. A clean commit passes through.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VrVv49ZydmFUGUVbVTGWeN

hibukki and others added 5 commits July 27, 2026 14:22
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VrVv49ZydmFUGUVbVTGWeN
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VrVv49ZydmFUGUVbVTGWeN
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review: eslint no-explicit-any -> error; husky pre-commit (lint only); config cleanups

Overall this is a clean, well-scoped tooling PR. Three independent, low-risk changes bundled together, each explained clearly in the PR body with verification steps. Nice attention to detail on the lint-staged gotchas (function-form config for tsc -b, --no-warn-ignored for flat-config ignored files).

1. no-explicit-any: error

  • Straightforward and matches the stated owner decision. Confirmed the only any in the repo is the generated routeTree.gen.ts, which is excluded via its own /* eslint-disable */, so this requires no code changes elsewhere -- good verification.
  • The comment cleanup ("Several rules below are relaxed...") and the fix to the stale/backwards ban-ts-comment comment is a nice, honest touch -- the old comment text really did contradict the "error" setting.

2. Husky + lint-staged pre-commit hook

  • Lint-only (no formatter) matches the stated project convention against auto-formatting churn -- good.
  • Using the function-returning-array form for lint-staged.config.js is correct and necessary since tsc -b cannot take file arguments; documented with a link back to the lint-staged docs for future readers.
  • --no-warn-ignored addresses a real issue (e.g. vite.config.ts is eslint-ignored but matches the *.{ts,tsx} glob by basename) -- good catch, would otherwise have caused a confusing hook failure.
  • Minor: stagedFiles.join(" ") interpolates filenames directly into a shell command string without quoting. If a staged file path ever contains a space, the eslint invocation would break. Since this is a local dev-only hook (not shipped to prod or run on untrusted input), the risk is minor -- worth noting, not a blocker.
  • tsc -b runs project-wide (not scoped to staged files) on every commit touching a ts/tsx file, so larger codebases could see commit latency grow over time. Not an issue at the current repo size.
  • No automated test coverage for the hook itself (reasonable, since this is git tooling rather than application logic); the PR body documents manual verification (staged probes for eslint and tsc failures, plus a clean-commit pass-through).

3. Config cleanups

  • Dropping the dead --ext ts,tsx flag is correct -- it is a no-op under ESLint flat config.
  • Swapping npm-run-all for the maintained npm-run-all2 fork is reasonable.
  • vite.config.ts: adding server.strictPort: true is a sensible improvement -- failing loudly on a busy port beats silently drifting to 5174, avoiding confusing "why is my change not showing up" moments from a stale dev server still bound to 5173.

Security
No concerns -- all changes are dev-tooling/lint config, nothing touching runtime code paths, auth, or data handling.

Test coverage
Not directly applicable given this PR scope (lint/tooling config). The PR body documents manual pnpm lint/pnpm build/pnpm test verification plus manual hook testing, which is appropriate here.

Summary
Solid, well-documented, low-risk change. The only thing worth a passing thought is the unquoted filename interpolation in lint-staged.config.js, and that is not blocking given the local/dev-only context. Good job keeping the PR scoped and verifying each piece independently.

hibukki and others added 2 commits July 27, 2026 14:37
9.x/17.x floor node at 22.22, breaking our engines ">=22" (hard
fail once engine-strict lands). Newest versions satisfied by 22.0.0:
npm-run-all2@8.0.4 (^20.5.0 || >=22.0.0), lint-staged@16.4.0 (>=20.17).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VrVv49ZydmFUGUVbVTGWeN
Joining with spaces broke on filenames containing spaces (lint-staged
spawns without a shell, so the command string is arg-split on
whitespace). Quote each via JSON.stringify; `--` guards a leading `-`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VrVv49ZydmFUGUVbVTGWeN
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review

Overview

Three related dev-tooling changes: (1) flips @typescript-eslint/no-explicit-any to "error" and tidies the stale comments around the relaxed-rules block, (2) adds a Husky + lint-staged pre-commit hook that runs ESLint and tsc -b (no formatter), and (3) small config cleanups (npm-run-all → npm-run-all2, dropped dead --ext flag, vite.config.ts strictPort: true). Well-scoped and well-documented — the PR description clearly explains rationale and verification steps.

Code quality & style

  • eslint.config.js: the comment fix on ban-ts-comment ("Allow escaping the compiler" → correctly describes "error" now) and the reframing of the block comment ("all of these" → "several below") are good, honest cleanups.
  • lint-staged.config.js follows the documented lint-staged pattern closely (function returning an array so tsc -b doesn't get filenames appended, --no-warn-ignored for flat-config-ignored files like vite.config.ts). The comment explaining why is appreciated since these are non-obvious gotchas.
  • Minor nit: in package.json, husky is added with a caret range (^9.1.7) while lint-staged (16.4.0) and npm-run-all2 (8.0.4) are pinned exact. Not wrong, just an inconsistency in versioning style within the same PR — worth a one-line note on why (recency/stability?) if intentional.

Potential issues / risks

  • vite.config.ts: server.strictPort: true. This is a real behavior change: if port 5173 is already in use (e.g. a leftover pnpm dev from another worktree/session), vite --open now hard-fails instead of silently moving to 5174. Given CLAUDE.md's workflow of running pnpm dev in the background, this could cause friction if a prior dev server wasn't cleanly killed. The fail-loud rationale is reasonable, just flagging the UX trade-off in case it surprises someone mid-session.
  • tsc -b runs project-wide on every commit, not just staged files (as noted in the PR description, this is required since build mode rejects file args). This means a commit touching one small .ts file pays for a full project type-check. Acceptable given the constraint, but worth knowing it'll make commits noticeably slower as the codebase grows.
  • .husky/pre-commit is committed as a plain (non-executable) file in the diff. Husky v9's generated wrapper in .husky/_/ invokes it via sh explicitly, so this should work fine after pnpm install runs prepare: husky — just flagging since older Husky versions required the hook file itself to be chmod +x, in case this trips up any tooling that expects that bit.

Test coverage

No new automated tests, which is appropriate here — this is dev-tooling/config, not application logic. The PR description documents manual verification (throwaway staged probes for any and a type error, confirming eslint/tsc block the commit while a clean commit passes) which is a solid substitute given hooks are hard to unit test.

Security

No concerns. husky/lint-staged/npm-run-all2 are widely-used, actively maintained packages. The command-string construction in lint-staged.config.js (JSON.stringify per staged filename) matches lint-staged's own documented pattern, and lint-staged parses these command strings itself rather than invoking a real shell, so shell metacharacters in filenames aren't a concern here.

Summary

Solid, low-risk tooling PR with good documentation of rationale and verification. The strictPort: true change is the one item worth a second thought given the team's dev workflow; everything else looks safe to merge.

🤖 Generated with Claude Code

@hibukki
hibukki merged commit fe35b0c into main Jul 27, 2026
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