Skip to content

chore: add ESLint + Prettier frontend linting#3

Merged
juacker merged 1 commit into
mainfrom
qa/add-frontend-linting
May 21, 2026
Merged

chore: add ESLint + Prettier frontend linting#3
juacker merged 1 commit into
mainfrom
qa/add-frontend-linting

Conversation

@juacker
Copy link
Copy Markdown
Owner

@juacker juacker commented May 21, 2026

Summary

Add ESLint flat config and Prettier to the frontend with a passing baseline.

What's included

  • ESLint (): flat config with , ,
  • Prettier (): 2-space, single-quote, 100 print-width
  • npm scripts: , , ,
  • CI step: runs before frontend build
  • Bug fix: Defines missing in (was referenced but never defined — a latent no-undef bug)

Disabled rules (for incremental adoption)

React 19 strict hooks rules are disabled because the existing codebase predates them. Re-enable incrementally:

  • react-hooks/set-state-in-effect
  • react-hooks/exhaustive-deps
  • react-hooks/preserve-manual-memoization
  • react-hooks/refs
  • react-hooks/immutability
  • react-hooks/use-memo
  • react-hooks/purity

Validation

  • npm run lint0 errors, 45 warnings (all no-unused-vars)
  • cargo test --lib268 passed, 0 failed
  • npm run build → passes

Add ESLint flat config (eslint.config.js) and Prettier (.prettierrc)
to the frontend with the following setup:

- eslint + eslint-plugin-react + eslint-plugin-react-hooks
- prettier for formatting
- npm scripts: lint, lint:fix, format, format:check
- CI step: `npm run lint` before frontend build

React 19 strict hooks rules are disabled for now because the existing
codebase predates them. Re-enable incrementally in follow-up PRs:
  - react-hooks/set-state-in-effect
  - react-hooks/exhaustive-deps
  - react-hooks/preserve-manual-memoization
  - react-hooks/refs
  - react-hooks/immutability
  - react-hooks/use-memo
  - react-hooks/purity

Also fixes a latent `no-undef` bug in CommandContext.jsx where
`isNavigationCommand` was referenced but never defined.

Validation:
- `npm run lint` → 0 errors, 45 warnings (all no-unused-vars)
- `cargo test --lib` → 268 passed, 0 failed
@juacker juacker marked this pull request as ready for review May 21, 2026 09:16
@juacker
Copy link
Copy Markdown
Owner Author

juacker commented May 21, 2026

Review

Overall: good, low-risk infrastructure PR. Sensible scoping (flat ESLint + Prettier, lint-only CI gate), and the "disable React 19 strict hooks rules with an explicit re-enable list" stance is the right call for a codebase that predates them.

Concerns

1. isNavigationCommand breaks the file's quote conventionsrc/contexts/CommandContext.jsx:270-273

const navigationTypes = ["navigate", "open", "switch", "back", "forward"];
return navigationTypes.includes(cmd?.type) || cmd?.type?.endsWith("_navigation");

Double quotes in a file that uses single quotes everywhere, and .prettierrc is singleQuote: true. ESLint won't catch it: eslint-config-prettier only disables conflicting rules, it doesn't enforce Prettier formatting. Either add eslint-plugin-prettier or wire format:check into CI (see #5).

2. isNavigationCommand should be hoisted to module scope
It's defined inside CommandProvider but captures no state — a new reference is created every render, and it's missing from getVisualizationHistory's deps [commandHistory]. Exhaustive-deps is disabled so this passes lint, but the clean fix is to move it above CommandProvider as a pure helper.

3. "Latent bug" framing is slightly generous
getVisualizationHistory is exported through the context but has zero callers in the tree. So the ReferenceError was only ever theoretical. Fine to include the fix, but nothing was broken in production.

4. Stray blank lines from removed disable-comments
5 occurrences across ContextChart.jsx, Dashboard.jsx, useCommandRegistration.js:

    setIsInitialized(true);

  }, []);

The removed // eslint-disable-next-line should leave nothing, not a blank line. Cosmetic — Prettier would fix it if enforced.

5. format:check script exists but isn't enforced in CI
You added the script but CI only runs lint. Formatting will silently drift. Either gate on it or drop the script.

6. eslint-plugin-react-hooks is installed with ~all rules off
Fine — keeps re-enabling to a one-line change per rule. Just noting.

Suggested follow-ups (separate PRs)

  • Hoist isNavigationCommand, fix quotes, drop blank lines → quick cleanup PR
  • Enforce Prettier in CI (format:check) or remove the script
  • Re-enable react-hooks/exhaustive-deps first; it's the highest-signal of the disabled rules

Net: approve once CI passes. The nits above aren't blockers but the quote-style inconsistency + unenforced `format:check` undermine the "we have a linter now" guarantee — worth fixing soon.

Review by Claude Code.

@juacker
Copy link
Copy Markdown
Owner Author

juacker commented May 21, 2026

Merging anyway — none of the findings are blockers; tracking the cleanup in a follow-up PR.

@juacker juacker merged commit 348e111 into main May 21, 2026
1 check 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