fix(core): clear dataset when disposing and fix few unsubscribed even…

…ts to avoid leak (#156) * fix(core): do not keep dataset local copy avoid leak when disposing * fix: add unbindAll to BindingService to avoid detached events * fix(filters): any Filter should only trigger search callback once - some Filters have multiple events attached to the elements and in some cases it was calling multiple search filtering execution * fix: make sure to remove every event listeners when disposing
ghiscoding · Nov 12, 2020 · 78c80b4 · 78c80b4
1 parent 073edd8
commit 78c80b4
Show file tree

Hide file tree

Showing 75 changed files with 1,150 additions and 755 deletions.
diff --git a/examples/webpack-demo-vanilla-bundle/src/app.ts b/examples/webpack-demo-vanilla-bundle/src/app.ts
@@ -2,7 +2,14 @@ import { AppRouting } from './app-routing';
 import { Renderer } from './renderer';
 import { RouterConfig } from './interfaces';
 
+interface ElementEventListener {
+  element: Element;
+  eventName: string;
+  listener: EventListenerOrEventListenerObject;
+}
+
 export class App {
+  private _boundedEventWithListeners: ElementEventListener[] = [];
   documentTitle = 'Slickgrid-Universal';
   defaultRouteName: string;
   stateBangChar: string;
@@ -40,17 +47,32 @@ export class App {
     };
   }
 
+  addElementEventListener(element: Element, eventName: string, listener: EventListenerOrEventListenerObject) {
+    element.addEventListener(eventName, listener);
+    this._boundedEventWithListeners.push({ element, eventName, listener });
+  }
+
+  /** Dispose of the SPA App */
+  disposeApp() {
+    document.removeEventListener('DOMContentLoaded', this.handleNavbarHamburgerToggle);
+  }
+
+  /** Dispose of all View Models of the SPA */
   disposeAll() {
-    this.renderer?.dispose();
+    this.unbindAllEvents();
 
     for (const vmKey of Object.keys(this.viewModelObj)) {
       const viewModel = this.viewModelObj[vmKey];
-      if (viewModel && viewModel.dispose) {
+      if (viewModel?.dispose) {
         viewModel?.dispose();
-        delete window[vmKey];
-        delete this.viewModelObj[vmKey];
       }
+      // nullify the object and then delete them to make sure it's picked by the garbage collector
+      window[vmKey] = null;
+      this.viewModelObj[vmKey] = null;
+      delete window[vmKey];
+      delete this.viewModelObj[vmKey];
     }
+    this.renderer?.dispose();
   }
 
   loadRoute(routeName: string, changeBrowserState = true) {
@@ -63,12 +85,15 @@ export class App {
         return;
       }
       const viewModel = this.renderer.loadViewModel(require(`${mapRoute.moduleId}.ts`));
-      if (viewModel && viewModel.dispose) {
-        window.onunload = viewModel.dispose; // dispose when leaving SPA
+      if (viewModel?.dispose) {
+        window.onunload = () => {
+          viewModel.dispose; // dispose when leaving SPA
+          this.disposeApp();
+        };
       }
 
       this.renderer.loadView(require(`${mapRoute.moduleId}.html`));
-      if (viewModel && viewModel.attached && this.renderer.className) {
+      if (viewModel?.attached && this.renderer.className) {
         this.viewModelObj[this.renderer.className] = viewModel;
         viewModel.attached();
         this.dropdownToggle(); // rebind bulma dropdown toggle event handlers
@@ -87,14 +112,14 @@ export class App {
     const $dropdowns = document.querySelectorAll('.dropdown:not(.is-hoverable)');
 
     if ($dropdowns.length > 0) {
-      $dropdowns.forEach(($el) => {
-        $el.addEventListener('click', (event) => {
+      $dropdowns.forEach($el => {
+        this.addElementEventListener($el, 'click', (event) => {
           event.stopPropagation();
           $el.classList.toggle('is-active');
         });
       });
 
-      document.addEventListener('click', () => this.closeDropdowns());
+      this.addElementEventListener(document.body, 'click', this.closeDropdowns.bind(this));
     }
   }
 
@@ -105,27 +130,38 @@ export class App {
 
   /** Add event listener for the navbar hamburger menu toggle when menu shows up on mobile */
   navbarHamburgerToggle() {
-    document.addEventListener('DOMContentLoaded', () => {
+    document.addEventListener('DOMContentLoaded', this.handleNavbarHamburgerToggle);
+  }
 
-      // Get all "navbar-burger" elements
-      const $navbarBurgers = Array.prototype.slice.call(document.querySelectorAll('.navbar-burger'), 0);
+  handleNavbarHamburgerToggle() {
+    // Get all "navbar-burger" elements
+    const $navbarBurgers = Array.prototype.slice.call(document.querySelectorAll('.navbar-burger'), 0);
 
-      // Check if there are any navbar burgers
-      if ($navbarBurgers.length > 0) {
-        // Add a click event on each of them
-        $navbarBurgers.forEach(el => {
-          el.addEventListener('click', () => {
+    // Check if there are any navbar burgers
+    if ($navbarBurgers.length > 0) {
+      // Add a click event on each of them
+      $navbarBurgers.forEach(el => {
+        el.addEventListener('click', () => {
 
-            // Get the target from the "data-target" attribute
-            const target = el.dataset.target;
-            const $target = document.getElementById(target);
+          // Get the target from the "data-target" attribute
+          const target = el.dataset.target;
+          const $target = document.getElementById(target);
 
-            // Toggle the "is-active" class on both the "navbar-burger" and the "navbar-menu"
-            el.classList.toggle('is-active');
-            $target.classList.toggle('is-active');
-          });
+          // Toggle the "is-active" class on both the "navbar-burger" and the "navbar-menu"
+          el.classList.toggle('is-active');
+          $target.classList.toggle('is-active');
         });
+      });
+    }
+  }
+
+  /** Unbind All (remove) bounded elements with listeners */
+  unbindAllEvents() {
+    for (const boundedEvent of this._boundedEventWithListeners) {
+      const { element, eventName, listener } = boundedEvent;
+      if (element?.removeEventListener) {
+        element.removeEventListener(eventName, listener);
       }
-    });
+    }
   }
 }
diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/event.service.ts b/examples/webpack-demo-vanilla-bundle/src/examples/event.service.ts
@@ -0,0 +1,24 @@
+interface ElementEventListener {
+  element: Element;
+  eventName: string;
+  listener: EventListenerOrEventListenerObject;
+}
+
+export class EventService {
+  private _boundedEventWithListeners: ElementEventListener[] = [];
+
+  addElementEventListener(element: Element, eventName: string, listener: EventListenerOrEventListenerObject) {
+    element.addEventListener(eventName, listener);
+    this._boundedEventWithListeners.push({ element, eventName, listener });
+  }
+
+  /** Unbind All (remove) bounded elements with listeners */
+  unbindAllEvents() {
+    for (const boundedEvent of this._boundedEventWithListeners) {
+      const { element, eventName, listener } = boundedEvent;
+      if (element?.removeEventListener) {
+        element.removeEventListener(eventName, listener);
+      }
+    }
+  }
+}
diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example01.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example01.ts
@@ -20,15 +20,13 @@ export class Example1 {
 
   attached() {
     this.defineGrids();
-    const gridContainerElm1 = document.querySelector<HTMLDivElement>(`.grid1`);
-    const gridContainerElm2 = document.querySelector<HTMLDivElement>(`.grid2`);
 
     // mock some data (different in each dataset)
     this.dataset1 = this.mockData(NB_ITEMS);
     this.dataset2 = this.mockData(NB_ITEMS);
 
-    this.sgb1 = new Slicker.GridBundle(gridContainerElm1, this.columnDefinitions1, { ...ExampleGridOptions, ...this.gridOptions1 }, this.dataset1);
-    this.sgb2 = new Slicker.GridBundle(gridContainerElm2, this.columnDefinitions2, { ...ExampleGridOptions, ...this.gridOptions2 }, this.dataset2);
+    this.sgb1 = new Slicker.GridBundle(document.querySelector<HTMLDivElement>(`.grid1`), this.columnDefinitions1, { ...ExampleGridOptions, ...this.gridOptions1 }, this.dataset1);
+    this.sgb2 = new Slicker.GridBundle(document.querySelector<HTMLDivElement>(`.grid2`), this.columnDefinitions2, { ...ExampleGridOptions, ...this.gridOptions2 }, this.dataset2);
 
     // setTimeout(() => {
     //   this.slickgridLwc2.dataset = this.dataset2;

diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example02.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example02.ts
@@ -20,10 +20,12 @@ import { Slicker, SlickerGridInstance, SlickVanillaGridBundle } from '@slickgrid
 import { ExampleGridOptions } from './example-grid-options';
 import '../material-styles.scss';
 import './example02.scss';
+import { EventService } from './event.service';
 
 const NB_ITEMS = 500;
 
 export class Example2 {
+  private eventService: EventService;
   columnDefinitions: Column[];
   gridOptions: GridOption;
   dataset: any[];
@@ -41,13 +43,17 @@ export class Example2 {
     return this.sgb?.instances;
   }
 
+  constructor() {
+    this.eventService = new EventService();
+  }
+
   attached() {
     this.initializeGrid();
     this.dataset = this.loadData(NB_ITEMS);
     const gridContainerElm = document.querySelector<HTMLDivElement>(`.grid2`);
 
-    gridContainerElm.addEventListener('onbeforeexporttoexcel', () => console.log('onBeforeExportToExcel'));
-    gridContainerElm.addEventListener('onafterexporttoexcel', () => console.log('onAfterExportToExcel'));
+    this.eventService.addElementEventListener(gridContainerElm, 'onbeforeexporttoexcel', () => console.log('onBeforeExportToExcel'));
+    this.eventService.addElementEventListener(gridContainerElm, 'onafterexporttoexcel', () => console.log('onAfterExportToExcel'));
     this.sgb = new Slicker.GridBundle(gridContainerElm, this.columnDefinitions, { ...ExampleGridOptions, ...this.gridOptions }, this.dataset);
   }
 

diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example03.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example03.ts
@@ -22,6 +22,7 @@ import { Slicker, SlickerGridInstance, SlickVanillaGridBundle } from '@slickgrid
 import { ExampleGridOptions } from './example-grid-options';
 import '../salesforce-styles.scss';
 import './example03.scss';
+import { EventService } from './event.service';
 
 // using external SlickGrid JS libraries
 declare const Slick: SlickNamespace;
@@ -37,6 +38,7 @@ interface ReportItem {
 }
 
 export class Example3 {
+  private eventService: EventService;
   columnDefinitions: Column<ReportItem>[];
   gridOptions: GridOption;
   dataset: any[];
@@ -49,21 +51,26 @@ export class Example3 {
   draggableGroupingPlugin: SlickDraggableGrouping;
   selectedGroupingFields: Array<string | GroupingGetterFunction> = ['', '', ''];
 
+  constructor() {
+    this.eventService = new EventService();
+  }
+
   attached() {
     this.initializeGrid();
     this.dataset = this.loadData(500);
     const gridContainerElm = document.querySelector<HTMLDivElement>(`.grid3`);
 
-    gridContainerElm.addEventListener('onclick', this.handleOnClick.bind(this));
-    gridContainerElm.addEventListener('oncellchange', this.handleOnCellChange.bind(this));
-    gridContainerElm.addEventListener('onvalidationerror', this.handleValidationError.bind(this));
-    gridContainerElm.addEventListener('onitemdeleted', this.handleItemDeleted.bind(this));
-    gridContainerElm.addEventListener('onslickergridcreated', this.handleOnSlickerGridCreated.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'onclick', this.handleOnClick.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'oncellchange', this.handleOnCellChange.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'onvalidationerror', this.handleValidationError.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'onitemdeleted', this.handleItemDeleted.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'onslickergridcreated', this.handleOnSlickerGridCreated.bind(this));
     this.sgb = new Slicker.GridBundle(gridContainerElm, this.columnDefinitions, { ...ExampleGridOptions, ...this.gridOptions }, this.dataset);
   }
 
   dispose() {
     this.sgb?.dispose();
+    this.eventService.unbindAllEvents();
   }
 
   initializeGrid() {

diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example04.html b/examples/webpack-demo-vanilla-bundle/src/examples/example04.html
@@ -17,7 +17,7 @@ <h3 class="title is-3">Example 04 - Pinned (frozen) Columns/Rows</h3>
   </span>
   <span style="margin-left: 15px">
     <button class="button is-small" onclick.delegate="setFrozenColumns(-1)"
-      data-test="remove-frozen-column-button">
+            data-test="remove-frozen-column-button">
       <span class="icon mdi mdi-close"></span>
       <span>Remove Frozen Columns</span>
     </button>

diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example04.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example04.ts
@@ -13,6 +13,7 @@ import {
 } from '@slickgrid-universal/common';
 import { ExcelExportService } from '@slickgrid-universal/excel-export';
 import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle';
+import { EventService } from './event.service';
 import { ExampleGridOptions } from './example-grid-options';
 import './example02.scss';
 
@@ -41,6 +42,7 @@ const customEditableInputFormatter = (_row: number, _cell: number, _value: any,
 };
 
 export class Example4 {
+  private eventService: EventService;
   columnDefinitions: Column<ReportItem>[];
   gridOptions: GridOption;
   dataset: any[];
@@ -52,18 +54,23 @@ export class Example4 {
   isFrozenBottom = false;
   sgb: SlickVanillaGridBundle;
 
+  constructor() {
+    this.eventService = new EventService();
+  }
+
   attached() {
     const dataset = this.initializeGrid();
     const gridContainerElm = document.querySelector<HTMLDivElement>(`.grid4`);
 
-    // gridContainerElm.addEventListener('onclick', handleOnClick);
-    gridContainerElm.addEventListener('onvalidationerror', this.handleOnValidationError.bind(this));
-    gridContainerElm.addEventListener('onitemdeleted', this.handleOnItemDeleted.bind(this));
+    // this.eventService.addElementEventListener(gridContainerElm, 'onclick', handleOnClick);
+    this.eventService.addElementEventListener(gridContainerElm, 'onvalidationerror', this.handleOnValidationError.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'onitemdeleted', this.handleOnItemDeleted.bind(this));
     this.sgb = new Slicker.GridBundle(gridContainerElm, this.columnDefinitions, { ...ExampleGridOptions, ...this.gridOptions }, dataset);
   }
 
   dispose() {
     this.sgb?.dispose();
+    this.eventService.unbindAllEvents();
   }
 
   initializeGrid() {

diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example07.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example07.ts
@@ -8,22 +8,28 @@ import {
 } from '@slickgrid-universal/common';
 import { ExcelExportService } from '@slickgrid-universal/excel-export';
 import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle';
+import { EventService } from './event.service';
 
 import { ExampleGridOptions } from './example-grid-options';
 
 export class Example7 {
+  private eventService: EventService;
   columnDefinitions: Column[];
   gridOptions: GridOption;
   dataset: any[];
   sgb: SlickVanillaGridBundle;
   duplicateTitleHeaderCount = 1;
 
+  constructor() {
+    this.eventService = new EventService();
+  }
+
   attached() {
     this.initializeGrid();
     this.dataset = this.loadData(500);
     const gridContainerElm = document.querySelector<HTMLDivElement>(`.grid7`);
-    gridContainerElm.addEventListener('oncellchange', this.handleOnCellChange.bind(this));
-    gridContainerElm.addEventListener('onvalidationerror', this.handleValidationError.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'oncellchange', this.handleOnCellChange.bind(this));
+    this.eventService.addElementEventListener(gridContainerElm, 'onvalidationerror', this.handleValidationError.bind(this));
     this.sgb = new Slicker.GridBundle(gridContainerElm, this.columnDefinitions, { ...ExampleGridOptions, ...this.gridOptions }, this.dataset);
   }
 
@@ -107,8 +113,8 @@ export class Example7 {
         singleRowMove: true,
         disableRowSelection: true,
         cancelEditOnDrag: true,
-        onBeforeMoveRows: (e, args) => this.onBeforeMoveRow(e, args),
-        onMoveRows: (e, args) => this.onMoveRows(e, args),
+        onBeforeMoveRows: this.onBeforeMoveRow,
+        onMoveRows: this.onMoveRows.bind(this),
 
         // you can also override the usability of the rows, for example make every 2nd row the only moveable rows,
         // usabilityOverride: (row, dataContext, grid) => dataContext.id % 2 === 1
@@ -178,7 +184,7 @@ export class Example7 {
       selectedRows.push(left.length + i);
     }
 
-    this.sgb.slickGrid.resetActiveCell();
+    args.grid.resetActiveCell();
     this.sgb.dataset = this.dataset; // update dataset and re-render the grid
   }