fix(memory): clear & dispose of grid to avoid mem leaks & detached elm

- note that Flatpickr still has some mem/event leaks but a fix was put in place, we are just waiting for a new version release from their side to fix that leak
ghiscoding · Jan 17, 2022 · 7035db5 · 7035db5
1 parent 5863115
commit 7035db5
Show file tree

Hide file tree

Showing 22 changed files with 108 additions and 20 deletions.
diff --git a/examples/webpack-demo-vanilla-bundle/src/app.ts b/examples/webpack-demo-vanilla-bundle/src/app.ts
@@ -81,6 +81,11 @@ export class App {
       const viewModel = this.viewModelObj[vmKey];
       if (viewModel?.dispose) {
         viewModel?.dispose();
+
+        // also clear all of its variable references to avoid detached elements
+        for (const ref of Object.keys(viewModel)) {
+          viewModel[ref] = null;
+        }
       }
       // nullify the object and then delete them to make sure it's picked by the garbage collector
       window[vmKey] = null;

diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example18.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example18.ts
@@ -292,7 +292,7 @@ export class Example34 {
 
   /** remove change highlight css class from that cell */
   removeUnsavedStylingFromCell(_item: any, column: Column, row: number) {
-    this.sgb.slickGrid.removeCellCssStyles(`highlight_${[column.id]}${row}`);
+    this.sgb?.slickGrid?.removeCellCssStyles(`highlight_${[column.id]}${row}`);
   }
 
   toggleFullScreen() {

diff --git a/packages/common/src/extensions/menuBaseClass.ts b/packages/common/src/extensions/menuBaseClass.ts
@@ -18,7 +18,7 @@ import { BindingEventService } from '../services/bindingEvent.service';
 import { ExtensionUtility } from '../extensions/extensionUtility';
 import { PubSubService } from '../services/pubSub.service';
 import { SharedService } from '../services/shared.service';
-import { createDomElement } from '../services/domUtilities';
+import { createDomElement, emptyElement } from '../services/domUtilities';
 import { hasData } from '../services/utilities';
 
 // using external SlickGrid JS libraries
@@ -95,6 +95,8 @@ export class MenuBaseClass<M extends CellMenu | ContextMenu | GridMenu | HeaderM
     this._commandTitleElm?.remove();
     this._optionTitleElm?.remove();
     this.menuElement?.remove();
+    emptyElement(this._menuElm);
+    this._menuElm?.remove();
   }
 
   setOptions(newOptions: M) {

diff --git a/packages/common/src/extensions/slickDraggableGrouping.ts b/packages/common/src/extensions/slickDraggableGrouping.ts
@@ -231,6 +231,7 @@ export class SlickDraggableGrouping {
     this._eventHandler.unsubscribeAll();
     this.pubSubService.unsubscribeAll(this._subscriptions);
     emptyElement(document.querySelector('.slick-preheader-panel'));
+    this._grid = undefined as any;
   }
 
   clearDroppedGroups() {

diff --git a/packages/common/src/filters/compoundDateFilter.ts b/packages/common/src/filters/compoundDateFilter.ts
@@ -141,8 +141,13 @@ export class CompoundDateFilter implements Filter {
         destroyObjectDomElementProps(this.flatInstance);
       }
     }
-    this._filterElm?.remove?.();
-    this._selectOperatorElm?.remove?.();
+    emptyElement(this.filterContainerElm);
+    emptyElement(this._filterDivInputElm);
+    this._filterDivInputElm?.remove();
+    this.filterContainerElm?.remove();
+    this._selectOperatorElm?.remove();
+    this._filterElm?.remove();
+    this.grid = null as any;
   }
 
   hide() {

diff --git a/packages/common/src/filters/dateRangeFilter.ts b/packages/common/src/filters/dateRangeFilter.ts
@@ -134,7 +134,12 @@ export class DateRangeFilter implements Filter {
         destroyObjectDomElementProps(this.flatInstance);
       }
     }
-    this._filterElm?.remove?.();
+    emptyElement(this.filterContainerElm);
+    emptyElement(this._filterDivInputElm);
+    this._filterDivInputElm?.remove();
+    this.filterContainerElm?.remove();
+    this._filterElm?.remove();
+    this.grid = null as any;
   }
 
   hide() {

diff --git a/packages/common/src/services/__tests__/bindingEvent.service.spec.ts b/packages/common/src/services/__tests__/bindingEvent.service.spec.ts
@@ -13,6 +13,7 @@ describe('BindingEvent Service', () => {
   afterEach(() => {
     div.remove();
     service.unbindAll();
+    service?.dispose();
     jest.clearAllMocks();
   });
 

diff --git a/packages/common/src/services/bindingEvent.service.ts b/packages/common/src/services/bindingEvent.service.ts
@@ -7,6 +7,11 @@ export class BindingEventService {
     return this._boundedEvents;
   }
 
+  dispose() {
+    this.unbindAll();
+    this._boundedEvents = [];
+  }
+
   /** Bind an event listener to any element */
   bind(elementOrElements: Element | NodeListOf<Element>, eventNameOrNames: string | string[], listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions) {
     const eventNames = (Array.isArray(eventNameOrNames)) ? eventNameOrNames : [eventNameOrNames];

diff --git a/packages/common/src/services/extension.service.ts b/packages/common/src/services/extension.service.ts
@@ -79,6 +79,19 @@ export class ExtensionService {
     for (const key of Object.keys(this._extensionList)) {
       delete this._extensionList[key as keyof Record<ExtensionName, ExtensionModel<any>>];
     }
+    this._cellMenuPlugin = null as any;
+    this._cellExcelCopyManagerPlugin = null as any;
+    this._checkboxSelectColumn = null as any;
+    this._contextMenuPlugin = null as any;
+    this._columnPickerControl = null as any;
+    this._draggleGroupingPlugin = null as any;
+    this._gridMenuControl = null as any;
+    this._groupItemMetadataProviderService = null as any;
+    this._headerMenuPlugin = null as any;
+    this._rowMoveManagerPlugin = null as any;
+    this._rowSelectionModel = null as any;
+    this._extensionCreatedList = null as any;
+    this._extensionList = null as any;
   }
 
   /**

diff --git a/packages/common/src/services/filter.service.ts b/packages/common/src/services/filter.service.ts
@@ -128,6 +128,7 @@ export class FilterService {
     }
     this.disposeColumnFilters();
     this._onSearchChange = null;
+    this._grid = null as any;
   }
 
   /**
@@ -138,11 +139,11 @@ export class FilterService {
 
     // also destroy each Filter instances
     if (Array.isArray(this._filtersMetadata)) {
-      this._filtersMetadata.forEach(filter => {
-        if (filter?.destroy) {
-          filter.destroy();
-        }
-      });
+      let filter = this._filtersMetadata.pop();
+      while (filter) {
+        filter?.destroy();
+        filter = this._filtersMetadata.pop();
+      }
     }
   }
 

diff --git a/packages/composite-editor-component/src/compositeEditor.factory.spec.ts b/packages/composite-editor-component/src/compositeEditor.factory.spec.ts
@@ -92,7 +92,6 @@ const container1 = document.createElement('div');
 const container2 = document.createElement('div');
 const container3 = document.createElement('div');
 const container4 = document.createElement('div');
-const containers = [container1, container2, container3, container4];
 
 describe('Composite Editor Factory', () => {
   let factory: any;
@@ -102,6 +101,7 @@ describe('Composite Editor Factory', () => {
   let editors;
   let compositeOptions;
   let textEditorArgs;
+  let containers;
 
   beforeEach(() => {
     cancelChangeMock = jest.fn();
@@ -126,6 +126,7 @@ describe('Composite Editor Factory', () => {
     editors = columnsMock.map(col => col.editor);
     compositeOptions = { destroy: destroyMock, modalType: 'create', validationMsgPrefix: '* ', formValues: {}, editors };
 
+    containers = [container1, container2, container3, container4];
     factory = new (CompositeEditor as any)(columnsMock, containers, compositeOptions);
   });
 

diff --git a/packages/composite-editor-component/src/compositeEditor.factory.ts b/packages/composite-editor-component/src/compositeEditor.factory.ts
@@ -5,6 +5,7 @@ import {
   EditorArguments,
   EditorValidationResult,
   ElementPosition,
+  emptyElement,
   getHtmlElementOffset,
   HtmlElementPosition,
   SlickNamespace
@@ -64,8 +65,8 @@ export function CompositeEditor(this: any, columns: Column[], containers: Array<
   const getContainerBox = (i: number): ElementPosition => {
     const container = containers[i];
     const offset = getHtmlElementOffset(container);
-    const width = container.clientWidth || 0;
-    const height = container.clientHeight || 0;
+    const width = container?.clientWidth ?? 0;
+    const height = container?.clientHeight ?? 0;
 
     return {
       top: offset?.top ?? 0,
@@ -119,14 +120,22 @@ export function CompositeEditor(this: any, columns: Column[], containers: Array<
     };
 
     context.destroy = () => {
-      let idx = 0;
-      while (idx < editors.length) {
-        editors[idx].destroy();
-        idx++;
+      let tmpEditor = editors.pop();
+      while (tmpEditor) {
+        tmpEditor?.destroy();
+        tmpEditor = editors.pop();
+      }
+
+      let tmpContainer = containers.pop();
+      while (tmpContainer) {
+        emptyElement(tmpContainer);
+        tmpContainer?.remove();
+        tmpContainer = containers.pop();
       }
 
       options?.destroy?.();
       editors = [];
+      containers = null as any;
     };
 
     context.focus = () => {

diff --git a/packages/composite-editor-component/src/slick-composite-editor.component.ts b/packages/composite-editor-component/src/slick-composite-editor.component.ts
@@ -140,12 +140,17 @@ export class SlickCompositeEditorComponent implements ExternalResource {
 
   /** Dispose of the Component without unsubscribing any events */
   disposeComponent() {
+    // protected _editorContainers!: Array<HTMLElement | null>;
+    this._modalBodyTopValidationElm?.remove();
+    this._modalSaveButtonElm?.remove();
+
     if (typeof this._modalElm?.remove === 'function') {
       this._modalElm.remove();
 
       // remove the body backdrop click listener, every other listeners will be dropped automatically since we destroy the component
       document.body.classList.remove('slick-modal-open');
     }
+    this._editorContainers = [];
   }
 
   /**

diff --git a/packages/custom-footer-component/src/slick-footer.component.ts b/packages/custom-footer-component/src/slick-footer.component.ts
@@ -95,11 +95,11 @@ export class SlickFooterComponent {
 
   dispose() {
     // also dispose of all Subscriptions
+    this._eventHandler.unsubscribeAll();
     this.pubSubService.unsubscribeAll(this._subscriptions);
 
     this._bindingHelper.dispose();
     this._footerElement?.remove();
-    this._eventHandler.unsubscribeAll();
   }
 
   /**

diff --git a/packages/event-pub-sub/src/eventPubSub.service.spec.ts b/packages/event-pub-sub/src/eventPubSub.service.spec.ts
@@ -13,6 +13,7 @@ describe('EventPubSub Service', () => {
 
   afterEach(() => {
     service.unsubscribeAll();
+    service?.dispose();
   });
 
   it('should create the service', () => {

diff --git a/packages/event-pub-sub/src/eventPubSub.service.ts b/packages/event-pub-sub/src/eventPubSub.service.ts
@@ -8,6 +8,7 @@ export interface PubSubEvent<T = any> {
 export class EventPubSubService implements PubSubService {
   protected _elementSource: Element;
   protected _subscribedEvents: PubSubEvent[] = [];
+  protected _timer: any;
 
   eventNamingStyle = EventNamingStyle.camelCase;
 
@@ -32,6 +33,14 @@ export class EventPubSubService implements PubSubService {
     this._elementSource = elementSource || document.createElement('div');
   }
 
+  dispose() {
+    this.unsubscribeAll();
+    this._subscribedEvents = [];
+    clearTimeout(this._timer);
+    this._elementSource?.remove();
+    this._elementSource = null as any;
+  }
+
   /**
    * Dispatch of Custom Event, which by default will bubble up & is cancelable
    * @param {String} eventName - event name to dispatch
@@ -45,7 +54,7 @@ export class EventPubSubService implements PubSubService {
     if (data) {
       eventInit.detail = data;
     }
-    return this._elementSource.dispatchEvent(new CustomEvent<T>(eventName, eventInit));
+    return this._elementSource?.dispatchEvent(new CustomEvent<T>(eventName, eventInit));
   }
 
   /**
@@ -90,7 +99,7 @@ export class EventPubSubService implements PubSubService {
 
     if (delay) {
       return new Promise(resolve => {
-        setTimeout(() => resolve(this.dispatchCustomEvent<T>(eventNameByConvention, data, true, true)), delay);
+        this._timer = setTimeout(() => resolve(this.dispatchCustomEvent<T>(eventNameByConvention, data, true, true)), delay);
       });
     } else {
       return this.dispatchCustomEvent<T>(eventNameByConvention, data, true, true);

diff --git a/packages/excel-export/src/excelExport.service.spec.ts b/packages/excel-export/src/excelExport.service.spec.ts
@@ -102,6 +102,7 @@ describe('ExcelExportService', () => {
 
     afterEach(() => {
       delete mockGridOptions.backendServiceApi;
+      service?.dispose();
       jest.clearAllMocks();
     });
 

diff --git a/packages/excel-export/src/excelExport.service.ts b/packages/excel-export/src/excelExport.service.ts
@@ -78,6 +78,10 @@ export class ExcelExportService implements ExternalResource, BaseExcelExportServ
     return (this._grid && this._grid.getOptions) ? this._grid.getOptions() : {};
   }
 
+  dispose() {
+    this._pubSubService?.unsubscribeAll();
+  }
+
   /**
    * Initialize the Export Service
    * @param grid

diff --git a/packages/text-export/src/textExport.service.spec.ts b/packages/text-export/src/textExport.service.spec.ts
@@ -107,6 +107,7 @@ describe('ExportService', () => {
 
     afterEach(() => {
       delete mockGridOptions.backendServiceApi;
+      service?.dispose();
       jest.clearAllMocks();
     });
 

diff --git a/packages/text-export/src/textExport.service.ts b/packages/text-export/src/textExport.service.ts
@@ -75,6 +75,10 @@ export class TextExportService implements ExternalResource, BaseTextExportServic
     return (this._grid?.getOptions) ? this._grid.getOptions() : {};
   }
 
+  dispose() {
+    this._pubSubService?.unsubscribeAll();
+  }
+
   /**
    * Initialize the Service
    * @param grid

diff --git a/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts b/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts
@@ -1941,6 +1941,9 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
     });
 
     describe('Tree Data View', () => {
+      beforeEach(() => {
+        component.eventPubSubService = new EventPubSubService(divContainer);
+      });
       afterEach(() => {
         component.dispose();
         jest.clearAllMocks();
@@ -2021,6 +2024,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
         component.dispose();
         component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file', initialSort: { columndId: 'file', direction: 'ASC' } } } as unknown as GridOption;
         component.datasetHierarchical = mockHierarchical;
+        component.eventPubSubService = new EventPubSubService(divContainer);
         component.initialization(divContainer, slickEventHandler);
 
         expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);

diff --git a/packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts b/packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts
@@ -56,6 +56,7 @@ import {
 
   // utilities
   emptyElement,
+  unsubscribeAll,
 } from '@slickgrid-universal/common';
 import { EventPubSubService } from '@slickgrid-universal/event-pub-sub';
 import { SlickEmptyWarningComponent } from '@slickgrid-universal/empty-warning-component';
@@ -203,6 +204,10 @@ export class SlickVanillaGridBundle {
     this._isDatasetHierarchicalInitialized = true;
   }
 
+  set eventPubSubService(pubSub: EventPubSubService) {
+    this._eventPubSubService = pubSub;
+  }
+
   get gridOptions(): GridOption {
     return this._gridOptions || {};
   }
@@ -425,6 +430,7 @@ export class SlickVanillaGridBundle {
     this.slickEmptyWarning?.dispose();
     this.slickPagination?.dispose();
 
+    unsubscribeAll(this.subscriptions);
     this._eventPubSubService?.unsubscribeAll();
     this.dataView?.setItems([]);
     if (this.dataView?.destroy) {
@@ -435,6 +441,8 @@ export class SlickVanillaGridBundle {
 
     emptyElement(this._gridContainerElm);
     emptyElement(this._gridParentContainerElm);
+    this._gridContainerElm?.remove();
+    this._gridParentContainerElm?.remove();
 
     if (this.backendServiceApi) {
       for (const prop of Object.keys(this.backendServiceApi)) {
@@ -455,6 +463,9 @@ export class SlickVanillaGridBundle {
     if (shouldEmptyDomElementContainer) {
       this.emptyGridContainerElm();
     }
+    this._eventPubSubService?.dispose();
+    this._slickerGridInstances = null as any;
+    delete (window as any).Slicker;
   }
 
   initialization(gridContainerElm: HTMLElement, eventHandler: SlickEventHandler) {