fix: add unsubscribe mechanism to prevent callback memory leaks

packages/phoenix-event-display/src/event-display.ts

@@ -584,18 +584,36 @@ export class EventDisplay {
    * Add a callback to onDisplayedEventChange array to call
    * the callback on changes to the displayed event.
    * @param callback Callback to be added to the onDisplayedEventChange array.
+   * @returns Unsubscribe function to remove the callback.
    */
-  public listenToDisplayedEventChange(callback: (event: any) => any) {
+  public listenToDisplayedEventChange(
+    callback: (event: any) => any,
+  ): () => void {
     this.onDisplayedEventChange.push(callback);
+    return () => {
+      const index = this.onDisplayedEventChange.indexOf(callback);
+      if (index > -1) {
+        this.onDisplayedEventChange.splice(index, 1);
+      }
+    };
   }
 
   /**
    * Add a callback to onEventsChange array to call
    * the callback on changes to the events.
    * @param callback Callback to be added to the onEventsChange array.
+   * @returns Unsubscribe function to remove the callback.
    */
-  public listenToLoadedEventsChange(callback: (events: any) => any) {
+  public listenToLoadedEventsChange(
+    callback: (events: any) => any,
+  ): () => void {
     this.onEventsChange.push(callback);
+    return () => {
+      const index = this.onEventsChange.indexOf(callback);
+      if (index > -1) {
+        this.onEventsChange.splice(index, 1);
+      }
+    };
   }
 
   /**

packages/phoenix-event-display/src/helpers/active-variable.ts

@@ -28,8 +28,15 @@ export class ActiveVariable<T = any> {
   /**
    * Call a function on updating the value of variable.
    * @param callback Callback to call with updated value when the variable is updated.
+   * @returns Unsubscribe function to remove the callback.
    */
-  public onUpdate(callback: CallbackFunction<T>) {
+  public onUpdate(callback: CallbackFunction<T>): () => void {
     this.callbacks.push(callback);
+    return () => {
+      const index = this.callbacks.indexOf(callback);
+      if (index > -1) {
+        this.callbacks.splice(index, 1);
+      }
+    };
   }
 }

packages/phoenix-ng/projects/phoenix-app/src/app/sections/playground/playground.component.test.ts

@@ -24,7 +24,7 @@ describe('PlaygroundComponent', () => {
     clippingEnabled: jest.fn().mockReturnThis(),
     startClippingAngle: jest.fn().mockReturnThis(),
     openingClippingAngle: jest.fn().mockReturnThis(),
-    listenToLoadedEventsChange: jest.fn(),
+    listenToLoadedEventsChange: jest.fn(() => jest.fn()),
     getUIManager: jest.fn().mockReturnThis(),
     getThreeManager: jest.fn().mockReturnThis(),
     getPresetViews: jest.fn().mockReturnThis().mockReturnValue([]),
@@ -37,9 +37,9 @@ describe('PlaygroundComponent', () => {
   };
 
   const mockStateManager = mockEventDisplay.getStateManager();
-  mockStateManager.clippingEnabled.onUpdate = jest.fn();
-  mockStateManager.startClippingAngle.onUpdate = jest.fn();
-  mockStateManager.openingClippingAngle.onUpdate = jest.fn();
+  mockStateManager.clippingEnabled.onUpdate = jest.fn(() => jest.fn());
+  mockStateManager.startClippingAngle.onUpdate = jest.fn(() => jest.fn());
+  mockStateManager.openingClippingAngle.onUpdate = jest.fn(() => jest.fn());
 
   beforeEach(() => {
     TestBed.configureTestingModule({

packages/phoenix-ng/projects/phoenix-app/src/app/sections/trackml/trackml.component.test.ts

@@ -29,7 +29,7 @@ describe('TrackmlComponent', () => {
     addLoadListenerWithCheck: jest.fn().mockImplementation(() => {
       component.loaded = true;
     }),
-    listenToLoadedEventsChange: jest.fn(),
+    listenToLoadedEventsChange: jest.fn(() => jest.fn()),
     getUIManager: jest.fn().mockReturnThis(),
     getThreeManager: jest.fn().mockReturnThis(),
     getPresetViews: jest.fn().mockReturnValue([]),
@@ -43,9 +43,9 @@ describe('TrackmlComponent', () => {
   };
 
   const mockStateManager = mockEventDisplay.getStateManager();
-  mockStateManager.clippingEnabled.onUpdate = jest.fn();
-  mockStateManager.startClippingAngle.onUpdate = jest.fn();
-  mockStateManager.openingClippingAngle.onUpdate = jest.fn();
+  mockStateManager.clippingEnabled.onUpdate = jest.fn(() => jest.fn());
+  mockStateManager.startClippingAngle.onUpdate = jest.fn(() => jest.fn());
+  mockStateManager.openingClippingAngle.onUpdate = jest.fn(() => jest.fn());
 
   beforeEach(async () => {
     await TestBed.configureTestingModule({

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/collections-info/collections-info-overlay/collections-info-overlay.component.test.ts

@@ -10,14 +10,18 @@ describe('CollectionsInfoOverlayComponent', () => {
   let fixture: ComponentFixture<CollectionsInfoOverlayComponent>;
 
   const mockEventDisplay = {
-    listenToDisplayedEventChange: jest.fn((callback) => callback()),
+    listenToDisplayedEventChange: jest.fn((callback) => {
+      callback();
+      return jest.fn();
+    }),
     getCollections: jest.fn().mockReturnValue({
       Hits: ['hitsCollection1', 'hitsCollection2'],
       Tracks: ['trackCollection1'],
     }),
     getActiveObjectId: () => ({
       onUpdate: (callback) => {
         callback();
+        return jest.fn();
       },
     }),
     enableHighlighting: jest.fn(),
@@ -55,7 +59,7 @@ describe('CollectionsInfoOverlayComponent', () => {
     component.activeObject = {
       value: '',
       update: jest.fn(),
-      onUpdate: jest.fn(),
+      onUpdate: jest.fn(() => jest.fn()),
     } as any;
   });
 

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/collections-info/collections-info-overlay/collections-info-overlay.component.ts

@@ -1,4 +1,10 @@
-import { Component, ElementRef, Input, type OnInit } from '@angular/core';
+import {
+  Component,
+  ElementRef,
+  Input,
+  type OnInit,
+  type OnDestroy,
+} from '@angular/core';
 import {
   ActiveVariable,
   PrettySymbols,
@@ -12,7 +18,7 @@ import { EventDisplayService } from '../../../../services/event-display.service'
   templateUrl: './collections-info-overlay.component.html',
   styleUrls: ['./collections-info-overlay.component.scss'],
 })
-export class CollectionsInfoOverlayComponent implements OnInit {
+export class CollectionsInfoOverlayComponent implements OnInit, OnDestroy {
   @Input() showObjectsInfo: boolean;
   hideInvisible: boolean;
   collections: { type: string; collections: string[] }[];
@@ -21,30 +27,39 @@ export class CollectionsInfoOverlayComponent implements OnInit {
   collectionColumns: string[];
   getPrettySymbol = PrettySymbols.getPrettySymbol;
   activeObject: ActiveVariable<string>;
+  private unsubscribes: (() => void)[] = [];
 
   constructor(
     private elementRef: ElementRef,
     private eventDisplay: EventDisplayService,
   ) {}
 
   ngOnInit() {
-    this.eventDisplay.listenToDisplayedEventChange(() => {
-      const collectionsGrouped: { [key: string]: string[] } =
-        this.eventDisplay.getCollections();
-      this.collections = Object.entries(collectionsGrouped).map(
-        ([type, collections]: [string, string[]]) => ({
-          type,
-          collections,
-        }),
-      );
-    });
+    this.unsubscribes.push(
+      this.eventDisplay.listenToDisplayedEventChange(() => {
+        const collectionsGrouped: { [key: string]: string[] } =
+          this.eventDisplay.getCollections();
+        this.collections = Object.entries(collectionsGrouped).map(
+          ([type, collections]: [string, string[]]) => ({
+            type,
+            collections,
+          }),
+        );
+      }),
+    );
 
     this.activeObject = this.eventDisplay.getActiveObjectId();
-    this.activeObject.onUpdate((value: string) => {
-      if (document.getElementById(value)) {
-        document.getElementById(value).scrollIntoView(false);
-      }
-    });
+    this.unsubscribes.push(
+      this.activeObject.onUpdate((value: string) => {
+        if (document.getElementById(value)) {
+          document.getElementById(value).scrollIntoView(false);
+        }
+      }),
+    );
+  }
+
+  ngOnDestroy() {
+    this.unsubscribes.forEach((unsubscribe) => unsubscribe?.());
   }
 
   changeCollection(selectedCollection: string) {

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/cycle-events/cycle-events.component.test.ts

@@ -9,9 +9,10 @@ describe('CycleEventsComponent', () => {
   let fixture: ComponentFixture<CycleEventsComponent>;
 
   const mockEventDisplay = {
-    listenToLoadedEventsChange: jest.fn((callback) =>
-      callback(['eventKey1', 'eventKey2']),
-    ),
+    listenToLoadedEventsChange: jest.fn((callback) => {
+      callback(['eventKey1', 'eventKey2']);
+      return jest.fn();
+    }),
     loadEvent: jest.fn(),
   };
 

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/cycle-events/cycle-events.component.ts

@@ -25,6 +25,7 @@ export class CycleEventsComponent implements OnInit, OnDestroy {
 
   private events: string[];
   private eventsChangeCallback: (events: string[]) => void;
+  private unsubscribe: () => void;
 
   constructor(
     private eventDisplay: EventDisplayService,
@@ -40,7 +41,9 @@ export class CycleEventsComponent implements OnInit, OnDestroy {
         this.startCycleInterval();
       }
     };
-    this.eventDisplay.listenToLoadedEventsChange(this.eventsChangeCallback);
+    this.unsubscribe = this.eventDisplay.listenToLoadedEventsChange(
+      this.eventsChangeCallback,
+    );
   }
 
   ngOnDestroy(): void {
@@ -49,6 +52,8 @@ export class CycleEventsComponent implements OnInit, OnDestroy {
       clearInterval(this.intervalId);
       this.intervalId = null;
     }
+    // Unsubscribe from events change listener
+    this.unsubscribe?.();
   }
 
   toggleCycle() {

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/event-selector/event-selector.component.test.ts

@@ -9,7 +9,7 @@ describe('EventSelectorComponent', () => {
   let fixture: ComponentFixture<EventSelectorComponent>;
 
   const mockEventDisplayService = {
-    listenToLoadedEventsChange: jest.fn(),
+    listenToLoadedEventsChange: jest.fn(() => jest.fn()),
     loadEvent: jest.fn(),
   };
 

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/event-selector/event-selector.component.ts

@@ -10,22 +10,26 @@ import { EventDisplayService } from '../../../services/event-display.service';
 export class EventSelectorComponent implements OnInit, OnDestroy {
   // Array containing the keys of the multiple loaded events
   events: string[];
+  private unsubscribe: () => void;
 
   /** Prevents callbacks on destroyed component */
   private isDestroyed = false;
 
   constructor(private eventDisplay: EventDisplayService) {}
 
   ngOnInit() {
-    this.eventDisplay.listenToLoadedEventsChange((events) => {
-      if (!this.isDestroyed) {
-        this.events = events;
-      }
-    });
+    this.unsubscribe = this.eventDisplay.listenToLoadedEventsChange(
+      (events) => {
+        if (!this.isDestroyed) {
+          this.events = events;
+        }
+      },
+    );
   }
 
   ngOnDestroy() {
     this.isDestroyed = true;
+    this.unsubscribe?.();
   }
 
   changeEvent(selected: any) {

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/experiment-info/experiment-info.component.test.ts

@@ -10,7 +10,7 @@ describe('ExperimentInfoComponent', () => {
 
   const mockEventDisplayService = {
     getEventMetadata: jest.fn(),
-    listenToDisplayedEventChange: jest.fn(),
+    listenToDisplayedEventChange: jest.fn(() => jest.fn()),
   };
 
   beforeEach(() => {

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/experiment-info/experiment-info.component.ts

@@ -1,4 +1,4 @@
-import { Component, type OnInit, Input } from '@angular/core';
+import { Component, type OnInit, type OnDestroy, Input } from '@angular/core';
 import { EventDisplayService } from '../../../services/event-display.service';
 
 @Component({
@@ -7,17 +7,22 @@ import { EventDisplayService } from '../../../services/event-display.service';
   templateUrl: './experiment-info.component.html',
   styleUrls: ['./experiment-info.component.scss'],
 })
-export class ExperimentInfoComponent implements OnInit {
+export class ExperimentInfoComponent implements OnInit, OnDestroy {
   experimentInfo: any[];
   @Input() url: string;
   @Input() logo: string;
   @Input() tagline: string;
+  private unsubscribe: () => void;
 
   constructor(private eventDisplay: EventDisplayService) {}
 
   ngOnInit(): void {
-    this.eventDisplay.listenToDisplayedEventChange(() => {
+    this.unsubscribe = this.eventDisplay.listenToDisplayedEventChange(() => {
       this.experimentInfo = this.eventDisplay.getEventMetadata();
     });
   }
+
+  ngOnDestroy(): void {
+    this.unsubscribe?.();
+  }
 }

packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/geometry-browser/geometry-browser-overlay/geometry-browser-overlay.component.ts

@@ -1,4 +1,4 @@
-import { Component, type OnInit, Input } from '@angular/core';
+import { Component, type OnInit, type OnDestroy, Input } from '@angular/core';
 import { ActiveVariable } from 'phoenix-event-display';
 import { EventDisplayService } from '../../../../services/event-display.service';
 import { Object3D, type Object3DEventMap } from 'three';
@@ -9,12 +9,13 @@ import { Object3D, type Object3DEventMap } from 'three';
   templateUrl: './geometry-browser-overlay.component.html',
   styleUrls: ['./geometry-browser-overlay.component.scss'],
 })
-export class GeometryBrowserOverlayComponent implements OnInit {
+export class GeometryBrowserOverlayComponent implements OnInit, OnDestroy {
   @Input() browseDetectorParts: boolean;
   selectedCollection: string;
   showingCollection: any;
   activeObject: ActiveVariable<string>;
   children: Object3D<Object3DEventMap>[];
+  private unsubscribe: () => void;
 
   constructor(private eventDisplay: EventDisplayService) {}
 
@@ -25,13 +26,17 @@ export class GeometryBrowserOverlayComponent implements OnInit {
       .getGeometries().children;
 
     this.activeObject = this.eventDisplay.getActiveObjectId();
-    this.activeObject.onUpdate((value: string) => {
+    this.unsubscribe = this.activeObject.onUpdate((value: string) => {
       if (document.getElementById(value)) {
         document.getElementById(value).scrollIntoView(false);
       }
     });
   }
 
+  ngOnDestroy() {
+    this.unsubscribe?.();
+  }
+
   changeCollection(selectedCollection: string) {
     this.selectedCollection = selectedCollection;
     for (const child of this.children) {