From 4195f4d768be2f6907b07e481d755b3157530e5a Mon Sep 17 00:00:00 2001 From: Adnan9-63 Date: Sun, 25 Oct 2026 01:44:27 +0530 Subject: [PATCH] fix(sidebar): dispose message listener on view reopen to prevent memory leaks Fixes #30 --- CHANGELOG.md | 1 + extension.js | 1 + src/sidebar.js | 30 ++++++++++-- tests/sidebar.test.js | 108 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 tests/sidebar.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index a5c5181..c4cebf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Format based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Query diagnostics only for visible documents and count split editors once per document, avoiding a workspace-wide scan on every diagnostics event. - Fix double-counting of session coding time in `StatsEngine.getSnapshot()` (Fixes #22). - Automatically recalculate and reset inactive streaks to 0 after 2 or more days without activity (Fixes #24). +- Dispose previous Sidebar webview message listener on view reopen to prevent memory leaks (Fixes #30). --- diff --git a/extension.js b/extension.js index 672de60..fd69476 100644 --- a/extension.js +++ b/extension.js @@ -469,6 +469,7 @@ function activate(context) { // ── Register everything ── context.subscriptions.push( statusBarItem, + sidebarProvider, cmdSayHi, cmdDashboard, cmdReset, diff --git a/src/sidebar.js b/src/sidebar.js index 114bedc..e13d768 100644 --- a/src/sidebar.js +++ b/src/sidebar.js @@ -4,6 +4,7 @@ const vscode = require('vscode'); const path = require('path'); const fs = require('fs'); const { PET_THEMES } = require('./moods'); +const { getPetSvg, getAccessoriesSvg } = require('./pets'); class SidebarProvider { /** @@ -19,6 +20,8 @@ class SidebarProvider { this.getCurrentMood = getCurrentMood; /** @type {vscode.WebviewView | undefined} */ this._view = undefined; + /** @type {vscode.Disposable | undefined} */ + this._messageListener = undefined; } resolveWebviewView(webviewView) { @@ -28,15 +31,37 @@ class SidebarProvider { localResourceRoots: [this.context.extensionUri] }; - webviewView.webview.onDidReceiveMessage(msg => { + // Dispose any previous message listener to prevent memory leaks on view reopen + if (this._messageListener) { + this._messageListener.dispose(); + this._messageListener = undefined; + } + + this._messageListener = webviewView.webview.onDidReceiveMessage(msg => { if (msg.command === 'openDashboard') { vscode.commands.executeCommand('codecritter.showDashboard'); } - }, undefined, this.context.subscriptions); + }); + + webviewView.onDidDispose(() => { + if (this._messageListener) { + this._messageListener.dispose(); + this._messageListener = undefined; + } + this._view = undefined; + }); this.update(); } + dispose() { + if (this._messageListener) { + this._messageListener.dispose(); + this._messageListener = undefined; + } + this._view = undefined; + } + update() { if (!this._view) return; @@ -49,7 +74,6 @@ class SidebarProvider { const currentMood = this.getCurrentMood(); const petName = this.getPetName(); - const { getPetSvg, getAccessoriesSvg } = require('./pets'); const petSvgRaw = getPetSvg(petSpecies).replace('__ACCESSORIES__', getAccessoriesSvg(stats.level)); const htmlPath = path.join(__dirname, '..', 'media', 'sidebar.html'); diff --git a/tests/sidebar.test.js b/tests/sidebar.test.js new file mode 100644 index 0000000..b2cdbe2 --- /dev/null +++ b/tests/sidebar.test.js @@ -0,0 +1,108 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const test = require('node:test'); +const vm = require('node:vm'); + +test('Issue #30: SidebarProvider disposes previous message listener when view is reopened', () => { + let executedCommand = null; + const vscode = { + commands: { + executeCommand: cmd => { executedCommand = cmd; } + }, + workspace: { + getConfiguration: () => ({ get: (k, def) => def }) + } + }; + + const dependencies = { + vscode, + path, + fs, + './moods': { + PET_THEMES: { default: { label: 'Default', body: '', bodyAlt: '', accent: '', antennae: '' } } + }, + './pets': { + getPetSvg: () => '__ACCESSORIES__', + getAccessoriesSvg: () => '' + } + }; + + const sandbox = { + module: { exports: {} }, + require: name => dependencies[name] || require(name), + __dirname: path.join(__dirname, '..', 'src') + }; + + const sidebarCode = fs.readFileSync(path.join(__dirname, '..', 'src', 'sidebar.js'), 'utf8'); + vm.runInNewContext(sidebarCode, sandbox); + const SidebarProvider = sandbox.module.exports.SidebarProvider; + + const mockContext = { + extensionUri: { toString: () => 'file:///ext' }, + subscriptions: [] + }; + + const mockStats = { + getSnapshot: () => ({ level: 1, xpProgress: 0 }) + }; + + const provider = new SidebarProvider( + mockContext, + mockStats, + () => 'Critter', + () => 'happy' + ); + + let firstListenerDisposed = false; + let secondListenerDisposed = false; + let activeMessageHandler = null; + + // First time resolving view + const mockWebviewView1 = { + webview: { + options: {}, + html: '', + onDidReceiveMessage: handler => { + activeMessageHandler = handler; + return { + dispose: () => { firstListenerDisposed = true; } + }; + } + }, + onDidDispose: () => {} + }; + + provider.resolveWebviewView(mockWebviewView1); + assert.equal(firstListenerDisposed, false, 'first listener must be active initially'); + + // Trigger message on first view + activeMessageHandler({ command: 'openDashboard' }); + assert.equal(executedCommand, 'codecritter.showDashboard'); + + // Second time resolving view (e.g. view hidden and reopened) + const mockWebviewView2 = { + webview: { + options: {}, + html: '', + onDidReceiveMessage: () => { + return { + dispose: () => { secondListenerDisposed = true; } + }; + } + }, + onDidDispose: () => {} + }; + + provider.resolveWebviewView(mockWebviewView2); + + // The first listener must have been disposed when resolving the second view! + assert.equal(firstListenerDisposed, true, 'previous message listener must be disposed on view reopen'); + assert.equal(secondListenerDisposed, false, 'new message listener must remain active'); + + // Disposing provider cleans up second listener + provider.dispose(); + assert.equal(secondListenerDisposed, true, 'provider.dispose() must dispose active listener'); +});