Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): clear dataset when disposing and fix few unsubscribed events to avoid leak #156

Merged
merged 18 commits into from
Nov 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 62 additions & 26 deletions examples/webpack-demo-vanilla-bundle/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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));
}
}

Expand All @@ -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);
}
});
}
}
}
24 changes: 24 additions & 0 deletions examples/webpack-demo-vanilla-bundle/src/examples/event.service.ts
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 8 additions & 2 deletions examples/webpack-demo-vanilla-bundle/src/examples/example02.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand All @@ -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);
}

Expand Down
17 changes: 12 additions & 5 deletions examples/webpack-demo-vanilla-bundle/src/examples/example03.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,6 +38,7 @@ interface ReportItem {
}

export class Example3 {
private eventService: EventService;
columnDefinitions: Column<ReportItem>[];
gridOptions: GridOption;
dataset: any[];
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
13 changes: 10 additions & 3 deletions examples/webpack-demo-vanilla-bundle/src/examples/example04.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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[];
Expand All @@ -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() {
Expand Down
16 changes: 11 additions & 5 deletions examples/webpack-demo-vanilla-bundle/src/examples/example07.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
Loading