chore: make eslint run again and format the codebase - #215
Closed
edospadoni wants to merge 2 commits into
Closed
edospadoni wants to merge 2 commits into
edospadoni wants to merge 2 commits into
Conversation
eslint aborted on every run: .eslintrc.cjs extended react-app and react-app/jest from eslint-config-react-app, which is neither installed nor listed in package.json. - drop the react-app configs, CRA era and unmaintained, not applicable to an electron app - install eslint-plugin-react-hooks, already declared in plugins but missing, and extend plugin:react-hooks/recommended to keep the hook rules react-app used to provide - 'react/display-name': false is not a valid severity, use 'off' - drop import/no-anonymous-default-export and the import resolver setting, both referring to the uninstalled eslint-plugin-import - set settings.react.version to detect and dedupe the repeated extends entries This only makes eslint run again. It does not address the 4149 problems it now reports on the existing codebase, and note that the lint script runs with --fix, so it would reformat the whole repository.
Runs the autofixer over the whole repository and settles the rules that did not fit this codebase, taking eslint from 4149 problems (682 errors) to 0 errors. Rule changes, all of them cases where the rule was wrong for this project rather than the code: - spaced-comment keeps the '/' marker, otherwise the autofix rewrites TypeScript triple-slash directives into // / <reference ... /> and the asset module declarations silently stop being picked up - camelcase only checks declarations: the NethVoice API and the design tokens are snake_case on the wire - quotes allows double quotes to avoid escaping, for strings that embed single quotes - react/prop-types off, TypeScript prop types already cover it - react-hooks rules do not apply to main, preload and shared: a use* factory there is not a React hook - ban-ts-comment downgraded to a warning Hand fixes for what the autofixer could not resolve: duplicate imports, empty blocks, an empty props interface, Function as a type, and a Promise wrapper around an already async function whose executor was async, which swallowed rejections. 317 warnings are left (mostly no-explicit-any and unused vars) and are untouched on purpose: they need real code changes, not formatting.
This was referenced Sep 18, 2026
edospadoni
added a commit
that referenced
this pull request
Sep 18, 2026
ESLint was entirely broken on main: .eslintrc.cjs extended 'react-app' and 'react-app/jest' while eslint-config-react-app was never installed, so 'npm run lint' failed before linting a single file. Rather than repair the eslintrc setup on eslint 8, this migrates to eslint 9 flat config, which also unblocks the two Renovate majors that were stuck on the 'eslint >= 9' peer requirement: - eslint ^8.56.0 -> ^9.39.5 - @electron-toolkit/eslint-config-ts ^1.0.1 -> ^3.1.0 (#206) - @electron-toolkit/eslint-config-prettier ^2.0.0 -> ^3.0.0 (#205) - eslint-plugin-react ^7.33.2 -> ^7.37.5 (flat config support) - eslint-plugin-react-hooks ^5.2.0 (was referenced, never installed) .eslintrc.cjs and .eslintignore are replaced by eslint.config.mjs. Config decisions: - React and react-hooks rules are scoped to src/renderer/**. The main process has plain functions named use* (useNethVoiceAPI, useLogin) that rules-of-hooks reported as hook violations in AccountController. - camelcase is off. Every one of its 41 reports was a snake_case field fixed by the backend API contract (speeddial_num, shared_groups, dst_cnam) or a theme token (extra_large, full_w). - react/prop-types is off, since TypeScript already checks props. - spaced-comment carries markers: ['/'] so --fix cannot mangle TypeScript triple-slash directives into '// / <reference ... />', which silently broke the vite/client types behind every .svg import. Beyond formatting, these lint errors needed real fixes: - useNethVoiceAPI.ts: drop the redundant async Promise executor around logout; the body already resolved unconditionally in finally, so behaviour is unchanged and no rejection path is lost. - logger.ts: replace the unsafe Function type with an explicit signature. - main.ts, Modal/index.tsx: merge duplicate imports. - AboutModule.tsx: drop the empty destructuring pattern. - usePresenceService.ts: drop an empty finally block. - ipcEvents.ts: document the intentionally silent catch in the drag path. - store.ts: move a // @ts-ignore next to the expression it suppresses. Reformatting split the statement it used to cover onto two lines, which silently un-suppressed a zustand typing error. 'npm run lint' now reports 0 errors and 339 warnings. typecheck and build are green, and the code merged from #192 and #213 is intact. Supersedes #215.
Member
Author
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.
Why
npm run lintdid not run at all: the config extendedreact-appandreact-app/jest, which are neither installed nor inpackage.json, so eslint aborted before linting a single file.With that unblocked it reported 4149 problems (682 errors). This PR takes it to 0 errors.
Two commits
1.
chore:make eslint run again — config only, no code touched.react-appconfigs (CRA-era, unmaintained, not applicable to an Electron app)eslint-plugin-react-hooks, already listed inpluginsbut missing from the deps'react/display-name': falseis not a valid severity →'off'import/no-anonymous-default-exportand the import resolver setting, both referring to the uninstalledeslint-plugin-import2.
style:format the codebase — the autofixer over the whole repo, plus the rules that did not fit this project.Rule decisions
Each of these is a case where the rule was wrong for this codebase, not the code:
spaced-comment/marker — see the warning belowcamelcaseproperties: 'never': the NethVoice API and the design tokens are snake_case on the wirequotesavoidEscape: true, for strings that embed single quotesreact/prop-typesreact-hooks/*main,preload,shared: ause*factory there is not a React hook, so the rule only produced false positivesban-ts-commentBoth were caught by building, not by the linter. Worth knowing before running the autofixer again:
1. It destroyed the TypeScript triple-slash directives.
spaced-commentrewrote/// <reference types="vite/client" /> → // / <reference types="vite/client" />in both
env.d.tsfiles, which silently removed the module declarations for.svgand.cssimports. Fixed by configuring the rule withmarkers: ['/'].2. It moved a
@ts-ignoreoff its target. Prettier split a call across lines, so the directive ended up one line above the expression that actually errors. Fixed by putting it back on the right line, with a comment explaining why it has to stay there.Hand fixes
What the autofixer could not resolve: duplicate imports, empty blocks, an empty props interface,
Functionas a type, and — the only non-mechanical one — anew Promise()wrapper around an already-asyncfunction whose executor wasasync, which swallowed rejections. The rewrite is behaviour-equivalent.What is left
316 warnings, untouched on purpose — mostly
no-explicit-any(175) and unused vars (92). They need real code changes, not formatting, and belong in their own PR.Note
npm run lintruns with--fix, so it rewrites files in place.How to test
npm run lint→ 0 errors.npm run typecheckandnpm run buildpass, andelectron-builder --dirstill packages.Since this touches 137 files, CI going green is necessary but not sufficient: the app is worth a manual smoke test (login, calls, PhoneIsland, settings) before merging.