Skip to content

[tech-debt 2/5 medium-low risk] Fix leaked listeners and unhandled rejections - #14775

Merged
dylanjeffers merged 1 commit into
mainfrom
fix/tech-debt-2-leaks
Nov 12, 2026
Merged

dylanjeffers merged 1 commit into
mainfrom
fix/tech-debt-2-leaks

Conversation

@dylanjeffers

Copy link
Copy Markdown
Contributor

Second of five PRs that re-ship #14665 on current main, ordered from lowest to highest risk. Branched off main, independent of the others.

What's in it

Listener leaks and unhandled rejections. None of these are meant to change what the user sees.

harmony

  • Modal: the touchmove listener on the modal root was only removed on the next close render, so it leaked on unmount and stacked when onTouchMove or modalRoot changed. It's now removed in the effect cleanup.
  • Slider: document mouse/touch listeners leaked when it unmounted mid-drag. They're removed on unmount.

mobile

  • GoogleCast: the PlaybackActiveTrackChanged listener was never removed, so it piled up on each reconnect or track change and kept calling stale cast clients. Moved from useAsync to useEffect with a cleanup, and load errors are logged.
  • Airplay: cleanup called .stop(), which doesn't exist on the subscription, so the listener was never removed. It now calls .remove().
  • useRevertOnCancel: navigation.addListener returns an unsubscribe function, and passing that to removeListener did nothing. The effect now returns the unsubscribe.
  • audius-sdk: each concurrent init waiter added a permanent listener. It uses once().
  • ChatScreen: the AppState listener effect had no deps array, so it was removed and re-added on every render.

web

  • StripeOnRampModal: the onramp_session_updated listener is removed on cleanup (it stacked on each reopen with the same session), and a failed session init is caught and logged.
  • foregroundPollingDaemon: the visibilitychange listener is removed when the saga is cancelled.

common

  • uuid(): the variant digit came from 'a' & 0x3, which is NaN & 3, so hex letters always became 8. It now parses the digit first. Output format is unchanged. This was in tier 3 in the plan; I moved it here because it only changes the random distribution of one character.

Why this tier

Each fix is local to one component or helper and only touches cleanup paths or error handling.

Verified

  • tsc: harmony clean. common, mobile and web show the same errors as origin/main in this checkout, all in untouched files.
  • eslint on changed files: clean.
  • common vitest and mobile jest: same results as tier 1 (common's pre-existing store/upload/selectors.test.ts failure only).

Not verified

Not run on a device or in a browser. Chromecast, AirPlay and the Stripe onramp in particular were not exercised.

🤖 Generated with Claude Code

Contained fixes from #14665 with no intended UX change:
- harmony Modal: remove the touchmove listener on close/unmount
- harmony Slider: detach document drag listeners on unmount
- mobile GoogleCast: remove the PlaybackActiveTrackChanged listener
- mobile Airplay: call remove() on the subscription (stop() doesn't exist)
- mobile useRevertOnCancel: return the unsubscribe from addListener
- mobile audius-sdk: init waiters use once()
- mobile ChatScreen: add deps so the AppState listener isn't re-added each render
- web StripeOnRampModal: remove onramp_session_updated listener; catch init failure
- web foregroundPollingDaemon: remove visibilitychange listener on cancel
- common uuid(): variant digit came from a string bitwise op

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dylanjeffers
dylanjeffers merged commit 33989e2 into main Nov 12, 2026
27 checks passed
@dylanjeffers
dylanjeffers deleted the fix/tech-debt-2-leaks branch November 12, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant