Fix correctness and suspicious lint debt (issue #65)#67
Merged
Conversation
Address every remaining `lint/correctness/*` and `lint/suspicious/*` violation surfaced by the biome 2.x upgrade, except `useExhaustiveDependencies` (66 cases that need per-call review). - noNonNullAssertedOptionalChain: guard against null `originalIndexPath` in `scoreCandidate` instead of trusting `?.[...]!` (anchoring.ts) - noGlobalIsFinite: switch to `Number.isFinite` (grid-tables.ts) - noImplicitAnyLet: type `browser: Browser` (mermaid-chromium.ts) - noUnusedImports: drop `Identity` and `TocNode` - noUnusedFunctionParameters: drop unused `docSource` and `docFormat` props from `InlineCommentsLayer` and `InlineCommentsList` - noControlCharactersInRegex: keep the control-char strip in the commit-message sanitizer with a targeted biome-ignore - noDuplicateProperties: keep `100vh` → `100dvh` progressive-enhancement pairs with targeted biome-ignores (app.css) - noVoidTypeReturn: split `return walkChildren(...)` calls into a call plus a bare `return` in `walkInline` (export-docx.ts) - noAssignInExpressions: rewrite `regex.exec()` while-loops and `??=` ensure-and-get patterns into separate statements Typecheck and `bun test:ci` (393 pass, 4 skip) green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fix the 18 `lint/a11y/*` errors that biome 2.x flagged but issue #65 didn't list: - `noSvgWithoutTitle` — mark the brand-icon `<svg>` `aria-hidden` (parent already had it). - `useButtonType` — add `type="button"` to the toast close + action buttons so they don't submit any enclosing form. - `noAutofocus` — replace the `autoFocus` attribute in the comment-edit textarea with a ref-based focus driven by the `editing` state. - `useAriaPropsSupportedByRole` — drop unused `aria-label` from containers without a role (mention/shortcode menus, diff overview). - `noStaticElementInteractions` / `useKeyWithClickEvents` — - lightbox stage: real toggle, so add `role="button"`, `tabIndex`, keyboard handler, descriptive `aria-label`. - lightbox controls / caption / drop-zone wrapper: stop-propagation or drag-only, no real click interaction; targeted suppressions. - `useSemanticElements` — - `MarkdownToolbar` is genuinely a toolbar: add `role="toolbar"`. - `ResizeHandle` (`role="separator"`) and the file-drop button-styled div can't be replaced with `<hr>`/`<button>` without losing interactivity; targeted suppressions. - Reaction groups (`role="group"`) — `<fieldset>` is form-only; targeted suppression. Also drop the dead `noDuplicateProperties` suppression on the lightbox `max-height` pair (biome doesn't flag it; it was added speculatively). Reduces biome errors from 85 → 67 (the remaining 67 are all `useExhaustiveDependencies` and need per-`useEffect` review). Typecheck clean; `bun run test:ci` 393 pass / 4 skip / 0 fail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
paulwellnerbou
added a commit
that referenced
this pull request
May 8, 2026
Closes #68. Resolves the 66 useExhaustiveDependencies error-class cases the issue called out (#67 already cleared the other rule classes), and adds typecheck + lint as CI gates so they can't pile up again. Approach in DocumentLayout (the bulk of the cases): - `resolveIdentity` is now a `useCallback([displayName, effectiveDisplayName])` so downstream callbacks list it as a single dep instead of mirroring the two display-name fields. - `refreshDoc` / `refreshThreads` hoisted above the useEffects / useCallbacks that need them as deps (the dep array is evaluated at render time — TDZ otherwise). - 12 callback dep arrays updated to reference `resolveIdentity` and `refreshThreads`/`refreshDoc` directly. The corresponding eslint-disables are removed. `biome-ignore` (with reason) is reserved for genuine intentional re-trigger effects: mount-time deep link, doc-swap resets, layout remeasure, headings scroll, content-swap re-attach, events subscription. Each ignore explains *why* the dep is there or omitted. Other files: same pattern — callback deps updated where straight- forward, biome-ignore for re-trigger / mount-once / static-fn cases. `InlineCommentsLayer` hoists `resolveAnchorElement` to module scope so it's stable as a dep. CI now runs `bun run typecheck` and `bun run lint` before tests, so any future PR introducing a biome error fails the build. Lint warnings stay non-blocking. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
5 tasks
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
Tackles the highest-priority warning classes from #65: every remaining
lint/correctness/*andlint/suspicious/*violation surfaced by the biome 2.x upgrade, exceptuseExhaustiveDependencies(66 cases — sampled them and the auto-fix wants to remove deps likeversionthat are intentional re-fetch triggers, so they need per-useEffectreview and a follow-up PR).Per-rule notes:
originalIndexPath?.[…]!inscoreCandidatewith an explicit guard so a missingoriginalIndexPathno longer producesNaNscores.isFinite→Number.isFinite(no coercion).browserlocal inmermaid-chromium.ts.IdentityandTocNode.docSourceanddocFormatwere destructured but unused in bothInlineCommentsLayerandInlineCommentsList. Dropped the props from the interfaces, destructures, and call sites inDocumentLayout.tsx.biome-ignore.100vh→100dvhpairs are intentional progressive enhancement for older browsers. Targetedbiome-ignoreper pair.walkInlinewas usingreturn walkChildren(...)for control flow; split into call + barereturn.for/while (m = re.exec(x))loops intolet m = re.exec(x); while (m !== null) { …; m = re.exec(x); }, and??=ensure-and-get patterns into separate statements.After this PR the only remaining lint classes are style/complexity (
noNonNullAssertion,noImportantStyles,useOptionalChain,useTemplate,noDescendingSpecificity,useLiteralKeys) plus theuseExhaustiveDependenciesfollow-up.Test plan
bun run typecheck— cleanbun run test:ci— 393 pass, 4 skip, 0 failbunx biome check .— 0 remainingcorrectness/*orsuspicious/*violations (other thanuseExhaustiveDependencies)🤖 Generated with Claude Code