fix(events): every subscribed event should have an unsubscribe

- add all missing unsubscribe SlickGrid / dispose Aurelia
- use the Slick EventHandler when possible, with unsubscribeAll
- also rename destroy() functions to dispose() in all Services

aurelia-slickgrid/src/aurelia-slickgrid/aurelia-slickgrid.ts

@@ -45,7 +45,7 @@ import {
 } from './services/index';
 import * as $ from 'jquery';
 
-// using external js modules in Aurelia
+// using external non-typed js libraries
 declare var Slick: any;
 
 const eventPrefix = 'sg';
@@ -147,12 +147,12 @@ export class AureliaSlickgridCustomElement {
       detail: this.grid
     }));
     this.dataview = [];
-    this.controlAndPluginService.destroy();
-    this.gridEventService.dispose();
-    this.filterService.destroy();
-    this.resizer.destroy();
-    this.sortService.destroy();
     this._eventHandler.unsubscribeAll();
+    this.controlAndPluginService.dispose();
+    this.gridEventService.dispose();
+    this.filterService.dispose();
+    this.resizer.dispose();
+    this.sortService.dispose();
     this.grid.destroy();
     this.localeChangedSubscriber.dispose();
     this.ea.publish('onAfterGridDestroyed', true);
@@ -176,10 +176,6 @@ export class AureliaSlickgridCustomElement {
     };
   }
 
-  unbind(binding: any, scope: any) {
-    this.resizer.destroy();
-  }
-
   datasetChanged(newValue: any[], oldValue: any[]) {
     this._dataset = newValue;
     this.refreshGridData(newValue);
@@ -309,11 +305,11 @@ export class AureliaSlickgridCustomElement {
     this.gridEventService.attachOnCellChange(grid, this._gridOptions, dataView);
     this.gridEventService.attachOnClick(grid, this._gridOptions, dataView);
 
-    dataView.onRowCountChanged.subscribe((e: any, args: any) => {
+    this._eventHandler.subscribe(dataView.onRowCountChanged, (e: any, args: any) => {
       grid.updateRowCount();
       grid.render();
     });
-    dataView.onRowsChanged.subscribe((e: any, args: any) => {
+    this._eventHandler.subscribe(dataView.onRowsChanged, (e: any, args: any) => {
       grid.invalidateRows(args.rows);
       grid.render();
     });

aurelia-slickgrid/src/aurelia-slickgrid/services/controlAndPlugin.service.ts

@@ -195,7 +195,7 @@ export class ControlAndPluginService {
     this._grid.autosizeColumns();
   }
 
-  destroy() {
+  dispose() {
     this._grid = null;
     this._dataView = null;
     this.visibleColumns = [];

aurelia-slickgrid/src/aurelia-slickgrid/services/filter.service.ts

@@ -19,13 +19,14 @@ declare var Slick: any;
 
 @inject(FilterFactory)
 export class FilterService {
+  private _eventHandler = new Slick.EventHandler();
+  private _subscriber: SlickEvent = new Slick.Event();
   private _filters: any[] = [];
   private _columnFilters: ColumnFilters = {};
   private _dataView: any;
   private _grid: any;
   private _gridOptions: GridOption;
   private _onFilterChangedOptions: any;
-  private subscriber: SlickEvent;
   onFilterChanged = new EventAggregator();
 
   constructor(private filterFactory: FilterFactory) { }
@@ -41,12 +42,12 @@ export class FilterService {
    * @param gridOptions Grid Options object
    */
   attachBackendOnFilter(grid: any, options: GridOption) {
-    this.subscriber = new Slick.Event();
+    this._filters = [];
     this.emitFilterChangedBy('remote');
-    this.subscriber.subscribe(this.attachBackendOnFilterSubscribe);
+    this._subscriber.subscribe(this.attachBackendOnFilterSubscribe);
 
-    this._filters = [];
-    grid.onHeaderRowCellRendered.subscribe((e: Event, args: any) => {
+    // subscribe to SlickGrid onHeaderRowCellRendered event to create filter template
+    this._eventHandler.subscribe(grid.onHeaderRowCellRendered, (e: Event, args: any) => {
       this.addFilterTemplateToHeaderRow(args);
     });
   }
@@ -117,21 +118,21 @@ export class FilterService {
    */
   attachLocalOnFilter(grid: any, options: GridOption, dataView: any) {
     this._dataView = dataView;
-    this.subscriber = new Slick.Event();
+    this._filters = [];
     this.emitFilterChangedBy('local');
 
     dataView.setFilterArgs({ columnFilters: this._columnFilters, grid: this._grid });
     dataView.setFilter(this.customLocalFilter.bind(this, dataView));
 
-    this.subscriber.subscribe((e: any, args: any) => {
+    this._subscriber.subscribe((e: any, args: any) => {
       const columnId = args.columnId;
       if (columnId != null) {
         dataView.refresh();
       }
     });
 
-    this._filters = [];
-    grid.onHeaderRowCellRendered.subscribe((e: Event, args: any) => {
+    // subscribe to SlickGrid onHeaderRowCellRendered event to create filter template
+    this._eventHandler.subscribe(grid.onHeaderRowCellRendered, (e: Event, args: any) => {
       this.addFilterTemplateToHeaderRow(args);
     });
   }
@@ -141,6 +142,9 @@ export class FilterService {
       const columnFilter = args.columnFilters[columnId];
       const columnIndex = args.grid.getColumnIndex(columnId);
       const columnDef = args.grid.getColumns()[columnIndex];
+      if (!columnDef) {
+        return false;
+      }
       const fieldType = columnDef.type || FieldType.string;
       const filterSearchType = (columnDef.filterSearchType) ? columnDef.filterSearchType : null;
 
@@ -219,17 +223,22 @@ export class FilterService {
     return true;
   }
 
-  destroy() {
-    this.destroyFilters();
-    if (this.subscriber && typeof this.subscriber.unsubscribe === 'function') {
-      this.subscriber.unsubscribe();
+  dispose() {
+    this.disposeColumnFilters();
+
+    // unsubscribe all SlickGrid events
+    this._eventHandler.unsubscribeAll();
+
+    // unsubscribe local event
+    if (this._subscriber && typeof this._subscriber.unsubscribe === 'function') {
+      this._subscriber.unsubscribe();
     }
   }
 
   /**
-   * Destroy the filters, since it's a singleton, we don't want to affect other grids with same columns
+   * Dispose the filters, since it's a singleton, we don't want to affect other grids with same columns
    */
-  destroyFilters() {
+  disposeColumnFilters() {
     // we need to loop through all columnFilters and delete them 1 by 1
     // only trying to make columnFilter an empty (without looping) would not trigger a dataset change
     for (const columnId in this._columnFilters) {
@@ -266,7 +275,7 @@ export class FilterService {
       };
     }
 
-    this.triggerEvent(this.subscriber, {
+    this.triggerEvent(this._subscriber, {
       columnId,
       columnDef: args.columnDef || null,
       columnFilters: this._columnFilters,
@@ -339,7 +348,7 @@ export class FilterService {
    * @param {string} sender
    */
   emitFilterChangedBy(sender: string) {
-    this.subscriber.subscribe(() => this.onFilterChanged.publish('filterService:changed', `onFilterChanged by ${sender}`));
+    this._subscriber.subscribe(() => this.onFilterChanged.publish('filterService:changed', `onFilterChanged by ${sender}`));
   }
 
   private keepColumnFilters(searchTerm: string | number | boolean, searchTerms: any, columnDef: any) {

aurelia-slickgrid/src/aurelia-slickgrid/services/gridEvent.service.ts

@@ -1,6 +1,6 @@
 import { OnEventArgs, CellArgs, GridOption } from './../models/index';
 
-// using external js modules in Aurelia
+// using external non-typed js libraries
 declare var Slick: any;
 
 export class GridEventService {

aurelia-slickgrid/src/aurelia-slickgrid/services/resizer.service.ts

@@ -84,9 +84,9 @@ export class ResizerService {
   }
 
   /**
-   * Destroy function when element is destroyed
+   * Dispose function when element is destroyed
    */
-  destroy() {
+  dispose() {
     $(window).off('resize.grid');
   }
 

aurelia-slickgrid/src/aurelia-slickgrid/services/sort.service.ts

@@ -1,9 +1,9 @@
 import { EventAggregator } from 'aurelia-event-aggregator';
-import { FieldType, GridOption } from './../models/index';
+import { FieldType, GridOption, SlickEvent } from './../models/index';
 import { Sorters } from './../sorters/index';
 
 export class SortService {
-  subscriber: any;
+  private _subscriber: SlickEvent;
   onSortChanged = new EventAggregator();
 
   /**
@@ -12,9 +12,9 @@ export class SortService {
    * @param gridOptions Grid Options object
    */
   attachBackendOnSort(grid: any, gridOptions: GridOption) {
-    this.subscriber = grid.onSort;
+    this._subscriber = grid.onSort;
     this.emitSortChangedBy('remote');
-    this.subscriber.subscribe(this.attachBackendOnSortSubscribe);
+    this._subscriber.subscribe(this.attachBackendOnSortSubscribe);
   }
 
   async attachBackendOnSortSubscribe(event: Event, args: any) {
@@ -53,9 +53,9 @@ export class SortService {
    * @param dataView
    */
   attachLocalOnSort(grid: any, gridOptions: GridOption, dataView: any) {
-    this.subscriber = grid.onSort;
+    this._subscriber = grid.onSort;
     this.emitSortChangedBy('local');
-    this.subscriber.subscribe((e: any, args: any) => {
+    this._subscriber.subscribe((e: any, args: any) => {
       // multiSort and singleSort are not exactly the same, but we want to structure it the same for the (for loop) after
       // also to avoid having to rewrite the for loop in the sort, we will make the singleSort an array of 1 object
       const sortColumns = (args.multiColumnSort) ? args.sortCols : new Array({ sortAsc: args.sortAsc, sortCol: args.sortCol });
@@ -102,9 +102,10 @@ export class SortService {
     });
   }
 
-  destroy() {
-    if (this.subscriber && typeof this.subscriber.unsubscribe === 'function') {
-      this.subscriber.unsubscribe();
+  dispose() {
+    // unsubscribe local event
+    if (this._subscriber && typeof this._subscriber.unsubscribe === 'function') {
+      this._subscriber.unsubscribe();
     }
   }
 
@@ -114,6 +115,6 @@ export class SortService {
    * @param sender
    */
   emitSortChangedBy(sender: string) {
-    this.subscriber.subscribe(() => this.onSortChanged.publish('sortService:changed', `onSortChanged by ${sender}`));
+    this._subscriber.subscribe(() => this.onSortChanged.publish('sortService:changed', `onSortChanged by ${sender}`));
   }
 }

aurelia-slickgrid/src/aurelia-slickgrid/slick-pagination.ts

@@ -1,10 +1,13 @@
 import { bindable, inject } from 'aurelia-framework';
+import { Subscription } from 'aurelia-event-aggregator';
 import { GridOption } from './models/index';
 import { FilterService } from './services/filter.service';
 import { SortService } from './services/sort.service';
 
 @inject(FilterService, SortService)
 export class SlickPaginationCustomElement {
+  private _filterSubcription: Subscription;
+  private _sorterSubcription: Subscription;
   @bindable() grid: any;
   @bindable() gridPaginationOptions: GridOption;
   private _gridPaginationOptions: GridOption;
@@ -31,14 +34,18 @@ export class SlickPaginationCustomElement {
     }
 
     // Subscribe to Event Emitter of Filter & Sort changed, go back to page 1 when that happen
-    this.filterService.onFilterChanged.subscribe('filterService:changed', (data: string) => {
+    this._filterSubcription = this.filterService.onFilterChanged.subscribe('filterService:changed', (data: string) => {
       this.refreshPagination(true);
     });
-    this.sortService.onSortChanged.subscribe('sortService:changed', (data: string) => {
+    this._sorterSubcription = this.sortService.onSortChanged.subscribe('sortService:changed', (data: string) => {
       this.refreshPagination(true);
     });
   }
 
+  detached() {
+    this.dispose();
+  }
+
   ceil(number: number) {
     return Math.ceil(number);
   }
@@ -67,6 +74,15 @@ export class SlickPaginationCustomElement {
     }
   }
 
+  dispose() {
+    if (this._filterSubcription) {
+      this._filterSubcription.dispose();
+    }
+    if (this._sorterSubcription) {
+      this._sorterSubcription.dispose();
+    }
+  }
+
   onChangeItemPerPage(event: any) {
     const itemsPerPage = +event.target.value;
     this.pageCount = Math.ceil(this.totalItems / itemsPerPage);

aurelia-slickgrid/src/examples/slickgrid/example10.ts

@@ -26,6 +26,12 @@ export class Example2 {
     this.getData();
   }
 
+  detached() {
+    // unsubscrible any Slick.Event you might have used
+    // a reminder again, these are SlickGrid Event, not Event Aggregator events
+    this.gridObj.onSelectedRowsChanged.unsubscribe();
+  }
+
   /* Define grid Options and Columns */
   defineGrid() {
     this.columnDefinitions = [

aurelia-slickgrid/src/examples/slickgrid/example3.ts

@@ -33,6 +33,13 @@ export class Example3 {
     this.getData();
   }
 
+  detached() {
+    // unsubscrible any Slick.Event you might have used
+    // a reminder again, these are SlickGrid Event, not Event Aggregator events
+    this.gridObj.onCellChange.unsubscribe();
+    this.gridObj.onClick.unsubscribe();
+  }
+
   /* Define grid Options and Columns */
   defineGrid() {
     this.columnDefinitions = [