fix(core): mem leaks w/orphan DOM elements when disposing, fixes #625 (…

…#626)
ghiscoding · Nov 6, 2020 · d1e284f · d1e284f
1 parent 6153fe4
commit d1e284f
Show file tree

Hide file tree

Showing 39 changed files with 116 additions and 28 deletions.
diff --git a/src/app/examples/grid-clientside.component.ts b/src/app/examples/grid-clientside.component.ts
@@ -1,4 +1,4 @@
-import { Component, OnInit, OnDestroy } from '@angular/core';
+import { Component, OnInit } from '@angular/core';
 import { HttpClient } from '@angular/common/http';
 import { TranslateService } from '@ngx-translate/core';
 import { CustomInputFilter } from './custom-inputFilter';
@@ -25,7 +25,7 @@ const URL_SAMPLE_COLLECTION_DATA = 'assets/data/collection_500_numbers.json';
 @Component({
   templateUrl: './grid-clientside.component.html'
 })
-export class GridClientSideComponent implements OnInit, OnDestroy {
+export class GridClientSideComponent implements OnInit {
   title = 'Example 4: Client Side Sort/Filter';
   subTitle = `
     Sort/Filter on client side only using SlickGrid DataView (<a href="https://github.com/ghiscoding/Angular-Slickgrid/wiki/Sorting" target="_blank">Wiki docs</a>)
@@ -280,8 +280,4 @@ export class GridClientSideComponent implements OnInit, OnDestroy {
       });
     }
   }
-
-  ngOnDestroy() {
-    this.angularGrid.destroy();
-  }
 }
diff --git a/src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid-constructor.spec.ts b/src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid-constructor.spec.ts
@@ -683,7 +683,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
       });
 
       it('should destroy component and its DOM element when requested', () => {
-        const spy = jest.spyOn(component, 'destroyGridContainerElm');
+        const spy = jest.spyOn(component, 'emptyGridContainerElm');
 
         component.ngAfterViewInit();
         component.destroy(true);

diff --git a/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts b/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts
@@ -272,8 +272,6 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
   }
 
   destroy(shouldEmptyDomElementContainer = false) {
-    this.dataView = undefined;
-    this.gridOptions = {};
     this.extensionService.dispose();
     this.filterService.dispose();
     this.gridEventService.dispose();
@@ -286,20 +284,25 @@ 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([]);
+      this.dataView = null;
+    }
     if (this.grid && this.grid.destroy) {
       this.grid.destroy();
+      this.grid = null;
     }
 
     // we could optionally also empty the content of the grid container DOM element
     if (shouldEmptyDomElementContainer) {
-      this.destroyGridContainerElm();
+      this.emptyGridContainerElm();
     }
 
     // also unsubscribe all RxJS subscriptions
     this.subscriptions = unsubscribeAllObservables(this.subscriptions);
   }
 
-  destroyGridContainerElm() {
+  emptyGridContainerElm() {
     const gridContainerId = this.gridOptions && this.gridOptions.gridContainerId || 'grid1';
     $(gridContainerId).empty();
   }

diff --git a/src/app/modules/angular-slickgrid/editors/autoCompleteEditor.ts b/src/app/modules/angular-slickgrid/editors/autoCompleteEditor.ts
@@ -140,11 +140,17 @@ export class AutoCompleteEditor implements Editor {
   }
 
   destroy() {
-    this._$editorElm.off('keydown.nav').remove();
+    if (this._$editorElm) {
+      this._$editorElm.autocomplete('destroy');
+      this._$editorElm.off('keydown.nav').remove();
+      this._$editorElm = null;
+    }
   }
 
   focus() {
-    this._$editorElm.focus().select();
+    if (this._$editorElm) {
+      this._$editorElm.focus().select();
+    }
   }
 
   getValue() {

diff --git a/src/app/modules/angular-slickgrid/editors/checkboxEditor.ts b/src/app/modules/angular-slickgrid/editors/checkboxEditor.ts
@@ -64,6 +64,7 @@ export class CheckboxEditor implements Editor {
 
   destroy(): void {
     this._$input.remove();
+    this._$input = null;
   }
 
   focus(): void {

diff --git a/src/app/modules/angular-slickgrid/editors/dateEditor.ts b/src/app/modules/angular-slickgrid/editors/dateEditor.ts
@@ -163,12 +163,15 @@ export class DateEditor implements Editor {
     this._$input.remove();
     if (this._$editorInputElm && this._$editorInputElm.remove) {
       this._$editorInputElm.remove();
+      this._$editorInputElm = null;
     }
     if (this._$inputWithData && typeof this._$inputWithData.remove === 'function') {
       this._$inputWithData.remove();
+      this._$inputWithData = null;
     }
     if (this.flatInstance && typeof this.flatInstance.destroy === 'function') {
       this.flatInstance.destroy();
+      this.flatInstance = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/floatEditor.ts b/src/app/modules/angular-slickgrid/editors/floatEditor.ts
@@ -77,11 +77,14 @@ export class FloatEditor implements Editor {
   destroy() {
     if (this._$input) {
       this._$input.off('keydown.nav').remove();
+      this._$input = null;
     }
   }
 
   focus() {
-    this._$input.focus();
+    if (this._$input) {
+      this._$input.focus();
+    }
   }
 
   getDecimalPlaces(): number {

diff --git a/src/app/modules/angular-slickgrid/editors/integerEditor.ts b/src/app/modules/angular-slickgrid/editors/integerEditor.ts
@@ -74,10 +74,13 @@ export class IntegerEditor implements Editor {
 
   destroy() {
     this._$input.off('keydown.nav focusout').remove();
+    this._$input = null;
   }
 
   focus() {
-    this._$input.focus();
+    if (this._$input) {
+      this._$input.focus();
+    }
   }
 
   getValue(): string {

diff --git a/src/app/modules/angular-slickgrid/editors/longTextEditor.ts b/src/app/modules/angular-slickgrid/editors/longTextEditor.ts
@@ -144,7 +144,16 @@ export class LongTextEditor implements Editor {
   }
 
   destroy() {
-    this._$wrapper.remove();
+    if (this._$textarea) {
+      this._$textarea.off('keydown');
+      this._$textarea.off('keyup');
+    }
+    if (this._$wrapper) {
+      this._$wrapper.find('.btn-save').off('click');
+      this._$wrapper.find('.btn-cancel').off('click');
+      this._$wrapper.remove();
+    }
+    this._$wrapper = null;
   }
 
   focus() {

diff --git a/src/app/modules/angular-slickgrid/editors/selectEditor.ts b/src/app/modules/angular-slickgrid/editors/selectEditor.ts
@@ -359,13 +359,12 @@ export class SelectEditor implements Editor {
     this._destroying = true;
     if (this.$editorElm && typeof this.$editorElm.multipleSelect === 'function') {
       this.$editorElm.multipleSelect('destroy');
+      this.$editorElm.remove();
       const elementClassName = this.elementName.toString().replace('.', '\\.'); // make sure to escape any dot "." from CSS class to avoid console error
       $(`[name=${elementClassName}].ms-drop`).remove();
-    }
-    if (this.$editorElm && typeof this.$editorElm.remove === 'function') {
       this.$editorElm.remove();
+      this.$editorElm = null;
     }
-    this._subscriptions = unsubscribeAllObservables(this._subscriptions);
   }
 
   loadValue(item: any): void {

diff --git a/src/app/modules/angular-slickgrid/editors/sliderEditor.ts b/src/app/modules/angular-slickgrid/editors/sliderEditor.ts
@@ -99,7 +99,10 @@ export class SliderEditor implements Editor {
   }
 
   destroy() {
-    this._$editorElm.off('input change mouseup').remove();
+    if (this._$editorElm) {
+      this._$editorElm.off('input change mouseup touchend').remove();
+      this._$editorElm = null;
+    }
   }
 
   focus() {

diff --git a/src/app/modules/angular-slickgrid/editors/textEditor.ts b/src/app/modules/angular-slickgrid/editors/textEditor.ts
@@ -74,10 +74,13 @@ export class TextEditor implements Editor {
 
   destroy() {
     this._$input.off('keydown.nav focusout').remove();
+    this._$input = null;
   }
 
   focus() {
-    this._$input.focus();
+    if (this._$input) {
+      this._$input.focus();
+    }
   }
 
   getValue(): string {

diff --git a/src/app/modules/angular-slickgrid/extensions/autoTooltipExtension.ts b/src/app/modules/angular-slickgrid/extensions/autoTooltipExtension.ts
@@ -15,6 +15,7 @@ export class AutoTooltipExtension implements Extension {
   dispose() {
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/cellExternalCopyManagerExtension.ts b/src/app/modules/angular-slickgrid/extensions/cellExternalCopyManagerExtension.ts
@@ -50,6 +50,7 @@ export class CellExternalCopyManagerExtension implements Extension {
     this._eventHandler.unsubscribeAll();
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
     document.removeEventListener('keydown', this.hookUndoShortcutKey.bind(this));
   }

diff --git a/src/app/modules/angular-slickgrid/extensions/cellMenuExtension.ts b/src/app/modules/angular-slickgrid/extensions/cellMenuExtension.ts
@@ -44,6 +44,7 @@ export class CellMenuExtension implements Extension {
 
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/checkboxSelectorExtension.ts b/src/app/modules/angular-slickgrid/extensions/checkboxSelectorExtension.ts
@@ -15,6 +15,7 @@ export class CheckboxSelectorExtension implements Extension {
   dispose() {
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/columnPickerExtension.ts b/src/app/modules/angular-slickgrid/extensions/columnPickerExtension.ts
@@ -24,6 +24,7 @@ export class ColumnPickerExtension implements Extension {
     this._eventHandler.unsubscribeAll();
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/contextMenuExtension.ts b/src/app/modules/angular-slickgrid/extensions/contextMenuExtension.ts
@@ -52,6 +52,7 @@ export class ContextMenuExtension implements Extension {
 
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
     if (this.sharedService.gridOptions && this.sharedService.gridOptions.contextMenu && this.sharedService.gridOptions.contextMenu.commandItems) {
       this.sharedService.gridOptions.contextMenu = this._userOriginalContextMenu;

diff --git a/src/app/modules/angular-slickgrid/extensions/draggableGroupingExtension.ts b/src/app/modules/angular-slickgrid/extensions/draggableGroupingExtension.ts
@@ -26,6 +26,7 @@ export class DraggableGroupingExtension implements Extension {
 
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/gridMenuExtension.ts b/src/app/modules/angular-slickgrid/extensions/gridMenuExtension.ts
@@ -56,6 +56,7 @@ export class GridMenuExtension implements Extension {
     this._eventHandler.unsubscribeAll();
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
     if (this.sharedService.gridOptions && this.sharedService.gridOptions.gridMenu && this.sharedService.gridOptions.gridMenu.customItems) {
       this.sharedService.gridOptions.gridMenu = this._userOriginalGridMenu;
@@ -132,7 +133,7 @@ export class GridMenuExtension implements Extension {
           if (this.sharedService.grid && typeof this.sharedService.grid.autosizeColumns === 'function') {
             // make sure that the grid still exist (by looking if the Grid UID is found in the DOM tree)
             const gridUid = this.sharedService.grid.getUID();
-            if (this._areVisibleColumnDifferent && gridUid && $(`.${gridUid}`).length > 0) {
+            if (this._areVisibleColumnDifferent && gridUid && document.querySelector(`.${gridUid}`) !== null) {
               if (this.sharedService.gridOptions && this.sharedService.gridOptions.enableAutoSizeColumns) {
                 this.sharedService.grid.autosizeColumns();
               }

diff --git a/src/app/modules/angular-slickgrid/extensions/groupItemMetaProviderExtension.ts b/src/app/modules/angular-slickgrid/extensions/groupItemMetaProviderExtension.ts
@@ -11,6 +11,7 @@ export class GroupItemMetaProviderExtension implements Extension {
   dispose() {
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/headerButtonExtension.ts b/src/app/modules/angular-slickgrid/extensions/headerButtonExtension.ts
@@ -25,6 +25,7 @@ export class HeaderButtonExtension implements Extension {
 
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/headerMenuExtension.ts b/src/app/modules/angular-slickgrid/extensions/headerMenuExtension.ts
@@ -49,6 +49,7 @@ export class HeaderMenuExtension implements Extension {
     this._eventHandler.unsubscribeAll();
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/rowDetailViewExtension.ts b/src/app/modules/angular-slickgrid/extensions/rowDetailViewExtension.ts
@@ -66,6 +66,7 @@ export class RowDetailViewExtension implements Extension {
 
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
 
     // also unsubscribe all RxJS subscriptions

diff --git a/src/app/modules/angular-slickgrid/extensions/rowMoveManagerExtension.ts b/src/app/modules/angular-slickgrid/extensions/rowMoveManagerExtension.ts
@@ -25,6 +25,7 @@ export class RowMoveManagerExtension implements Extension {
 
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/extensions/rowSelectionExtension.ts b/src/app/modules/angular-slickgrid/extensions/rowSelectionExtension.ts
@@ -15,6 +15,7 @@ export class RowSelectionExtension implements Extension {
   dispose() {
     if (this._addon && this._addon.destroy) {
       this._addon.destroy();
+      this._addon = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/filters/__tests__/selectFilter.spec.ts b/src/app/modules/angular-slickgrid/filters/__tests__/selectFilter.spec.ts
@@ -188,6 +188,23 @@ describe('SelectFilter', () => {
     expect(spyCallback).toHaveBeenCalledWith(undefined, { columnDef: mockColumn, operator: 'IN', searchTerms: ['male'], shouldTriggerQuery: true });
   });
 
+  it('should trigger multiple select change event without choosing an option and expect the callback to be called without search terms and also expect the dropdown list to not have "filled" css class', () => {
+    const spyCallback = jest.spyOn(filterArguments, 'callback');
+    mockColumn.filter.collection = [{ value: 'male', label: 'male' }, { value: 'female', label: 'female' }];
+
+    filter.init(filterArguments);
+    const filterBtnElm = divContainer.querySelector<HTMLButtonElement>('.ms-parent.ms-filter.search-filter.filter-gender button.ms-choice');
+    const filterListElm = divContainer.querySelectorAll<HTMLInputElement>(`[name=filter-gender].ms-drop ul>li input[type=checkbox]`);
+    const filterOkElm = divContainer.querySelector<HTMLButtonElement>(`[name=filter-gender].ms-drop .ms-ok-button`);
+    filterBtnElm.click();
+    filterOkElm.click();
+
+    const filterFilledElms = divContainer.querySelectorAll<HTMLDivElement>('.ms-parent.ms-filter.search-filter.filter-gender.filled');
+    expect(filterListElm.length).toBe(2);
+    expect(filterFilledElms.length).toBe(0);
+    expect(spyCallback).toHaveBeenCalledWith(undefined, { columnDef: mockColumn, operator: 'IN', searchTerms: [], shouldTriggerQuery: true });
+  });
+
   it('should trigger multiple select change event and expect this to work with a regular array of strings', () => {
     const spyCallback = jest.spyOn(filterArguments, 'callback');
 

diff --git a/src/app/modules/angular-slickgrid/filters/autoCompleteFilter.ts b/src/app/modules/angular-slickgrid/filters/autoCompleteFilter.ts
@@ -192,7 +192,9 @@ export class AutoCompleteFilter implements Filter {
    */
   destroy() {
     if (this.$filterElm) {
+      this.$filterElm.autocomplete('destroy');
       this.$filterElm.off('keyup').remove();
+      this.$filterElm = null;
     }
     // also unsubscribe all RxJS subscriptions
     this.subscriptions = unsubscribeAllObservables(this.subscriptions);

diff --git a/src/app/modules/angular-slickgrid/filters/compoundDateFilter.ts b/src/app/modules/angular-slickgrid/filters/compoundDateFilter.ts
@@ -135,9 +135,11 @@ export class CompoundDateFilter implements Filter {
   destroy() {
     if (this.$filterElm) {
       this.$filterElm.off('keyup').remove();
+      this.$filterElm = null;
     }
     if (this.flatInstance && typeof this.flatInstance.destroy === 'function') {
       this.flatInstance.destroy();
+      this.flatInstance = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/filters/compoundInputFilter.ts b/src/app/modules/angular-slickgrid/filters/compoundInputFilter.ts
@@ -128,6 +128,8 @@ export class CompoundInputFilter implements Filter {
     if (this.$filterElm && this.$selectOperatorElm) {
       this.$filterElm.off('keyup input change').remove();
       this.$selectOperatorElm.off('change');
+      this.$filterElm = null;
+      this.$selectOperatorElm = null;
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/filters/compoundSliderFilter.ts b/src/app/modules/angular-slickgrid/filters/compoundSliderFilter.ts
@@ -156,6 +156,10 @@ export class CompoundSliderFilter implements Filter {
   destroy() {
     if (this.$filterInputElm) {
       this.$filterInputElm.off('input change').remove();
+      this.$selectOperatorElm.off('change').remove();
+      this.$filterInputElm = null;
+      this.$filterElm = null;
+      this.$selectOperatorElm = null;
     }
   }