chore: declare eslint + typescript-eslint as root devDependencies#7
Merged
Conversation
Previously the shared eslint.config.base.mjs (used by core/db/cli/sdk) imported `typescript-eslint` only via npm-workspace hoisting — it was a transitive dependency of the web package's eslint-config-next. That worked but was fragile: removing web's lint toolchain would break `npm run lint` with "Cannot find module typescript-eslint". Declaring them at the root removes that implicit coupling. - Add `eslint`/`typescript-eslint` to root devDependencies and update the base-config comment. - Pin `eslint-plugin-react-hooks` to 7.0.1 via overrides: eslint-config-next requests ^7.0.0, and a fresh lock resolves 7.1.x, whose stricter React Compiler rules flag ~12 pre-existing components. Keeping the lint behavior identical here keeps this change focused; upgrading the plugin and fixing those is a separate follow-up. - Regenerated package-lock.json with `npm install --include=optional --force` per the project's macOS->Linux rule; verified the 29 Linux-only optional deps (@rollup/rollup-linux-*, @emnapi/*) survived and `npm ci` succeeds. Build clean; lint 0 errors across all packages; 1166 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
iaj6
added a commit
that referenced
this pull request
Jun 5, 2026
…d smells (#8) Bumps the override from 7.0.1 (pinned in PR #7 to dodge the new rules) to 7.1.1, which activates the React Compiler lint rules (purity, set-state-in-effect, refs, immutability), and resolves the 12 violations they surface. Kept as an explicit pin: this plugin adds CI-gating rules in minor releases (eslint-config-next only requests ^7.0.0), so deliberate bumps avoid surprise breakage. Real fixes (behavior-preserving): - useEventSource: write onEvent into the ref from an effect, not during render (refs rule). Only async SSE handlers read it, after commit. - useSessions: hoist the markUpdated useCallback above its first use (immutability rule) — declaration-order only. - ApiTokens/Users settings sections: move the mount fetch into a cancellable async IIFE so setState lands after the await (set-state-in-effect), keeping the try/finally so a rejected fetch can't strand the loading spinner. - EventLog: read the clock from an interval-refreshed `now` state instead of during render (purity). Strictly less churny than the prior per-render Date.now(); the time-window cutoff lags <=30s. Scoped, justified eslint-disables for genuine false positives: terminating derived-state resets (setPage(0), setSelectedIndex(0)) and intentional point-in-time pause snapshots in EventLog. Verification: web lint 0/0, full build green, 1166 tests pass, clean npm ci. An adversarial 4-lens review caught the stranded-spinner error path, now fixed. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the deferred follow-up from #5: the shared
eslint.config.base.mjs(used bycore/db/cli/sdk) importedtypescript-eslintonly through npm-workspace hoisting — it was a transitive dep of the web package'seslint-config-next. That worked, but if web's lint toolchain ever changed,npm run lintwould break withCannot find module typescript-eslint. This declares the toolchain explicitly.Changes
devDependencies:eslint@^9+typescript-eslint@^8— removes the implicit coupling to web. Updated the base-config comment accordingly.overrides:eslint-plugin-react-hookspinned to7.0.1.eslint-config-nextrequests^7.0.0, and a fresh lock resolves7.1.x, whose stricter React-Compiler rules flag ~12 pre-existing components (EventLog, ApiTokensSection, AuditSection, CommandPalette, …). Pinning keeps lint behavior identical so this PR stays focused. (Upgrading the plugin + fixing those 12 is a separate, PR5-sized follow-up.)package-lock.jsonregenerated vianpm install --include=optional --force, per the repo's macOS→Linux optional-deps rule.Verification (the lock-regen gates)
@rollup/rollup-linux-*,@emnapi/*, …) — the exact thing the rule guards against.npm cisucceeds against the fresh lock (noMissing … from lock file).npm run lint= 0 errors across all packages (react-hooks pin held).The large
package-lock.jsondiff is the expected result of a full regeneration; the meaningful change is 3 lines inpackage.json+ the config comment.🤖 Generated with Claude Code