From fd0cb5b7b9ea939caeda3e19d4381fc3786edf32 Mon Sep 17 00:00:00 2001 From: Ghislain Beaulac Date: Fri, 6 Nov 2020 10:52:54 -0500 Subject: [PATCH 1/4] fix(core): mem leaks w/orphan DOM elements when disposing, fixes #625 --- src/app/examples/grid-clientside.component.ts | 8 ++------ .../angular-slickgrid-constructor.spec.ts | 2 +- .../components/angular-slickgrid.component.ts | 11 +++++++---- .../editors/autoCompleteEditor.ts | 10 ++++++++-- .../angular-slickgrid/editors/checkboxEditor.ts | 1 + .../angular-slickgrid/editors/dateEditor.ts | 3 +++ .../angular-slickgrid/editors/floatEditor.ts | 5 ++++- .../angular-slickgrid/editors/integerEditor.ts | 5 ++++- .../angular-slickgrid/editors/longTextEditor.ts | 11 ++++++++++- .../angular-slickgrid/editors/selectEditor.ts | 5 ++--- .../angular-slickgrid/editors/sliderEditor.ts | 5 ++++- .../angular-slickgrid/editors/textEditor.ts | 5 ++++- .../extensions/autoTooltipExtension.ts | 1 + .../cellExternalCopyManagerExtension.ts | 1 + .../extensions/cellMenuExtension.ts | 1 + .../extensions/checkboxSelectorExtension.ts | 1 + .../extensions/columnPickerExtension.ts | 1 + .../extensions/contextMenuExtension.ts | 1 + .../extensions/draggableGroupingExtension.ts | 1 + .../extensions/gridMenuExtension.ts | 3 ++- .../groupItemMetaProviderExtension.ts | 1 + .../extensions/headerButtonExtension.ts | 1 + .../extensions/headerMenuExtension.ts | 1 + .../extensions/rowDetailViewExtension.ts | 1 + .../extensions/rowMoveManagerExtension.ts | 1 + .../extensions/rowSelectionExtension.ts | 1 + .../filters/__tests__/selectFilter.spec.ts | 17 +++++++++++++++++ .../filters/autoCompleteFilter.ts | 2 ++ .../filters/compoundDateFilter.ts | 2 ++ .../filters/compoundInputFilter.ts | 2 ++ .../filters/compoundSliderFilter.ts | 4 ++++ .../filters/dateRangeFilter.ts | 2 ++ .../angular-slickgrid/filters/inputFilter.ts | 1 + .../filters/nativeSelectFilter.ts | 1 + .../angular-slickgrid/filters/selectFilter.ts | 7 ++++--- .../angular-slickgrid/filters/sliderFilter.ts | 4 +++- .../filters/sliderRangeFilter.ts | 4 ++++ .../services/extension.service.ts | 2 +- .../lib/multiple-select/multiple-select.js | 9 ++++++++- 39 files changed, 116 insertions(+), 28 deletions(-) diff --git a/src/app/examples/grid-clientside.component.ts b/src/app/examples/grid-clientside.component.ts index 245d9e8c4..038b7ba1d 100644 --- 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 (Wiki docs) @@ -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 index ad79a1ede..16b5fb89b 100644 --- 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 index 89e380e17..5236176b3 100644 --- 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 index 674f15409..16d321c3a 100644 --- 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 index b1dc25054..a0f91a136 100644 --- 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 index 33277bef4..4e39c6fd2 100644 --- 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 index 81320bb49..717dac7b2 100644 --- 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 index 6ecaff817..7bd43a34b 100644 --- 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 index da51870da..e444520ee 100644 --- 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 index 938fcf8e5..355b7efcb 100644 --- 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 index 7013ae7c4..f354be24b 100644 --- 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 index e6c229c55..bcdb7f9d2 100644 --- 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 index 1e446ed0d..bf59d0f0e 100644 --- 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 index 12ecf79bb..2379834f7 100644 --- 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 index b9efdfb87..d1b060961 100644 --- 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 index 819e00276..52909ce57 100644 --- 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 index b15b4f1af..db34f2a16 100644 --- 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 index 9e76cb4ec..aec3b78f0 100644 --- 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 index 59c6fc423..76042f788 100644 --- 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 index a474a659b..d99196817 100644 --- 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 index 92e9f9c3e..89570227c 100644 --- 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 index a8e5c6928..14547a99f 100644 --- 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 index 422bcdefd..9321f75a2 100644 --- 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 index 7368118e9..530f2f616 100644 --- 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 index ef543a672..1cfbd4cc6 100644 --- 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 index 4472d122c..30bbf7630 100644 --- 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 index cb2191e7a..3794f1b89 100644 --- 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('.ms-parent.ms-filter.search-filter.filter-gender button.ms-choice'); + const filterListElm = divContainer.querySelectorAll(`[name=filter-gender].ms-drop ul>li input[type=checkbox]`); + const filterOkElm = divContainer.querySelector(`[name=filter-gender].ms-drop .ms-ok-button`); + filterBtnElm.click(); + filterOkElm.click(); + + const filterFilledElms = divContainer.querySelectorAll('.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 index c98fb8aaf..0bf27e795 100644 --- 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 index eb28e1b83..af3c7df7c 100644 --- 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 index c3792f827..bcda1066f 100644 --- 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 index 02af1b8ec..25a38cff6 100644 --- 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; } } diff --git a/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts b/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts index 95874eec3..564e56f2c 100644 --- a/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts @@ -121,9 +121,11 @@ export class DateRangeFilter 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/inputFilter.ts b/src/app/modules/angular-slickgrid/filters/inputFilter.ts index 03bc4038a..d114cf079 100644 --- a/src/app/modules/angular-slickgrid/filters/inputFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/inputFilter.ts @@ -124,6 +124,7 @@ export class InputFilter implements Filter { destroy() { if (this.$filterElm) { this.$filterElm.off('keyup input change').remove(); + this.$filterElm = null; } } diff --git a/src/app/modules/angular-slickgrid/filters/nativeSelectFilter.ts b/src/app/modules/angular-slickgrid/filters/nativeSelectFilter.ts index 9f7029eeb..57ef028cf 100644 --- a/src/app/modules/angular-slickgrid/filters/nativeSelectFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/nativeSelectFilter.ts @@ -124,6 +124,7 @@ export class NativeSelectFilter implements Filter { destroy() { if (this.$filterElm) { this.$filterElm.off('change').remove(); + this.$filterElm = null; } } diff --git a/src/app/modules/angular-slickgrid/filters/selectFilter.ts b/src/app/modules/angular-slickgrid/filters/selectFilter.ts index 6575dbdc6..ecc4dfcdf 100644 --- a/src/app/modules/angular-slickgrid/filters/selectFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/selectFilter.ts @@ -181,11 +181,12 @@ export class SelectFilter implements Filter { * destroy the filter */ destroy() { - if (this.$filterElm) { - // remove event watcher - this.$filterElm.off().remove(); + if (this.$filterElm && typeof this.$filterElm.multipleSelect === 'function') { + this.$filterElm.multipleSelect('destroy'); + this.$filterElm.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(); + this.$filterElm = null; } // also dispose of all Subscriptions diff --git a/src/app/modules/angular-slickgrid/filters/sliderFilter.ts b/src/app/modules/angular-slickgrid/filters/sliderFilter.ts index d8967c210..3ea10e499 100644 --- a/src/app/modules/angular-slickgrid/filters/sliderFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/sliderFilter.ts @@ -140,7 +140,9 @@ export class SliderFilter implements Filter { */ destroy() { if (this.$filterInputElm) { - this.$filterInputElm.off('change').remove(); + this.$filterInputElm.off('input change').remove(); + this.$filterInputElm = null; + this.$filterElm = null; } } diff --git a/src/app/modules/angular-slickgrid/filters/sliderRangeFilter.ts b/src/app/modules/angular-slickgrid/filters/sliderRangeFilter.ts index 08de77500..d1411428a 100644 --- a/src/app/modules/angular-slickgrid/filters/sliderRangeFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/sliderRangeFilter.ts @@ -120,7 +120,11 @@ export class SliderRangeFilter implements Filter { */ destroy() { if (this.$filterElm) { + this.$filterElm.slider('destroy'); this.$filterElm.off('change').remove(); + this.$filterContainerElm.remove(); + this.$filterElm = null; + this.$filterContainerElm = null; } } diff --git a/src/app/modules/angular-slickgrid/services/extension.service.ts b/src/app/modules/angular-slickgrid/services/extension.service.ts index d919bbff5..899418708 100644 --- a/src/app/modules/angular-slickgrid/services/extension.service.ts +++ b/src/app/modules/angular-slickgrid/services/extension.service.ts @@ -72,7 +72,7 @@ export class ExtensionService { } } } - this._extensionList = {} as ExtensionList; + this._extensionList = null; } /** Get all columns (includes visible and non-visible) */ diff --git a/src/assets/lib/multiple-select/multiple-select.js b/src/assets/lib/multiple-select/multiple-select.js index c1ad1c8df..fc606de15 100644 --- a/src/assets/lib/multiple-select/multiple-select.js +++ b/src/assets/lib/multiple-select/multiple-select.js @@ -939,9 +939,16 @@ }, destroy: function () { - $('body').off('click', handleBodyOnClick.bind(this, this.$el)); + $('body').off('click'); this.$el.before(this.$parent).show(); this.$parent.remove(); + this.$choice = null; + this.$el = null; + this.$selectItems = null; + this.$selectGroups = null; + this.$noResults = null; + this.options = null; + this.$parent = null; }, checkAll: function () { From 06e795e55bc798d9decd3c8e15223bb362942e0b Mon Sep 17 00:00:00 2001 From: Ghislain Beaulac Date: Fri, 6 Nov 2020 17:38:34 -0500 Subject: [PATCH 2/4] fix(core): Flatpickr is not destroyed properly & leaks detached elements --- .../components/angular-slickgrid.component.ts | 2 -- .../angular-slickgrid/editors/dateEditor.ts | 19 +++++++++++-------- .../filters/compoundDateFilter.ts | 14 ++++++++++---- .../filters/dateRangeFilter.ts | 13 ++++++++----- .../services/extension.service.ts | 4 +++- .../angular-slickgrid/services/utilities.ts | 16 ++++++++++++++++ 6 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts b/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts index 5236176b3..240bddcb7 100644 --- a/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts +++ b/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts @@ -286,11 +286,9 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn } 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 diff --git a/src/app/modules/angular-slickgrid/editors/dateEditor.ts b/src/app/modules/angular-slickgrid/editors/dateEditor.ts index 4e39c6fd2..86f1b80f2 100644 --- a/src/app/modules/angular-slickgrid/editors/dateEditor.ts +++ b/src/app/modules/angular-slickgrid/editors/dateEditor.ts @@ -7,7 +7,7 @@ const flatpickr: FlatpickrFn = _flatpickr as any; // patch for rollup const moment = moment_; // patch to fix rollup "moment has no default export" issue, document here https://github.com/rollup/rollup/issues/670 import { Constants } from './../constants'; -import { mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType, setDeepValue, getDescendantProperty } from './../services/utilities'; +import { destroyObjectDomElementProps, getDescendantProperty, mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType, setDeepValue } from './../services/utilities'; import { Column, ColumnEditor, @@ -160,19 +160,22 @@ export class DateEditor implements Editor { destroy() { this.hide(); - this._$input.remove(); - if (this._$editorInputElm && this._$editorInputElm.remove) { + if (this.flatInstance && typeof this.flatInstance.destroy === 'function') { + this.flatInstance.destroy(); + if (this.flatInstance.element) { + destroyObjectDomElementProps(this.flatInstance); + } + this.flatInstance = null; + } + if (this._$editorInputElm) { this._$editorInputElm.remove(); this._$editorInputElm = null; } - if (this._$inputWithData && typeof this._$inputWithData.remove === 'function') { + if (this._$inputWithData) { this._$inputWithData.remove(); this._$inputWithData = null; } - if (this.flatInstance && typeof this.flatInstance.destroy === 'function') { - this.flatInstance.destroy(); - this.flatInstance = null; - } + this._$input.remove(); } focus() { diff --git a/src/app/modules/angular-slickgrid/filters/compoundDateFilter.ts b/src/app/modules/angular-slickgrid/filters/compoundDateFilter.ts index af3c7df7c..c61f41114 100644 --- a/src/app/modules/angular-slickgrid/filters/compoundDateFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/compoundDateFilter.ts @@ -19,7 +19,7 @@ import { SearchTerm, } from './../models/index'; import { buildSelectOperatorHtmlString } from './filterUtilities'; -import { getTranslationPrefix, mapFlatpickrDateFormatWithFieldType, mapOperatorToShorthandDesignation } from '../services/utilities'; +import { destroyObjectDomElementProps, getTranslationPrefix, mapFlatpickrDateFormatWithFieldType, mapOperatorToShorthandDesignation } from '../services/utilities'; // use Flatpickr from import or 'require', whichever works first declare function require(name: string): any; @@ -133,13 +133,19 @@ export class CompoundDateFilter implements Filter { * destroy the filter */ destroy() { + if (this.flatInstance && typeof this.flatInstance.destroy === 'function') { + this.flatInstance.destroy(); + if (this.flatInstance.element) { + destroyObjectDomElementProps(this.flatInstance); + } + this.flatInstance = null; + } 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; + if (this.$selectOperatorElm) { + this.$selectOperatorElm.off('change').remove(); } } diff --git a/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts b/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts index 564e56f2c..8ecb7034d 100644 --- a/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts +++ b/src/app/modules/angular-slickgrid/filters/dateRangeFilter.ts @@ -1,6 +1,6 @@ import { Optional } from '@angular/core'; import { TranslateService } from '@ngx-translate/core'; -import { mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType } from '../services/utilities'; +import { destroyObjectDomElementProps, mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType } from '../services/utilities'; import { Column, ColumnFilter, @@ -119,14 +119,17 @@ export class DateRangeFilter implements Filter { * destroy the filter */ destroy() { - if (this.$filterElm) { - this.$filterElm.off('keyup').remove(); - this.$filterElm = null; - } if (this.flatInstance && typeof this.flatInstance.destroy === 'function') { this.flatInstance.destroy(); + if (this.flatInstance.element) { + destroyObjectDomElementProps(this.flatInstance); + } this.flatInstance = null; } + if (this.$filterElm) { + this.$filterElm.off('keyup').remove(); + this.$filterElm = null; + } } hide() { diff --git a/src/app/modules/angular-slickgrid/services/extension.service.ts b/src/app/modules/angular-slickgrid/services/extension.service.ts index 899418708..ef42ab98c 100644 --- a/src/app/modules/angular-slickgrid/services/extension.service.ts +++ b/src/app/modules/angular-slickgrid/services/extension.service.ts @@ -72,7 +72,9 @@ export class ExtensionService { } } } - this._extensionList = null; + for (const key of Object.keys(this._extensionList)) { + delete this._extensionList[key]; + } } /** Get all columns (includes visible and non-visible) */ diff --git a/src/app/modules/angular-slickgrid/services/utilities.ts b/src/app/modules/angular-slickgrid/services/utilities.ts index 183e996f8..1d6e58cba 100644 --- a/src/app/modules/angular-slickgrid/services/utilities.ts +++ b/src/app/modules/angular-slickgrid/services/utilities.ts @@ -411,6 +411,22 @@ export function decimalFormatted(input: number | string, minDecimal?: number, ma return output; } +/** + * Loop through all properties of an object and nullify any properties that are instanceof HTMLElement, + * if we detect an array then use recursion to go inside it and apply same logic + * @param obj - object containing 1 or more properties with DOM Elements + */ +export function destroyObjectDomElementProps(obj: any) { + for (const key of Object.keys(obj)) { + if (Array.isArray(obj[key])) { + destroyObjectDomElementProps(obj[key]); + } + if (obj[key] instanceof HTMLElement) { + obj[key] = null; + } + } +} + /** * Format a number following options passed as arguments (decimals, separator, ...) * @param input From 9fc846ff3ba3f74433694b73508ae851fad04062 Mon Sep 17 00:00:00 2001 From: Ghislain Beaulac Date: Fri, 6 Nov 2020 19:27:26 -0500 Subject: [PATCH 3/4] refactor: fix date editor issue when clicking a date --- src/app/modules/angular-slickgrid/editors/dateEditor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/modules/angular-slickgrid/editors/dateEditor.ts b/src/app/modules/angular-slickgrid/editors/dateEditor.ts index 86f1b80f2..c4b9febd6 100644 --- a/src/app/modules/angular-slickgrid/editors/dateEditor.ts +++ b/src/app/modules/angular-slickgrid/editors/dateEditor.ts @@ -163,7 +163,7 @@ export class DateEditor implements Editor { if (this.flatInstance && typeof this.flatInstance.destroy === 'function') { this.flatInstance.destroy(); if (this.flatInstance.element) { - destroyObjectDomElementProps(this.flatInstance); + setTimeout(() => destroyObjectDomElementProps(this.flatInstance)); } this.flatInstance = null; } From 7941b4b997b9d8793d9474c5f1a1270a87b0fdf1 Mon Sep 17 00:00:00 2001 From: Ghislain Beaulac Date: Fri, 6 Nov 2020 19:30:13 -0500 Subject: [PATCH 4/4] refactor: make sure argument is object --- .../angular-slickgrid/services/utilities.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/app/modules/angular-slickgrid/services/utilities.ts b/src/app/modules/angular-slickgrid/services/utilities.ts index 1d6e58cba..b89a96a84 100644 --- a/src/app/modules/angular-slickgrid/services/utilities.ts +++ b/src/app/modules/angular-slickgrid/services/utilities.ts @@ -417,12 +417,14 @@ export function decimalFormatted(input: number | string, minDecimal?: number, ma * @param obj - object containing 1 or more properties with DOM Elements */ export function destroyObjectDomElementProps(obj: any) { - for (const key of Object.keys(obj)) { - if (Array.isArray(obj[key])) { - destroyObjectDomElementProps(obj[key]); - } - if (obj[key] instanceof HTMLElement) { - obj[key] = null; + if (obj) { + for (const key of Object.keys(obj)) { + if (Array.isArray(obj[key])) { + destroyObjectDomElementProps(obj[key]); + } + if (obj[key] instanceof HTMLElement) { + obj[key] = null; + } } } }