fix(core): Flatpickr is not destroyed properly & leaks detached eleme…

…nts (#154) * fix(core): mem leaks w/orphan DOM elements when disposing * fix(core): Flatpickr is not destroyed properly & leaks detached elements
ghiscoding · Nov 7, 2020 · 9633d4a · 9633d4a
1 parent faba5a6
commit 9633d4a
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 16 deletions.
diff --git a/packages/common/src/editors/dateEditor.ts b/packages/common/src/editors/dateEditor.ts
@@ -20,7 +20,7 @@ import {
   SlickGrid,
   SlickNamespace,
 } from './../interfaces/index';
-import { mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType, setDeepValue, getDescendantProperty } from './../services/utilities';
+import { destroyObjectDomElementProps, getDescendantProperty, mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType, setDeepValue } from './../services/utilities';
 import { TranslaterService } from '../services/translater.service';
 
 // using external non-typed js libraries
@@ -163,19 +163,21 @@ export class DateEditor implements Editor {
 
   destroy() {
     this.hide();
-    this._$input.remove();
+    if (this.flatInstance && typeof this.flatInstance.destroy === 'function') {
+      this.flatInstance.destroy();
+      if (this.flatInstance.element) {
+        setTimeout(() => destroyObjectDomElementProps(this.flatInstance));
+      }
+      this.flatInstance = null;
+    }
     if (this._$editorInputElm?.remove) {
       this._$editorInputElm.remove();
       this._$editorInputElm = null;
     }
-    if (this._$inputWithData && typeof this._$inputWithData.remove === 'function') {
+    if (this._$inputWithData?.remove) {
       this._$inputWithData.remove();
       this._$inputWithData = null;
     }
-    if (this.flatInstance && typeof this.flatInstance.destroy === 'function') {
-      this.flatInstance.destroy();
-      this.flatInstance = null;
-    }
   }
 
   disable(isDisabled = true) {

diff --git a/packages/common/src/filters/compoundDateFilter.ts b/packages/common/src/filters/compoundDateFilter.ts
@@ -17,7 +17,7 @@ import {
 import { FieldType, OperatorString, OperatorType, SearchTerm } from '../enums/index';
 import { Constants } from '../constants';
 import { buildSelectOperatorHtmlString } from './filterUtilities';
-import { getTranslationPrefix, mapFlatpickrDateFormatWithFieldType, mapOperatorToShorthandDesignation } from '../services/utilities';
+import { destroyObjectDomElementProps, getTranslationPrefix, mapFlatpickrDateFormatWithFieldType, mapOperatorToShorthandDesignation } from '../services/utilities';
 import { TranslaterService } from '../services/translater.service';
 
 export class CompoundDateFilter implements Filter {
@@ -121,13 +121,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/packages/common/src/filters/dateRangeFilter.ts b/packages/common/src/filters/dateRangeFilter.ts
@@ -18,7 +18,7 @@ import {
   GridOption,
   SlickGrid,
 } from '../interfaces/index';
-import { mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType } from '../services/utilities';
+import { destroyObjectDomElementProps, mapFlatpickrDateFormatWithFieldType, mapMomentDateFormatWithFieldType } from '../services/utilities';
 import { TranslaterService } from '../services/translater.service';
 
 export class DateRangeFilter implements Filter {
@@ -113,14 +113,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/packages/common/src/services/utilities.ts b/packages/common/src/services/utilities.ts
@@ -341,6 +341,24 @@ 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) {
+  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;
+      }
+    }
+  }
+}
+
 /**
  * Format a number following options passed as arguments (decimals, separator, ...)
  * @param input