Skip to content

docs: document ESLint-to-Biome rule coverage (Closes #60)#61

Merged
podclaude[bot] merged 2 commits intomainfrom
feature/issue-60-eslint-biome-coverage-doc
Feb 18, 2026
Merged

docs: document ESLint-to-Biome rule coverage (Closes #60)#61
podclaude[bot] merged 2 commits intomainfrom
feature/issue-60-eslint-biome-coverage-doc

Conversation

@podclaude
Copy link
Copy Markdown
Contributor

@podclaude podclaude Bot commented Feb 18, 2026

Summary

  • Audited all ESLint rules from the old config against Biome equivalents
  • Documented the coverage comparison in CLAUDE.md (Tech Stack section)
  • Removed legacy eslint-disable comment in src/services/storage/index.ts
  • All major rule groups (JS recommended, React hooks, React refresh, TypeScript) are covered
  • 6 minor TypeScript-ESLint rules have no Biome equivalent (none are load-bearing for this codebase)

Test plan

  • npm run lint passes (no new warnings)
  • npm test passes (345 tests)
  • Legacy ESLint comment removed and replaced with proper destructuring rename

🤖 Generated with Claude Code

- Remove legacy eslint-disable comment in storage/index.ts
- Document rule coverage comparison in CLAUDE.md (Tech Stack section)
- All major ESLint rule groups covered by Biome equivalents
- 6 minor TypeScript-ESLint rules have no Biome equivalent (low-impact)

Closes #60
@podclaude podclaude Bot added the needs-claude-review PR needs review by Claude Code label Feb 18, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Feb 18, 2026

Review: docs: document ESLint-to-Biome rule coverage (Closes #60)

Overall this is a clean, well-scoped PR. Two changes, both appropriate: documentation added to CLAUDE.md and a legacy lint-suppression comment removed from source code. Here is my detailed feedback.


CLAUDE.md — Biome Rule Coverage table

Positive:

  • The coverage table is genuinely useful for future contributors who might wonder whether switching away from ESLint introduced regressions.
  • Calling out the 6 uncovered TypeScript-ESLint rules explicitly (with a rationale for why each is low-impact) is the right level of honesty.
  • The three-rule "covered by Biome recommended" sub-section gives a concrete mapping rather than just saying "~90% covered."

Minor observations (no blocking issues):

  1. useExhaustiveDependencies is listed as warn — is that still accurate?
    The table says Explicitly enabled (warn) for react-hooks/exhaustive-deps. It is worth double-checking that biome.json currently uses "level": "warn" for that rule and not "error". If it has been changed since the initial Biome migration (PR feat: migrate from ESLint to Biome (Closes #58) #59), the table will be misleading.

  2. no-empty-object-type, no-unsafe-function-type, no-wrapper-object-types
    These three have had Biome equivalents since Biome v1.7+ (noEmptyTypeParameters, noExplicitAny-adjacent rules). It may be worth a quick re-check against the current Biome version in use to see if any of these gaps have since been closed, so the table stays accurate long-term.

  3. Table alignment
    The Markdown table header separators (|------|) have different widths across columns. This renders fine in GitHub's Markdown engine, but it is inconsistent with the other tables in CLAUDE.md which use aligned separators. Low priority, cosmetic only.


src/services/storage/index.ts — Remove eslint-disable comment

// Before
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { id, ...rest } = settings;

// After
const { id: _, ...rest } = settings;

Positive:

  • Correct approach. Using _ (or id: _) as the conventional "intentionally unused" name is idiomatic TypeScript/JavaScript and is understood by Biome's noUnusedVariables rule without needing any suppression comment.
  • The rename does not change runtime behavior; rest still contains all fields except id, which is what the getSettings return type (AppSettings) expects.

One thing to verify:

  • Biome's noUnusedVariables rule does suppress warnings for variables whose name starts with _. Confirm npm run lint produces no warning for _ here (the PR description says it does pass, which is expected — just noting it for completeness).

Summary

Area Status
Documentation accuracy ✅ Good, with minor caveats on warn level and Biome version gaps
Code change correctness ✅ Correct and idiomatic
Tests ✅ 345 tests pass, lint passes
Scope creep ✅ None — tightly scoped to the issue
Blocking issues ❌ None

Verdict: Approved with minor suggestions. The only actionable item worth following up before merge is confirming the useExhaustiveDependencies severity level in biome.json matches what the table documents (warn). Everything else is cosmetic or informational.

@github-actions github-actions Bot removed the needs-claude-review PR needs review by Claude Code label Feb 18, 2026
noBannedTypes (recommended) covers no-empty-object-type, no-unsafe-function-type,
and no-wrapper-object-types. noEmptyInterface also covers part of no-empty-object-type.
This raises coverage to ~95%. Also align table columns for consistency.
@podclaude podclaude Bot added the needs-claude-review PR needs review by Claude Code label Feb 18, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Feb 18, 2026

Review

This PR makes two tightly-scoped changes: adding a Biome-vs-ESLint rule coverage table to CLAUDE.md, and removing a legacy eslint-disable comment from the storage service. Both are correct and well-executed.


Verification of key claims

useExhaustiveDependencies at warn — Confirmed accurate. biome.json line 33 has "useExhaustiveDependencies": "warn". The table is correct.

noBannedTypes coverage for no-empty-object-type, no-unsafe-function-type, no-wrapper-object-types — Accurate. biome.json has "noBannedTypes": "error" in the complexity rules block (line 29), and recommended: true is set, so these are actively enforced.

id: _ rename in src/services/storage/index.ts — Correct and idiomatic. Biome's noUnusedVariables suppresses warnings for variables starting with _, so this eliminates the need for the eslint-disable comment without any runtime behavior change.


Minor observations

  1. ~95% TypeScript-ESLint coverage claim — Reasonable. The 3 uncovered rules listed (no-duplicate-enum-values, no-non-null-asserted-optional-chain, triple-slash-reference) are all genuinely low-impact for this codebase. The ~95% figure is a fair characterization.

  2. noEmptyInterface listed as Biome equivalent for no-empty-object-type — Worth noting that noEmptyInterface is distinct from noBannedTypes. noEmptyInterface disallows interface Foo {} while noBannedTypes handles the {} type annotation case. The table captures both correctly.

  3. no-unsafe-function-type and no-wrapper-object-types coverage — These are both handled by noBannedTypes in Biome, which is correctly noted. No issues here.


Summary

Area Status
Documentation accuracy ✅ Verified against actual biome.json
Code change correctness ✅ Idiomatic and correct
Scope ✅ Tightly scoped, no unrelated changes
Blocking issues None

Approved. The documentation is accurate, the code change is correct, and the PR is well-scoped. Ready to merge.

@github-actions github-actions Bot removed the needs-claude-review PR needs review by Claude Code label Feb 18, 2026
@podclaude podclaude Bot merged commit 0e01879 into main Feb 18, 2026
1 check passed
@podclaude podclaude Bot deleted the feature/issue-60-eslint-biome-coverage-doc branch February 18, 2026 20:14
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