Frontend: fix all ESLint findings and add CI enforcement - #1375
Merged
gusthoff merged 7 commits intoJul 24, 2026
Merged
Conversation
eslint.config.js registered eslint-plugin-jsdoc's flat/recommended config with no `files` restriction, unlike the sibling TypeScript config block that is scoped to src/**/*.ts and tests/**/*.ts. Because ESLint's flat config does not consult .gitignore, this meant that any file ESLint happened to find under tests/ was linted with the jsdoc rules, including frontend/tests/html/ -- a Sphinx documentation build output directory (gitignored, regenerated by the test suite) that contains furo theme JavaScript (search stemmers, sphinx_highlight.js, etc.) copied there by Sphinx itself, not written by this project. This accounted for 190 of 205 eslint warnings (93%) whenever that directory happened to exist on disk from a prior local build/test run. Fixed by adding a global `ignores: ['tests/html/**']` entry and scoping the jsdoc config block to the same files glob as the TypeScript block. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@typescript-eslint/no-unused-expressions doesn't know that a chai assertion chain like expect(x).to.be.true is a meaningful statement -- it just sees a property-access expression whose result is discarded, and flags it as dead code. This produced 26 false-positive errors across the test spec files, none of which were real bugs. Added eslint-plugin-chai-friendly and swapped in its chai-aware no-unused-expressions rule, scoped to tests/**/*.ts, in place of the plain rule. src/**/*.ts keeps the plain rule since it has no chai assertions to misread. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Every *.spec.ts file called chai's use() to register a plugin (chai-dom, chai-as-promised) but captured the return value in an unused `chai` binding; only the side effect of registering the plugin matters, so the binding is dropped. - download.spec.ts imported chai-as-promised but never registered it nor used any of its assertions (no .eventually, .rejectedWith, etc. anywhere in the file) -- removed as genuinely dead. - widget.spec.ts had chai-dom's registration commented out (`// const chai = use(chaiDom);`) even though this file's own tests use the chai-dom `.to.have.class()` matcher; it only worked because Mocha loads spec files alphabetically and every other spec file registers chai-dom first. Restored the registration so this file doesn't silently depend on load order, and dropped its unused `Client` import from mock-socket (never referenced). - Removed two copies of `const identifier = 123;` and one `let receivedMessages: Array<string> = [];` in widget.spec.ts that were declared but never read anywhere in their scope (verified against their sibling blocks, which do use same-named variables for real). - Renamed unused `(event)` callback parameters to `(_event)` in the mock-socket message handlers across server.spec.ts and widget.spec.ts, and added `argsIgnorePattern: '^_'` to the no-unused-vars rule in eslint.config.js -- widget.spec.ts already had one `_event` occurrence that was still being flagged because this option was never set. - Dropped `@typescript-eslint/no-empty-function` from an eslint-disable comment in widget.spec.ts: the rule was never reporting a problem there in the first place. Verified with `pnpm run test`: 126 passing, no change in behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
23 lines across the test spec files exceeded the 80-column limit. Reflowed each with the codebase's existing continuation style (indent the wrapped remainder relative to the statement, or break chained assertions at the dot), and split one nested type assertion into an intermediate id variable + as-cast rather than wrapping across the "as" keyword, which the TypeScript parser used here does not accept split across lines. Verified with `pnpm run test`: 126 passing, no change in behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
download.spec.ts stubs JSZip.prototype.generateAsync to capture the list of files being zipped, and already suppresses no-explicit-any at the two other (JSZip.prototype as any) casts used to install and type that stub -- JSZip's public TypeScript types don't expose this internal method. The after() hook that restores the original function used the same cast but was missing the matching suppression comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- resource.ts: dropped the @export tag from two type declarations. @export is a Closure Compiler annotation, not a recognized JSDoc/TSDoc tag, and is redundant here anyway since TypeScript's own `export` keyword already marks these types as exported. - scrolltop.spec.ts, server.spec.ts, widget.spec.ts: removed blank comment lines between a function's description and its @param tags (jsdoc/tag-lines wants none), added missing @param descriptions on scrollTo's x/y parameters, and fixed two copies of the removeListeners doc comment whose `*` continuation lines were missing their leading space (jsdoc/check-alignment). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pnpm run eslint was never wired into any CI workflow, so lint regressions could accumulate indefinitely without anyone noticing (this is exactly how frontend/eslint.config.js ended up silently linting generated build output, and how 71 real errors piled up in the test spec files). Added as its own workflow rather than a step in typescript-tests.js.yml: that workflow's setup installs the full GNAT/gnatprove/gprbuild toolchain and builds the Sphinx HTML test fixtures before any test runs, none of which ESLint needs. A dedicated workflow mirrors the existing code-projects-type-check.yml (pyright) -- a fast, narrow static-analysis check that fails in seconds and is unambiguous about what broke. Verified locally: `pnpm install --frozen-lockfile && pnpm run eslint` exits 0 against the current lockfile and source tree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
pnpm run eslintwent from 276 problems (71 errors, 205 warnings) to 0,and is now enforced in CI so regressions get caught instead of
accumulating silently again.
eslint.config.jshad an unscoped jsdoc rule block, so ESLint waslinting
tests/html/— a gitignored, regenerated Sphinx build outputdirectory containing furo theme JavaScript, not project code. That
accounted for 93% of all warnings. Scoped it to match the sibling
TypeScript config block.
eslint-plugin-chai-friendlyso chai assertions(
expect(x).to.be.true) aren't misread as dead code byno-unused-expressions(26 false-positive errors).variables/imports (including one chai-dom registration that was
silently relying on Mocha's alphabetical spec-file load order),
unwrapped long lines, a missing
no-explicit-anysuppressioncomment, and jsdoc formatting issues.
.github/workflows/eslint.js.yml, a fast dedicated lint check(mirrors
code-projects-type-check.yml's shape) rather than foldingit into
typescript-tests.js.yml, which installs a full GNAT/Sphinxtoolchain unrelated to a lint check.
Test plan
pnpm run eslintexits 0pnpm run test— 126 passing, coverage unchanged (99.94%)pnpm install --frozen-lockfilesucceeds against the updated lockfileESLintCI check passes on this PR