From 277a708d8e5486345e62befffa0747423d7aa7b1 Mon Sep 17 00:00:00 2001
From: atul-upadhyay-7 <atul-upadhyay-7@users.noreply.github.com>
Date: Sat, 20 Jun 2026 01:21:33 +0530
Subject: [PATCH] fix(ui): clean up store subscription on
 NotificationCenter.destroy()

NotificationCenter subscribes to NotificationStore in its constructor
but only cleaned up in unmount(). The base Widget.destroy() does not
call unmount(), so the subscription was never removed when the widget
was destroyed, causing memory leaks and callbacks on destroyed widgets.

Extract shared cleanup into a private _cleanup() method and call it
from both unmount() and a new destroy() override.

Closes #1662
---
 packages/ui/src/NotificationCenter.test.ts | 32 ++++++++++++++++++++++
 packages/ui/src/NotificationCenter.ts      | 17 ++++++++++--
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/packages/ui/src/NotificationCenter.test.ts b/packages/ui/src/NotificationCenter.test.ts
index 09e02f98..66996052 100644
--- a/packages/ui/src/NotificationCenter.test.ts
+++ b/packages/ui/src/NotificationCenter.test.ts
@@ -339,6 +339,38 @@ describe('NotificationCenter rendering and lifecycle', () => {
         expect(renderCenter(center)).not.toContain('+ After');
     });
 
+    it('destroy() cleans up store subscription and ignores later store updates', () => {
+        vi.spyOn(caps, 'unicode', 'get').mockReturnValue(false);
+        const center = new NotificationCenter({ width: 24 });
+
+        store.push('Before', 'info');
+        expect(renderCenter(center)).toContain('i Before');
+
+        center.destroy();
+        expect(renderCenter(center)).not.toContain('i Before');
+
+        store.push('After', 'success');
+        expect(renderCenter(center)).not.toContain('+ After');
+    });
+
+    it('destroy() is safe to call multiple times', () => {
+        const center = new NotificationCenter({ width: 24 });
+        center.destroy();
+        center.destroy();
+        // No error thrown
+    });
+
+    it('destroy() stops store callbacks from firing on the widget', () => {
+        const center = new NotificationCenter({ width: 24 });
+        const markDirtySpy = vi.spyOn(center, 'markDirty');
+
+        center.destroy();
+        markDirtySpy.mockClear();
+
+        store.push('After destroy', 'info');
+        expect(markDirtySpy).not.toHaveBeenCalled();
+    });
+
     it('renders safely for zero-sized screens, empty messages, many notifications, and long messages', () => {
         vi.spyOn(caps, 'unicode', 'get').mockReturnValue(false);
         const center = new NotificationCenter({ width: 80, maxVisible: 100 });
diff --git a/packages/ui/src/NotificationCenter.ts b/packages/ui/src/NotificationCenter.ts
index ade84450..8db5ec89 100644
--- a/packages/ui/src/NotificationCenter.ts
+++ b/packages/ui/src/NotificationCenter.ts
@@ -155,12 +155,23 @@ export class NotificationCenter extends Widget {
     }
 
     override unmount(): void {
-        this._unsub?.();
-        this._unsub = undefined;
-        this._current = [];
+        this._cleanup();
         super.unmount();
     }
 
+    override destroy(): void {
+        this._cleanup();
+        super.destroy();
+    }
+
+    private _cleanup(): void {
+        if (this._unsub) {
+            this._unsub();
+            this._unsub = undefined;
+        }
+        this._current = [];
+    }
+
     protected override _renderSelf(screen: Screen): void {
         if (this._maxVisible <= 0) return;