Skip to content

Conversation

@CodeMonkeyCybersecurity
Copy link
Owner

Summary

Fixes ESLint pre-commit hook failure caused by global TypeScript ESLint config taking precedence over project config.

Root Cause Analysis (5 Whys)

  1. Why did the hook fail? ESLint threw @typescript-eslint/no-unused-expressions error
  2. Why was TypeScript ESLint used? Global ~/eslint.config.mjs extends typescript-eslint
  3. Why did global config take precedence? ESLint flat config (eslint.config.*) takes precedence over legacy format (.eslintrc.*)
  4. Why does global config exist? Developer has global Node.js tooling with TypeScript ESLint
  5. Why doesn't project isolate from global? No flat config in project - only legacy .eslintrc.json which was also broken (JSON with comments)

Changes

  • Created eslint.config.js (flat config format) - takes precedence over global configs
  • Deleted .eslintrc.json (contained invalid JSON with // comments)
  • Updated package.json scripts (removed deprecated --ext flag)
  • Simplified .husky/pre-commit (config validation + lint-staged only)
  • Added @eslint/js and globals dependencies
  • Removed deprecated husky v9 shim lines

Testing Added

  • 17 new tests in tests/config/eslint-config.test.js:
    • Config file validation (flat config exists, no legacy)
    • ESLint execution (no TypeScript errors)
    • Package.json configuration
    • Pre-commit hook validation
    • Rule configuration (service worker, content scripts, tests)

CI Updates

  • Added ESLint config validation step
  • Added legacy config detection (fails if found)
  • Added config loading validation
  • Added config test execution

Documentation

Test Plan

  • npm run lint runs without TypeScript plugin errors
  • npm run test -- tests/config/eslint-config.test.js passes (17 tests)
  • Pre-commit hook passes without --no-verify
  • No @typescript-eslint errors in output
  • Husky deprecation warning resolved

UNIX Rules Applied

  • Rule fix: remove console wrapping anti-pattern #5: Fail Early, Fail Loud - global config silently took precedence
  • Rule #7: Prefer Simple Over Clever - flat config is simpler than cascading legacy
  • Rule #8: Build for Debuggability - error message was misleading (TypeScript error in JS project)
  • Rule #12: Respect the Environment - project must isolate from developer's global environment

🤖 Generated with Claude Code

Root cause: Global ESLint config at ~/eslint.config.mjs was taking
precedence over project's legacy .eslintrc.json, causing TypeScript
plugin errors in this JavaScript project.

Contributing factor: .eslintrc.json contained invalid JSON (// comments)

Changes:
- Created eslint.config.js (flat config format)
- Deleted .eslintrc.json (broken JSON with comments)
- Updated package.json scripts (removed deprecated --ext flag)
- Simplified pre-commit hook (config validation + lint-staged)
- Added @eslint/js and globals dependencies
- Added ESLint config validation tests
- Added CI workflow steps for config validation
- Created ADR-001 and RUNBOOK-eslint-config documentation
- Updated CANONICAL.md with issue tracking

See docs/adr/ADR-001-eslint-config-format.md for full rationale.

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Husky v10 will fail if shim lines present.
Removed per husky deprecation warning.

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updated @eslint/js to v9.39.2 with breaking changes:
- Minimum Node.js version now 18.18.0+ (was 12.22.0+)
- Added TypeScript type definitions (types/index.d.ts)
- Updated README with new flat config examples using defineConfig()
- Added meta object with name/version to package exports
- Added new rules: no-unassigned-vars, no-useless-assignment, preserve-caught-error
- Removed deprecated rules: line-comment-position, multiline-comment
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

import { join } from 'path';
import { execSync } from 'child_process';

const PROJECT_ROOT = join(import.meta.dirname, '../..');

Choose a reason for hiding this comment

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

P1 Badge Avoid import.meta.dirname in Node 18 test runtime

CI runs tests on Node 18 (.github/workflows/test.yml matrix includes 18.x), but import.meta.dirname is only available in newer Node releases; on Node 18 it is undefined, so join(import.meta.dirname, '../..') will throw before any assertions run. This will break the test:coverage job on Node 18 that picks up this new test. Use fileURLToPath(new URL('.', import.meta.url)) or similar to compute the directory, or raise the Node minimum if you intend to require 20+.

Useful? React with 👍 / 👎.

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.

2 participants