fix(Synchronizer): actually remove event listener on source removal

packages/tools/src/store/SynchronizerManager/Synchronizer.ts

@@ -36,6 +36,7 @@ class Synchronizer {
   private _eventName: string;
   private _auxiliaryEvents: auxiliaryEvent[];
   private _eventHandler: ISynchronizerEventHandler;
+  private _eventListener: EventListenerOrEventListenerObject;
   private _eventSource: eventSource;
   private _ignoreFiredEvents: boolean;
   private _sourceViewports: Array<Types.IViewportId>;
@@ -53,6 +54,7 @@ class Synchronizer {
     this._enabled = true;
     this._eventName = eventName;
     this._eventHandler = eventHandler;
+    this._eventListener = this._onEvent.bind(this);
     this._ignoreFiredEvents = false;
     this._sourceViewports = [];
     this._targetViewports = [];
@@ -130,12 +132,12 @@ class Synchronizer {
     const eventSource =
       this._eventSource === 'element' ? viewport.element : eventTarget;
 
-    eventSource.addEventListener(this._eventName, this._onEvent.bind(this));
+    eventSource.addEventListener(this._eventName, this._eventListener);
 
     // Use a default source of 'element' if not provided just like we do for the main event.
     this._auxiliaryEvents.forEach(({ name, source = 'element' }) => {
       const target = source === 'element' ? viewport.element : eventTarget;
-      target.addEventListener(name, this._onEvent.bind(this));
+      target.addEventListener(name, this._eventListener);
     });
 
     this._updateDisableHandlers();
@@ -205,16 +207,14 @@ class Synchronizer {
 
     this._sourceViewports.splice(index, 1);
 
-    //@ts-ignore
-    eventSource.removeEventListener(this._eventName, this._eventHandler);
+    eventSource.removeEventListener(this._eventName, this._eventListener);
 
-    this._auxiliaryEvents.forEach(({ name, source }) => {
+    this._auxiliaryEvents.forEach(({ name, source = 'element' }) => {
       const target =
         source === 'element'
           ? this.getViewportElement(viewportInfo)
           : eventTarget;
-      //@ts-ignore
-      target.removeEventListener(name, this._eventHandler);
+      target.removeEventListener(name, this._eventListener);
     });
 
     this._updateDisableHandlers();

packages/tools/test/synchronizer_test.js

@@ -0,0 +1,128 @@
+import * as cornerstone3D from '@cornerstonejs/core';
+import * as testUtils from '../../../utils/test/testUtils';
+import * as csTools3d from '../src/index';
+
+const { utilities, Enums } = cornerstone3D;
+const { ViewportType } = Enums;
+const { SynchronizerManager } = csTools3d;
+const renderingEngineId = utilities.uuidv4();
+
+const viewportId1 = 'VIEWPORT1';
+const viewportId2 = 'VIEWPORT2';
+
+describe('Synchronizer:', () => {
+  let testEnv;
+  let renderingEngine;
+
+  beforeEach(function () {
+    testEnv = testUtils.setupTestEnvironment({
+      renderingEngineId: renderingEngineId,
+      viewportIds: [viewportId1, viewportId2],
+    });
+
+    renderingEngine = testEnv.renderingEngine;
+  });
+
+  afterEach(function () {
+    testUtils.cleanupTestEnvironment({
+      renderingEngineId: renderingEngineId,
+      synchronizerId: 'testSynchronizer',
+    });
+  });
+
+  it('Should successfully remove event handlers on viewport removal', function () {
+    const [element1, _] = testUtils.createViewports(renderingEngine, [
+      {
+        viewportType: ViewportType.ORTHOGRAPHIC,
+        orientation: Enums.OrientationAxis.AXIAL,
+        viewportId: viewportId1,
+      },
+      {
+        viewportType: ViewportType.ORTHOGRAPHIC,
+        orientation: Enums.OrientationAxis.AXIAL,
+        viewportId: viewportId2,
+      },
+    ]);
+    let eventListenerCallCount = 0;
+    const eventListener = () => {
+      eventListenerCallCount += 1;
+    };
+
+    const synchronizer = SynchronizerManager.createSynchronizer(
+      'testSynchronizer',
+      'testEvent',
+      eventListener
+    );
+
+    const viewport1Info = {
+      renderingEngineId: renderingEngine.id,
+      viewportId: renderingEngine.getViewport(viewportId1).id,
+    };
+    const viewport2Info = {
+      renderingEngineId: renderingEngine.id,
+      viewportId: renderingEngine.getViewport(viewportId2).id,
+    };
+    synchronizer.addSource(viewport1Info);
+    synchronizer.addTarget(viewport2Info);
+    // we need a source so the event is fired, so remove and add the source back.
+    // there should be one event listener active after this
+    synchronizer.removeSource(viewport1Info);
+    synchronizer.addSource(viewport1Info);
+
+    element1.dispatchEvent(new CustomEvent('testEvent', {}));
+
+    expect(eventListenerCallCount).toEqual(1);
+  });
+
+  it('Should successfully remove auxilary event handlers on viewport removal', function () {
+    const [element1, _] = testUtils.createViewports(renderingEngine, [
+      {
+        viewportType: ViewportType.ORTHOGRAPHIC,
+        orientation: Enums.OrientationAxis.AXIAL,
+        viewportId: viewportId1,
+      },
+      {
+        viewportType: ViewportType.ORTHOGRAPHIC,
+        orientation: Enums.OrientationAxis.AXIAL,
+        viewportId: viewportId2,
+      },
+    ]);
+    let eventListenerCallCount = 0;
+    const eventListener = () => {
+      console.log('eventListener called');
+      eventListenerCallCount += 1;
+    };
+
+    const synchronizer = SynchronizerManager.createSynchronizer(
+      'testSynchronizer',
+      'testEvent',
+      eventListener,
+      {
+        auxiliaryEvents: [
+          {
+            name: 'auxiliaryEvent',
+          },
+        ],
+      }
+    );
+
+    const viewport1Info = {
+      renderingEngineId: renderingEngine.id,
+      viewportId: renderingEngine.getViewport(viewportId1).id,
+    };
+    const viewport2Info = {
+      renderingEngineId: renderingEngine.id,
+      viewportId: renderingEngine.getViewport(viewportId2).id,
+    };
+    synchronizer.addSource(viewport1Info);
+    synchronizer.addTarget(viewport2Info);
+    // we need a source so the event is fired, so remove and add the source back.
+    // there should be one event listener active after this
+    synchronizer.removeSource(viewport1Info);
+    synchronizer.addSource(viewport1Info);
+
+    element1.dispatchEvent(new CustomEvent('auxiliaryEvent', {}));
+
+    expect(eventListenerCallCount).toEqual(1);
+  });
+});