fix(core): clear dataset when destroying & fix few unsubscribed events (

#629)

Co-authored-by: Ghislain Beaulac <SESA213338@se.com>

src/app/examples/grid-frozen.component.ts

@@ -33,8 +33,9 @@ export class GridFrozenComponent implements OnInit, OnDestroy {
 
   ngOnDestroy() {
     // unsubscribe every SlickGrid subscribed event (or use the Slick.EventHandler)
-    this.gridObj.onMouseEnter.unsubscribe();
-    this.gridObj.onMouseLeave.unsubscribe();
+    this.gridObj.onMouseEnter.unsubscribe(this.highlightRow);
+    this.gridObj.onMouseLeave.unsubscribe(this.highlightRow);
+    this.highlightRow = null;
   }
 
   angularGridReady(angularGrid: AngularGridInstance) {
@@ -44,15 +45,15 @@ export class GridFrozenComponent implements OnInit, OnDestroy {
     // with frozen (pinned) grid, in order to see the entire row being highlighted when hovering
     // we need to do some extra tricks (that is because frozen grids use 2 separate div containers)
     // the trick is to use row selection to highlight when hovering current row and remove selection once we're not
-    this.gridObj.onMouseEnter.subscribe(event => {
-      const cell = this.gridObj.getCellFromEvent(event);
-      this.gridObj.setSelectedRows([cell.row]); // highlight current row
-      event.preventDefault();
-    });
-    this.gridObj.onMouseLeave.subscribe(event => {
-      this.gridObj.setSelectedRows([]); // remove highlight
-      event.preventDefault();
-    });
+    this.gridObj.onMouseEnter.subscribe(event => this.highlightRow(event, true));
+    this.gridObj.onMouseLeave.subscribe(event => this.highlightRow(event, false));
+  }
+
+  highlightRow(event: Event, isMouseEnter: boolean) {
+    const cell = this.gridObj.getCellFromEvent(event);
+    const rows = isMouseEnter ? [cell.row] : [];
+    this.gridObj.setSelectedRows(rows); // highlight current row
+    event.preventDefault();
   }
 
   prepareDataGrid() {

src/app/examples/grid-rowmove.component.ts

@@ -1,11 +1,11 @@
-import { Component, OnInit } from '@angular/core';
+import { Component, OnDestroy, OnInit } from '@angular/core';
 import { AngularGridInstance, Column, ExtensionName, Filters, Formatters, GridOption } from './../modules/angular-slickgrid';
 import { TranslateService } from '@ngx-translate/core';
 
 @Component({
   templateUrl: './grid-rowmove.component.html'
 })
-export class GridRowMoveComponent implements OnInit {
+export class GridRowMoveComponent implements OnInit, OnDestroy {
   title = 'Example 17: Row Move & Checkbox Selector';
   subTitle = `This example demonstrates using the <b>Slick.Plugins.RowMoveManager</b> plugin to easily move a row in the grid.<br/>
     <ul>
@@ -41,6 +41,12 @@ export class GridRowMoveComponent implements OnInit {
     return this.angularGrid && this.angularGrid.extensionService.getSlickgridAddonInstance(ExtensionName.rowMoveManager) || {};
   }
 
+  ngOnDestroy() {
+    // nullify the callbacks to avoid mem leaks
+    this.onBeforeMoveRow = null;
+    this.onMoveRows = null;
+  }
+
   ngOnInit(): void {
     this.columnDefinitions = [
       { id: 'title', name: 'Title', field: 'title', filterable: true, },
@@ -95,8 +101,8 @@ export class GridRowMoveComponent implements OnInit {
         disableRowSelection: true,
         cancelEditOnDrag: true,
         width: 30,
-        onBeforeMoveRows: (e, args) => this.onBeforeMoveRow(e, args),
-        onMoveRows: (e, args) => this.onMoveRows(e, args),
+        onBeforeMoveRows: this.onBeforeMoveRow,
+        onMoveRows: this.onMoveRows.bind(this),
 
         // you can change the move icon position of any extension (RowMove, RowDetail or RowSelector icon)
         // note that you might have to play with the position when using multiple extension
@@ -165,8 +171,7 @@ export class GridRowMoveComponent implements OnInit {
     for (let i = 0; i < rows.length; i++) {
       selectedRows.push(left.length + i);
     }
-
-    this.angularGrid.slickGrid.resetActiveCell();
+    args.grid.resetActiveCell();
     this.dataset = tmpDataset;
   }
 

src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid-constructor.spec.ts

@@ -176,6 +176,7 @@ const mockDataView = {
   endUpdate: jest.fn(),
   getItem: jest.fn(),
   getItems: jest.fn(),
+  getLength: jest.fn(),
   getItemMetadata: jest.fn(),
   getPagingInfo: jest.fn(),
   mapIdsToRows: jest.fn(),
@@ -335,7 +336,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
 
   it('should load jQuery mousewheel when using a frozen grid', () => {
     const loadSpy = jest.spyOn(component, 'loadJqueryMousewheelDynamically');
-    component.gridOptions.frozenRow = 3;
+    component.gridOptions.frozenColumn = 3;
 
     component.ngOnInit();
     component.ngAfterViewInit();
@@ -688,7 +689,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
         component.ngAfterViewInit();
         component.destroy(true);
 
-        expect(spy).toHaveBeenCalledWith();
+        expect(spy).toHaveBeenCalled();
       });
 
       it('should refresh a local grid and change pagination options pagination when a preset for it is defined in grid options', (done) => {

src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid.component.spec.ts

@@ -119,8 +119,6 @@ const treeDataServiceStub = {
 describe('App Component', () => {
   let fixture: ComponentFixture<AngularSlickgridComponent>;
   let component: AngularSlickgridComponent;
-  let graphqlService: GraphqlService;
-  let translate: TranslateService;
 
   beforeEach(async(() => {
     // @ts-ignore
@@ -180,8 +178,6 @@ describe('App Component', () => {
     // create the component
     fixture = TestBed.createComponent(AngularSlickgridComponent);
     component = fixture.debugElement.componentInstance;
-    graphqlService = TestBed.get(GraphqlService);
-    translate = TestBed.get(TranslateService);
 
     // setup bindable properties
     component.gridId = 'grid1';

src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts

@@ -120,7 +120,7 @@ const slickgridEventPrefix = 'sg';
   ]
 })
 export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnInit {
-  private _dataset: any[];
+  private _dataset: any[] | null;
   private _columnDefinitions: Column[];
   private _eventHandler: SlickEventHandler = new Slick.EventHandler();
   private _angularGridInstances: AngularGridInstance | undefined;
@@ -131,8 +131,8 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
   private _isDatasetInitialized = false;
   private _isPaginationInitialized = false;
   private _isLocalGrid = true;
-  dataView: any;
-  grid: any;
+  dataView: any | null;
+  grid: any | null;
   gridHeightString: string;
   gridWidthString: string;
   groupingDefinition: any = {};
@@ -196,19 +196,19 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
   }
 
   @Input()
-  get datasetHierarchical(): any[] {
+  get datasetHierarchical(): any[] | null {
     return this.sharedService.hierarchicalDataset;
   }
-  set datasetHierarchical(newHierarchicalDataset: any[]) {
+  set datasetHierarchical(newHierarchicalDataset: any[] | null) {
     this.sharedService.hierarchicalDataset = newHierarchicalDataset;
 
-    if (this.filterService && this.filterService.clearFilters) {
+    if (newHierarchicalDataset && this.columnDefinitions && this.filterService && this.filterService.clearFilters) {
       this.filterService.clearFilters();
     }
 
     // when a hierarchical dataset is set afterward, we can reset the flat dataset and call a tree data sort that will overwrite the flat dataset
     setTimeout(() => {
-      if (this.dataView && this.sortService && this.sortService.processTreeDataInitialSort && this.gridOptions && this.gridOptions.enableTreeData) {
+      if (newHierarchicalDataset && this.dataView && this.sortService && this.sortService.processTreeDataInitialSort && this.gridOptions && this.gridOptions.enableTreeData) {
         this.dataView.setItems([], this.gridOptions.datasetIdPropertyName);
         this.sortService.processTreeDataInitialSort();
       }
@@ -254,7 +254,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
   }
 
   ngOnInit(): void {
-    if (this.gridOptions && this.gridOptions.frozenRow >= 0) {
+    if (this.gridOptions && (this.gridOptions.frozenColumn >= 0 || this.gridOptions.frozenRow >= 0)) {
       this.loadJqueryMousewheelDynamically();
     }
 
@@ -285,11 +285,29 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
     if (this._eventHandler && this._eventHandler.unsubscribeAll) {
       this._eventHandler.unsubscribeAll();
     }
-    if (this.dataView && this.dataView.setItems) {
-      this.dataView.setItems([]);
+    if (this.dataView) {
+      if (this.dataView && this.dataView.setItems) {
+        this.dataView.setItems([]);
+      }
+      if (this.dataView.destroy) {
+        this.dataView.destroy();
+      }
     }
     if (this.grid && this.grid.destroy) {
-      this.grid.destroy();
+      this.grid.destroy(shouldEmptyDomElementContainer);
+    }
+
+    if (this.backendServiceApi) {
+      for (const prop of Object.keys(this.backendServiceApi)) {
+        this.backendServiceApi[prop] = null;
+      }
+      this.backendServiceApi = null;
+    }
+    for (const prop of Object.keys(this.columnDefinitions)) {
+      this.columnDefinitions[prop] = null;
+    }
+    for (const prop of Object.keys(this.sharedService)) {
+      this.sharedService[prop] = null;
     }
 
     // we could optionally also empty the content of the grid container DOM element
@@ -299,6 +317,11 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
 
     // also unsubscribe all RxJS subscriptions
     this.subscriptions = unsubscribeAllObservables(this.subscriptions);
+
+    this._dataset = null;
+    this.datasetHierarchical = null;
+    this._columnDefinitions = [];
+    this._angularGridInstances = null;
   }
 
   emptyGridContainerElm() {
@@ -326,11 +349,10 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
       if (backendApi.service instanceof GraphqlService || typeof backendApi.service.getDatasetName === 'function') {
         backendApi.internalPostProcess = (processResult: GraphqlResult | GraphqlPaginatedResult) => {
           const datasetName = (backendApi && backendApi.service && typeof backendApi.service.getDatasetName === 'function') ? backendApi.service.getDatasetName() : '';
-          this._dataset = [];
           if (processResult && processResult.data && processResult.data[datasetName]) {
-            this._dataset = processResult.data[datasetName].hasOwnProperty('nodes') ? (processResult as GraphqlPaginatedResult).data[datasetName].nodes : (processResult as GraphqlResult).data[datasetName];
+            const data = processResult.data[datasetName].hasOwnProperty('nodes') ? (processResult as GraphqlPaginatedResult).data[datasetName].nodes : (processResult as GraphqlResult).data[datasetName];
             const totalCount = processResult.data[datasetName].hasOwnProperty('totalCount') ? (processResult as GraphqlPaginatedResult).data[datasetName].totalCount : (processResult as GraphqlResult).data[datasetName].length;
-            this.refreshGridData(this._dataset, totalCount || 0);
+            this.refreshGridData(data, totalCount || 0);
           }
         };
       }
@@ -786,7 +808,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
     if (!this.customDataView && (this.dataView && this.dataView.beginUpdate && this.dataView.setItems && this.dataView.endUpdate)) {
       this.onDataviewCreated.emit(this.dataView);
       this.dataView.beginUpdate();
-      this.dataView.setItems(this._dataset, this.gridOptions.datasetIdPropertyName);
+      this.dataView.setItems(this._dataset || [], this.gridOptions.datasetIdPropertyName);
       this.dataView.endUpdate();
 
       // if you don't want the items that are not visible (due to being filtered out or being on a different page)
@@ -813,7 +835,8 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
         }
       }
 
-      if (this._dataset.length > 0) {
+      const datasetLn = this.dataView.getLength() || this._dataset && this._dataset.length || 0;
+      if (datasetLn > 0) {
         if (!this._isDatasetInitialized && (this.gridOptions.enableCheckboxSelector || this.gridOptions.enableRowSelection)) {
           this.loadRowSelectionPresetWhenExists();
         }
@@ -1076,7 +1099,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
   private swapInternalEditorToSlickGridFactoryEditor(columnDefinitions: Column[]) {
     return columnDefinitions.map((column: Column | any) => {
       // on every Editor that have a "collectionAsync", resolve the data and assign it to the "collection" property
-      if (column.editor && column.editor.collectionAsync) {
+      if (column && column.editor && column.editor.collectionAsync) {
         this.loadEditorCollectionAsync(column);
       }
       return { ...column, editor: column.editor && column.editor.model, internalColumnEditor: { ...column.editor } };

src/app/modules/angular-slickgrid/editors/__tests__/longTextEditor.spec.ts

@@ -220,7 +220,7 @@ describe('LongTextEditor', () => {
     describe('isValueChanged method', () => {
       it('should return True when previously dispatched keyboard event is a new char "a" and it should also update the text counter accordingly', () => {
         const eventKeyDown = new (window.window as any).KeyboardEvent('keydown', { keyCode: KEY_CHAR_A, bubbles: true, cancelable: true });
-        const eventKeyUp = new (window.window as any).KeyboardEvent('keyup', { keyCode: KEY_CHAR_A, bubbles: true, cancelable: true });
+        const eventInput = new (window.window as any).Event('input', { keyCode: KEY_CHAR_A, bubbles: true, cancelable: true });
         mockColumn.internalColumnEditor.maxLength = 255;
 
         editor = new LongTextEditor(editorArguments);
@@ -230,7 +230,7 @@ describe('LongTextEditor', () => {
         const maxTextLengthElm = document.body.querySelector<HTMLDivElement>('.editor-footer .max-length');
         editor.focus();
         editorElm.dispatchEvent(eventKeyDown);
-        editorElm.dispatchEvent(eventKeyUp);
+        editorElm.dispatchEvent(eventInput);
 
         expect(currentTextLengthElm.textContent).toBe('1');
         expect(maxTextLengthElm.textContent).toBe('255');

src/app/modules/angular-slickgrid/editors/autoCompleteEditor.ts

@@ -31,7 +31,7 @@ export class AutoCompleteEditor implements Editor {
   private _autoCompleteOptions: AutocompleteOption;
   private _currentValue: any;
   private _defaultTextValue: string;
-  private _elementCollection: any[];
+  private _elementCollection: any[] | null;
   private _lastInputEvent: JQuery.Event;
 
   /** The JQuery DOM element */
@@ -73,7 +73,7 @@ export class AutoCompleteEditor implements Editor {
   }
 
   /** Get the Final Collection used in the AutoCompleted Source (this may vary from the "collection" especially when providing a customStructure) */
-  get elementCollection(): any[] {
+  get elementCollection(): any[] | null {
     return this._elementCollection;
   }
 
@@ -143,8 +143,9 @@ export class AutoCompleteEditor implements Editor {
     if (this._$editorElm) {
       this._$editorElm.autocomplete('destroy');
       this._$editorElm.off('keydown.nav').remove();
-      this._$editorElm = null;
     }
+    this._$editorElm = null;
+    this._elementCollection = null;
   }
 
   focus() {

src/app/modules/angular-slickgrid/editors/longTextEditor.ts

@@ -120,7 +120,7 @@ export class LongTextEditor implements Editor {
     this._$wrapper.find('.btn-save').on('click', () => this.save());
     this._$wrapper.find('.btn-cancel').on('click', () => this.cancel());
     this._$textarea.on('keydown', this.handleKeyDown.bind(this));
-    this._$textarea.on('keyup', this.handleKeyUp.bind(this));
+    this._$textarea.on('input', this.handleOnInputChange.bind(this));
 
     this.position(this.args && this.args.position);
     this._$textarea.focus().select();
@@ -146,7 +146,7 @@ export class LongTextEditor implements Editor {
   destroy() {
     if (this._$textarea) {
       this._$textarea.off('keydown');
-      this._$textarea.off('keyup');
+      this._$textarea.off('input');
     }
     if (this._$wrapper) {
       this._$wrapper.find('.btn-save').off('click');
@@ -268,8 +268,8 @@ export class LongTextEditor implements Editor {
     }
   }
 
-  /** On every keyup event, we'll update the current text length counter */
-  private handleKeyUp(event: KeyboardEvent & { target: HTMLTextAreaElement }) {
+  /** On every input change event, we'll update the current text length counter */
+  private handleOnInputChange(event: KeyboardEvent & { target: HTMLTextAreaElement }) {
     const textLength = event.target.value.length;
     this._$currentLengthElm.text(textLength);
   }