fix(subscriptions): unsubscribe every subcriptions while disposing comp

ghiscoding · Nov 2, 2021 · bf0dcd4 · bf0dcd4
1 parent 2721bcb
commit bf0dcd4
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 15 deletions.
diff --git a/packages/common/src/extensions/draggableGroupingExtension.ts b/packages/common/src/extensions/draggableGroupingExtension.ts
@@ -1,6 +1,6 @@
 import 'slickgrid/plugins/slick.draggablegrouping';
 
-import { DraggableGrouping, Extension, GetSlickEventType, GridOption, SlickDraggableGrouping, SlickEventHandler, SlickNamespace } from '../interfaces/index';
+import { DraggableGrouping, EventSubscription, Extension, GetSlickEventType, GridOption, SlickDraggableGrouping, SlickEventHandler, SlickNamespace } from '../interfaces/index';
 import { ExtensionUtility } from './extensionUtility';
 import { PubSubService } from '../services/pubSub.service';
 import { SharedService } from '../services/shared.service';
@@ -12,6 +12,7 @@ export class DraggableGroupingExtension implements Extension {
   private _addon: SlickDraggableGrouping | null = null;
   private _draggableGroupingOptions: DraggableGrouping | null = null;
   private _eventHandler: SlickEventHandler;
+  private _subscriptions: EventSubscription[] = [];
 
   constructor(private readonly extensionUtility: ExtensionUtility, private readonly pubSubService: PubSubService, private readonly sharedService: SharedService) {
     this._eventHandler = new Slick.EventHandler();
@@ -24,6 +25,7 @@ export class DraggableGroupingExtension implements Extension {
   dispose() {
     // unsubscribe all SlickGrid events
     this._eventHandler.unsubscribeAll();
+    this.pubSubService.unsubscribeAll(this._subscriptions);
 
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
@@ -74,7 +76,9 @@ export class DraggableGroupingExtension implements Extension {
         }
 
         // we also need to subscribe to a possible user clearing the grouping via the Context Menu, we need to clear the pre-header bar as well
-        this.pubSubService.subscribe('onContextMenuClearGrouping', () => this._addon?.clearDroppedGroups?.());
+        this._subscriptions.push(
+          this.pubSubService.subscribe('onContextMenuClearGrouping', () => this._addon?.clearDroppedGroups?.())
+        );
       }
 
       return this._addon;

diff --git a/packages/common/src/services/groupingAndColspan.service.ts b/packages/common/src/services/groupingAndColspan.service.ts
@@ -1,13 +1,14 @@
 import {
   Column,
-  SlickDataView,
+  EventSubscription,
   GetSlickEventType,
   GridOption,
+  SlickColumnPicker,
+  SlickDataView,
   SlickEventHandler,
   SlickGrid,
   SlickNamespace,
   SlickResizer,
-  SlickColumnPicker,
 } from './../interfaces/index';
 import { ExtensionName } from '../enums/index';
 import { ExtensionUtility } from '../extensions/extensionUtility';
@@ -21,6 +22,7 @@ declare const Slick: SlickNamespace;
 export class GroupingAndColspanService {
   protected _eventHandler: SlickEventHandler;
   protected _grid!: SlickGrid;
+  protected _subscriptions: EventSubscription[] = [];
 
   constructor(protected readonly extensionUtility: ExtensionUtility, protected readonly extensionService: ExtensionService, protected readonly pubSubService: PubSubService,) {
     this._eventHandler = new Slick.EventHandler();
@@ -75,9 +77,9 @@ export class GroupingAndColspanService {
         if (columnPickerExtension?.instance?.onColumnsChanged) {
           this._eventHandler.subscribe(columnPickerExtension.instance.onColumnsChanged, () => this.renderPreHeaderRowGroupingTitles());
         }
-        this.pubSubService.subscribe('onHeaderMenuHideColumns', () => {
-          this.delayRenderPreHeaderRowGroupingTitles(0);
-        });
+        this._subscriptions.push(
+          this.pubSubService.subscribe('onHeaderMenuHideColumns', () => this.delayRenderPreHeaderRowGroupingTitles(0))
+        );
 
         const gridMenuExtension = this.extensionService.getExtensionByName(ExtensionName.gridMenu);
         if (gridMenuExtension && gridMenuExtension.instance && gridMenuExtension.instance.onColumnsChanged && gridMenuExtension.instance.onMenuClose) {
@@ -110,6 +112,7 @@ export class GroupingAndColspanService {
   dispose() {
     // unsubscribe all SlickGrid events
     this._eventHandler.unsubscribeAll();
+    this.pubSubService.unsubscribeAll(this._subscriptions);
   }
 
   /** call "renderPreHeaderRowGroupingTitles()" with a setTimeout delay */

diff --git a/packages/common/src/services/resizer.service.ts b/packages/common/src/services/resizer.service.ts
@@ -2,6 +2,7 @@ import { FieldType, } from '../enums/index';
 import {
   AutoResizeOption,
   Column,
+  EventSubscription,
   GetSlickEventType,
   GridOption,
   GridSize,
@@ -43,6 +44,7 @@ export class ResizerService {
   protected _timer!: NodeJS.Timeout;
   protected _resizePaused = false;
   protected _resizeObserver!: ResizeObserver;
+  protected _subscriptions: EventSubscription[] = [];
 
   get eventHandler(): SlickEventHandler {
     return this._eventHandler;
@@ -85,6 +87,8 @@ export class ResizerService {
   dispose() {
     // unsubscribe all SlickGrid events
     this._eventHandler?.unsubscribeAll();
+    this.pubSubService.unsubscribeAll(this._subscriptions);
+
     if (this._intervalId) {
       clearInterval(this._intervalId);
     }
@@ -138,15 +142,19 @@ export class ResizerService {
     // Events
     if (this.gridOptions.autoResize) {
       // resize by content could be called from the outside by other services via pub/sub event
-      this.pubSubService.subscribe('onFullResizeByContentRequested', () => this.resizeColumnsByCellContent(true));
+      this._subscriptions.push(
+        this.pubSubService.subscribe('onFullResizeByContentRequested', () => this.resizeColumnsByCellContent(true))
+      );
     }
 
     // on double-click resize, should we resize the cell by its cell content?
     // the same action can be called from a double-click and/or from column header menu
     if (this.gridOptions?.enableColumnResizeOnDoubleClick) {
-      this.pubSubService.subscribe('onHeaderMenuColumnResizeByContent', (data => {
-        this.handleSingleColumnResizeByContent(data.columnId);
-      }));
+      this._subscriptions.push(
+        this.pubSubService.subscribe('onHeaderMenuColumnResizeByContent', (data => {
+          this.handleSingleColumnResizeByContent(data.columnId);
+        }))
+      );
 
       const onColumnsResizeDblClickHandler = this._grid.onColumnsResizeDblClick;
       (this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onColumnsResizeDblClickHandler>>).subscribe(onColumnsResizeDblClickHandler, (_e, args) => {

diff --git a/packages/common/src/services/treeData.service.ts b/packages/common/src/services/treeData.service.ts
@@ -3,6 +3,7 @@ import { ToggleStateChangeType, ToggleStateChangeTypeString } from '../enums/ind
 import {
   Column,
   ColumnSort,
+  EventSubscription,
   GetSlickEventType,
   GridOption,
   OnClickEventArgs,
@@ -29,6 +30,7 @@ export class TreeDataService {
   protected _currentToggledItems: TreeToggledItem[] = [];
   protected _grid!: SlickGrid;
   protected _eventHandler: SlickEventHandler;
+  protected _subscriptions: EventSubscription[] = [];
 
   constructor(protected readonly pubSubService: PubSubService, protected readonly sharedService: SharedService, protected readonly sortService: SortService) {
     this._eventHandler = new Slick.EventHandler();
@@ -65,9 +67,8 @@ export class TreeDataService {
 
   dispose() {
     // unsubscribe all SlickGrid events
-    if (this._eventHandler?.unsubscribeAll) {
-      this._eventHandler.unsubscribeAll();
-    }
+    this._eventHandler.unsubscribeAll();
+    this.pubSubService.unsubscribeAll(this._subscriptions);
   }
 
   init(grid: SlickGrid) {
@@ -106,7 +107,9 @@ export class TreeDataService {
     }
 
     // when "Clear all Sorting" is triggered by the Grid Menu, we'll resort with `initialSort` when defined (or else by 'id')
-    this.pubSubService.subscribe('onGridMenuClearAllSorting', this.clearSorting.bind(this));
+    this._subscriptions.push(
+      this.pubSubService.subscribe('onGridMenuClearAllSorting', this.clearSorting.bind(this))
+    );
   }
 
   /**