Skip to content

fix(app): clean up online/offline listeners on Browse unmount - #7213

Merged
mergify[bot] merged 1 commit into
modrinth:mainfrom
waliser:fix/browse-online-offline-listener-leak
Aug 20, 2026
Merged

fix(app): clean up online/offline listeners on Browse unmount#7213
mergify[bot] merged 1 commit into
modrinth:mainfrom
waliser:fix/browse-online-offline-listener-leak

Conversation

@waliser

@waliser waliser commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Browse.vue registers window.addEventListener('online'/'offline', ...) but never removes the listeners.
  • The router only keeps LibraryPage alive via <KeepAlive include="LibraryPage"> (App.vue), so Browse.vue unmounts every time the user navigates away (e.g. into a project) and remounts on return.
  • Each remount leaks a new pair of closures on window, pinning the whole component setup scope from being garbage collected. Over a browsing session this accumulates continuously, contributing to reports of high/growing memory usage while browsing mods on Linux (related: Linux-specific performance, graphical, or crashing issues [MEGA-ISSUE] #3057).

Fix

Store the handlers in named refs, remove them in onBeforeUnmount, matching the cleanup pattern already used elsewhere in the app (e.g. ContextMenu.vue, QuickInstanceSwitcher.vue).

Test plan

  • Confirmed no other unmatched addEventListener/removeEventListener pairs were introduced
  • Would appreciate a maintainer/CI check on the frontend build, since I verified this locally by reading the router keep-alive config and lifecycle hooks, not with the full app-frontend dev toolchain

Browse.vue registers window 'online'/'offline' listeners but never
removes them. Since only LibraryPage is kept alive by the router,
Browse unmounts every time the user navigates away (e.g. into a
project) and remounts on return, leaking a new pair of closures (and
pinning the whole component scope) on window each time. Over a
browsing session this accumulates continuously.
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Note

This changelog has been baked. Any further edits will not be reflected.

Pull request changelog

App

Added

Changed

Deprecated

Removed

Fixed

  • Fixed memory leak on Discover page

Security

Website

Added

Changed

Deprecated

Removed

Fixed

Security

Hosting

Added

Changed

Deprecated

Removed

Fixed

Security

@Prospector

Copy link
Copy Markdown
Member

suspicious of the formatting of the PR description looking generated... but this fix does look correct so I'll merge

@Prospector

Copy link
Copy Markdown
Member

@Mergifyio queue

@mergify

mergify Bot commented Aug 20, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 7 minutes 56 seconds in the queue, including 6 minutes 49 seconds running CI.

Required conditions to merge

@mergify mergify Bot added the queued label Aug 20, 2026
@mergify
mergify Bot merged commit c94034f into modrinth:main Aug 20, 2026
9 checks passed
@mergify mergify Bot removed the queued label Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants