fix(ui): clean up store subscription on NotificationCenter.destroy()

[atul-upadhyay-7]

Summary

Fixes a memory leak in NotificationCenter where the store subscription was never cleaned up when the widget was destroyed via destroy().

Closes #1662

Root Cause

NotificationCenter subscribes to NotificationStore in its constructor and only unsubscribes in unmount(). The base Widget.destroy() method does not call unmount() — it only clears children, emits the unmount event, removes event listeners, and nulls the parent. This left the store subscription alive, causing:

• Memory leaks (instances never garbage collected)
• Callbacks firing on destroyed widgets (calling markDirty() on dead instances)
• Unbounded subscription growth in long-running apps

Fix

Extracted the subscription cleanup logic into a private _cleanup() method and call it from both unmount() and a new destroy() override:

override unmount(): void {
    this._cleanup();
    super.unmount();
}

override destroy(): void {
    this._cleanup();
    super.destroy();
}

private _cleanup(): void {
    if (this._unsub) {
        this._unsub();
        this._unsub = undefined;
    }
    this._current = [];
}

Tests added

Test	What it verifies
destroy() cleans up store subscription and ignores later store updates	The original bug scenario
destroy() is safe to call multiple times	Idempotency
destroy() stops store callbacks from firing on the widget	No markDirty after destroy

Verification

• All 2155 UI tests pass (including 3 new tests)
• Typecheck passes across all packages
• No breaking changes to public API

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • NotificationCenter now properly cleans up subscriptions when destroyed.
  • Callbacks are prevented from executing after component destruction.
  • destroy() can be safely called multiple times without errors.

[github-actions]

Hi @atul-upadhyay-7 👋

⭐ Star this repo before your PR merges.

Why? GSSoC 2026 contributors who star get priority review and points credit. After you star, push any commit (or re-run this check). The needs-star label lifts automatically.

Thanks for your contribution to TermUI.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0da6cbf0-bcbb-44c9-94d2-82ff1df1e743

📥 Commits

Reviewing files that changed from the base of the PR and between 4b874c0 and 277a708.

📒 Files selected for processing (2)

• packages/ui/src/NotificationCenter.test.ts
• packages/ui/src/NotificationCenter.ts

---

📝 Walkthrough

Walkthrough

NotificationCenter gains a private _cleanup() method that unsubscribes from NotificationStore and clears _current. Both unmount() and a new destroy() override call _cleanup() before delegating to super. Three tests are added to verify destroy() stops updates, is idempotent, and prevents markDirty from firing.

NotificationCenter destroy() lifecycle fix

Layer / File(s)	Summary
_cleanup() helper and destroy() override packages/ui/src/NotificationCenter.ts	Extracts the unsubscribe call and _current reset into a private _cleanup() method. unmount() now delegates to _cleanup() instead of inlining the teardown, and a new destroy() override calls _cleanup() then super.destroy().
destroy() test coverage packages/ui/src/NotificationCenter.test.ts	Adds three test cases: (1) destroy() removes the store subscription so subsequent store.push() calls no longer affect rendered output; (2) destroy() is safe to call multiple times; (3) markDirty is not triggered after destruction when the store receives new notifications.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Karanjot786/TermUI#877: Directly modifies NotificationCenter.ts unmount() unsubscription and _current clearing — the exact code path that this PR refactors into _cleanup().
• Karanjot786/TermUI#1049: Extends NotificationCenter.test.ts with lifecycle unsubscription/render-prevention assertions that parallel the destroy()-specific test cases added here.

Suggested labels

type:bug, quality:clean

Suggested reviewers

• Karanjot786

Poem

🐇 A widget once lingered long after its death,
Its store subscriptions still whispering with breath.
Now _cleanup() sweeps tidy, destroy() plays fair,
No ghost callbacks dangling in stale memory air.
The rabbit hops onward — no leaks anywhere! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main fix: cleaning up store subscription when NotificationCenter.destroy() is called.
Description check	✅ Passed	The PR description is comprehensive and well-structured, covering root cause, fix details, tests added, and verification with all required template sections properly filled.
Linked Issues check	✅ Passed	The PR successfully addresses all requirements from issue #1662: overrides destroy() to call cleanup logic, extracts shared _cleanup() method for unmount() and destroy() paths, and adds tests verifying the fix works correctly.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the NotificationCenter memory leak: implementation in NotificationCenter.ts and corresponding test coverage with no unrelated refactoring.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Warning

⚠️ This pull request shows signs of AI-generated slop (trivial_assertion). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.