Skip to content

Commit

Permalink
fix(core): unsubscribe all subscriptions on compose dispose (#446)
Browse files Browse the repository at this point in the history
* fix(core): unsubscribe all subscriptions on compose dispose
  • Loading branch information
ghiscoding committed Oct 27, 2020
1 parent 91d81c8 commit 9b20b61
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 13 deletions.
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
"@types/flatpickr": "^3.1.2",
"@types/i18next": "^13.0.0",
"@types/i18next-xhr-backend": "^1.4.2",
"@types/jest": "^26.0.13",
"@types/jest": "^26.0.15",
"@types/jquery": "^3.5.1",
"@types/moment": "^2.13.0",
"@types/node": "^14.10.1",
Expand Down Expand Up @@ -148,11 +148,11 @@
"i18next-xhr-backend": ">=1.5.0",
"isomorphic-fetch": "^2.2.1",
"istanbul-instrumenter-loader": "^3.0.1",
"jest": "^26.4.2",
"jest-cli": "^26.4.2",
"jest-environment-jsdom": "^26.3.0",
"jest": "^26.6.1",
"jest-cli": "^26.6.1",
"jest-environment-jsdom": "^26.6.1",
"jest-extended": "^0.11.5",
"jest-junit": "^11.1.0",
"jest-junit": "^12.0.0",
"jsdom-global": "^3.0.2",
"json-loader": "0.5.7",
"minimatch": "^3.0.4",
Expand All @@ -167,7 +167,7 @@
"standard-version": "^9.0.0",
"style-loader": "0.18.2",
"through2": "^4.0.2",
"ts-jest": "^26.3.0",
"ts-jest": "^26.4.2",
"ts-loader": "^5.2.1",
"ts-node": "^8.10.2",
"tslint": "^6.1.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class HttpStub extends HttpClient {
}

describe('rowDetailViewExtension', () => {
let ea: EventAggregator;
let pluginEa: EventAggregator;
let extensionUtility: ExtensionUtility;
let extension: RowDetailViewExtension;
let sharedService: SharedService;
Expand Down Expand Up @@ -136,10 +136,10 @@ describe('rowDetailViewExtension', () => {
} as GridOption;

beforeEach(() => {
ea = new EventAggregator();
pluginEa = new EventAggregator();
sharedService = new SharedService();
extensionUtility = new ExtensionUtility({ tr: jest.fn() } as unknown as I18N, sharedService);
extension = new RowDetailViewExtension(aureliaUtilServiceStub, ea, extensionUtility, sharedService);
extension = new RowDetailViewExtension(aureliaUtilServiceStub, pluginEa, extensionUtility, sharedService);
});

it('should return null after calling "create" method when either the column definitions or the grid options is missing', () => {
Expand Down Expand Up @@ -273,6 +273,8 @@ describe('rowDetailViewExtension', () => {
jest.spyOn(SharedService.prototype, 'grid', 'get').mockReturnValue(gridStub);
jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
jest.clearAllMocks();
gridStub.onColumnsReordered = new Slick.Event();
gridStub.onSort = new Slick.Event();
});

it('should register the addon', () => {
Expand Down Expand Up @@ -501,7 +503,7 @@ describe('rowDetailViewExtension', () => {

extension.register();
instance.onBeforeRowDetailToggle.subscribe(() => {
ea.publish('filterService:filterChanged', { columnId: 'field1', operator: '=', searchTerms: [] });
pluginEa.publish('filterService:filterChanged', { columnId: 'field1', operator: '=', searchTerms: [] });
expect(appendSpy).toHaveBeenCalledWith(
undefined,
expect.objectContaining({ model: mockColumn, addon: expect.anything(), grid: gridStub, dataView: dataViewStub }),
Expand Down
2 changes: 1 addition & 1 deletion src/aurelia-slickgrid/extensions/rowDetailViewExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ export class RowDetailViewExtension implements Extension {
if (this._addon && this._addon.destroy) {
this._addon.destroy();
}
disposeAllSubscriptions(this._subscriptions);
this.disposeAllViewSlot();
disposeAllSubscriptions(this._subscriptions);
}

/** Dispose of all the opened Row Detail Panels Aurelia View Slots */
Expand Down
8 changes: 7 additions & 1 deletion src/aurelia-slickgrid/services/groupingAndColspan.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { inject, singleton } from 'aurelia-framework';
import { Subscription } from 'aurelia-event-aggregator';
import * as $ from 'jquery';

import {
Expand All @@ -10,6 +11,7 @@ import {
import { ExtensionUtility } from '../extensions/extensionUtility';
import { ExtensionService } from './extension.service';
import { SlickgridEventAggregator } from '../custom-elements/slickgridEventAggregator';
import { disposeAllSubscriptions } from './utilities';

// using external non-typed js libraries
declare const Slick: any;
Expand All @@ -20,6 +22,7 @@ export class GroupingAndColspanService {
private _eventHandler: SlickEventHandler;
private _grid: any;
private _aureliaEventPrefix: string;
private _subscriptions: Subscription[] = [];

constructor(private extensionUtility: ExtensionUtility, private extensionService: ExtensionService, private pluginEa: SlickgridEventAggregator) {
this._eventHandler = new Slick.EventHandler();
Expand Down Expand Up @@ -57,7 +60,9 @@ export class GroupingAndColspanService {
this._eventHandler.subscribe(grid.onColumnsResized, () => this.renderPreHeaderRowGroupingTitles());
this._eventHandler.subscribe(grid.onColumnsReordered, () => this.renderPreHeaderRowGroupingTitles());
this._eventHandler.subscribe(dataView.onRowCountChanged, () => this.renderPreHeaderRowGroupingTitles());
this.pluginEa.subscribe(`resizerService:onAfterResize`, () => this.renderPreHeaderRowGroupingTitles());
this._subscriptions.push(
this.pluginEa.subscribe(`resizerService:onAfterResize`, () => this.renderPreHeaderRowGroupingTitles())
);

this._eventHandler.subscribe(grid.onSetOptions, (_e: Event, args: { grid: any; optionsBefore: GridOption; optionsAfter: GridOption; }) => {
// when user changes frozen columns dynamically (e.g. from header menu), we need to re-render the pre-header of the grouping titles
Expand Down Expand Up @@ -88,6 +93,7 @@ export class GroupingAndColspanService {
dispose() {
// unsubscribe all SlickGrid events
this._eventHandler.unsubscribeAll();
disposeAllSubscriptions(this._subscriptions);
}

/** Create or Render the Pre-Header Row Grouping Titles */
Expand Down
6 changes: 6 additions & 0 deletions src/examples/slickgrid/example20.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ export class Example20 {
this.getData();
}

detached() {
// unsubscribe every SlickGrid subscribed event (or use the Slick.EventHandler)
this.gridObj.onMouseEnter.unsubscribe();
this.gridObj.onMouseLeave.unsubscribe();
}

/* Define grid Options and Columns */
defineGrid() {
this.columnDefinitions = [
Expand Down
2 changes: 1 addition & 1 deletion test/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module.exports = {
'ts-jest': {
diagnostics: false,
isolatedModules: true,
tsConfig: '<rootDir>/test/tsconfig.spec.json'
tsconfig: '<rootDir>/test/tsconfig.spec.json'
},
},
globalSetup: '<rootDir>/test/jest-global-setup.js',
Expand Down

0 comments on commit 9b20b61

Please sign in to comment.