Repository navigation
fix(lint): resolve hook warnings and annotate intentional sites - #235
Merged
Merged
Conversation
Lint on main reported 36 warnings. Fix or annotate 19 of them: - react-hooks/immutability and exhaustive-deps: the keyboard shortcut, autoplay and player-init effects in Player.tsx read their latest values through React.useEffectEvent instead of omitting deps. The keyboard and autoplay effects move below the handlers they call. EmbedPlayer autoplay and the SourceDrawer Escape handler get the same treatment. Effect triggers are unchanged. - react-hooks/purity: playFromSource runs only from effects and event handlers. The false positive surfaced once the immutability errors stopped masking it; suppress it with the reason. - no-img-element: the three banners are local animated GIFs, which next/image does not optimize. Annotated. - no-sync-scripts: audio-prewarm.js must register its gesture listener before the first click (#11); chiptune3.js is a module script and already deferred. Annotated. The 17 react-hooks/set-state-in-effect warnings are left for a follow-up. Pin all workflow runners to ubuntu-26.04 ahead of the ubuntu-latest migration. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197CGJGHNcvPazPDauYEqEG Signed-off-by: Ronny Trommer <ronny@no42.org>
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
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.
Clears the annotations from CI run 40908935651. ESLint on main reported 36 warnings, and GitHub only shows the first 10. This PR takes it to 17.
Fixed (no change to when effects fire):
react-hooks/immutability(10) andexhaustive-deps(5): the keyboard shortcut, autoplay and player-init effects inPlayer.tsxnow read their latest values throughReact.useEffectEventinstead of leaving deps out on purpose. The keyboard and autoplay effects move below the handlers they call.EmbedPlayerautoplay and theSourceDrawerEscape handler get the same change.Annotated with a reason:
no-img-element(3): the banners are local animated GIFs, andnext/imagedoes not optimize GIFs.no-sync-scripts(2):audio-prewarm.jsmust register its gesture listener before the first click (Playback clock stays at 00:00 on Firefox/Linux (AudioContext never resumed) #11).chiptune3.jsis a module script and already deferred.react-hooks/purity(1, error): a false positive inplayFromSourcethat showed up once the immutability errors stopped hiding it. It only runs from effects and event handlers.Not in this PR: the 17
react-hooks/set-state-in-effectwarnings. Each needs its own refactor.Runners: every
runs-onis pinned toubuntu-26.04ahead of theubuntu-latestmigration notice.Please check by hand: the keyboard shortcuts (space, n/b, a/z/x, m, 1, d, e, shift, /), autoplay on page load, embed autoplay, and Escape closing the drawer. E2E covers part of this.
Verified locally:
make verifyexits 0 with 0 errors and 17 warnings, and 267 unit tests pass.🤖 Generated with Claude Code
https://claude.ai/code/session_0197CGJGHNcvPazPDauYEqEG