Skip to content

Commit b44bb1b

Browse files
teodosiahChronosSF
andauthored
fix(select): fix memory leak in IgxSelectionAPIService - 17.0.x (#13928)
* fix(select): fix memory leak in IgxSelectionAPIService * fix(select): fix select component unit tests --------- Co-authored-by: Stamen Stoychev <[email protected]>
1 parent 7e60c13 commit b44bb1b

File tree

8 files changed

+46
-18
lines changed

8 files changed

+46
-18
lines changed

projects/igniteui-angular/src/lib/combo/combo.common.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ export abstract class IgxComboBaseDirective extends DisplayDensityBase implement
10081008
this.destroy$.next();
10091009
this.destroy$.complete();
10101010
this.comboAPI.clear();
1011-
this.selectionService.clear(this.id);
1011+
this.selectionService.delete(this.id);
10121012
}
10131013

10141014
/**

projects/igniteui-angular/src/lib/combo/combo.component.spec.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ describe('igxCombo', () => {
8080
const elementRef = { nativeElement: null };
8181
const mockSelection: {
8282
[key: string]: jasmine.Spy;
83-
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
83+
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
8484
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
85-
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register']);
85+
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register', 'clear']);
8686
const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
8787
const mockInjector = jasmine.createSpyObj('Injector', {
8888
get: mockNgControl
@@ -835,6 +835,14 @@ describe('igxCombo', () => {
835835
expect([...selectionService.get(combo.id)][1]).toBe(subParams.newValue);
836836
sub.unsubscribe();
837837
}));
838+
839+
it('should delete the selection on destroy', () => {
840+
combo = new IgxComboComponent(elementRef, mockCdr, mockSelection as any, mockComboService,
841+
mockIconService, null, null, mockInjector);
842+
combo.ngOnDestroy();
843+
expect(mockComboService.clear).toHaveBeenCalled();
844+
expect(mockSelection.delete).toHaveBeenCalled();
845+
});
838846
});
839847

840848
describe('Combo feature tests: ', () => {

projects/igniteui-angular/src/lib/core/selection.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ export class IgxSelectionAPIService {
4242
this.selection.set(componentID, this.get_empty());
4343
}
4444

45+
/**
46+
* Removes selection for a component.
47+
* @param componentID
48+
*/
49+
public delete(componentID: string) {
50+
this.selection.delete(componentID);
51+
}
52+
4553
/**
4654
* Get current component selection length.
4755
*

projects/igniteui-angular/src/lib/drop-down/drop-down.component.spec.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe('IgxDropDown ', () => {
4444
{ value: 'Item5', index: 5 } as IgxDropDownItemComponent];
4545
const mockSelection: {
4646
[key: string]: jasmine.Spy;
47-
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
47+
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
4848
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
4949
mockSelection.get.and.returnValue(new Set([]));
5050
const mockForOf = jasmine.createSpyObj('IgxForOfDirective', ['totalItemCount']);
@@ -179,6 +179,13 @@ describe('IgxDropDown ', () => {
179179
dropdown.toggle();
180180
expect(dropdown.close).toHaveBeenCalledTimes(1);
181181
});
182+
it('should remove selection when component is destroyed', () => {
183+
const selectionService = new IgxSelectionAPIService();
184+
const selectionDeleteSpy = spyOn(selectionService, 'delete');
185+
dropdown = new IgxDropDownComponent({ nativeElement: null }, mockCdr, selectionService, null);
186+
dropdown.ngOnDestroy();
187+
expect(selectionDeleteSpy).toHaveBeenCalled();
188+
});
182189
});
183190
describe('User interaction tests', () => {
184191
describe('Selection & key navigation', () => {

projects/igniteui-angular/src/lib/drop-down/drop-down.component.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,8 @@ export class IgxDropDownComponent extends IgxDropDownBaseDirective implements ID
411411
public ngOnDestroy() {
412412
this.destroy$.next(true);
413413
this.destroy$.complete();
414-
this.selection.clear(this.id);
415-
this.selection.clear(`${this.id}-active`);
414+
this.selection.delete(this.id);
415+
this.selection.delete(`${this.id}-active`);
416416
}
417417

418418
/** @hidden @internal */

projects/igniteui-angular/src/lib/select/select.component.spec.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2666,7 +2666,7 @@ describe('igxSelect', () => {
26662666
describe('igxSelect ControlValueAccessor Unit', () => {
26672667
let select: IgxSelectComponent;
26682668
it('Should correctly implement interface methods', () => {
2669-
const mockSelection = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'clear', 'first_item']);
2669+
const mockSelection = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'clear', 'delete', 'first_item']);
26702670
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['detectChanges']);
26712671
const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
26722672
const mockInjector = jasmine.createSpyObj('Injector', {
@@ -2707,6 +2707,10 @@ describe('igxSelect ControlValueAccessor Unit', () => {
27072707
spyOnProperty(select, 'collapsed').and.returnValue(true);
27082708
select.onBlur();
27092709
expect(mockNgControl.registerOnTouchedCb).toHaveBeenCalledTimes(2);
2710+
2711+
// destroy
2712+
select.ngOnDestroy();
2713+
expect(mockSelection.delete).toHaveBeenCalled();
27102714
});
27112715

27122716
it('Should correctly handle ngControl validity', () => {

projects/igniteui-angular/src/lib/select/select.component.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -551,15 +551,6 @@ export class IgxSelectComponent extends IgxDropDownComponent implements IgxSelec
551551
}
552552
}
553553

554-
/**
555-
* @hidden @internal
556-
*/
557-
public override ngOnDestroy() {
558-
this.destroy$.next(true);
559-
this.destroy$.complete();
560-
this.selection.clear(this.id);
561-
}
562-
563554
/**
564555
* @hidden @internal
565556
* Prevent input blur - closing the items container on Header/Footer Template click.

projects/igniteui-angular/src/lib/simple-combo/simple-combo.component.spec.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ describe('IgxSimpleCombo', () => {
6767
const elementRef = { nativeElement: null };
6868
const mockSelection: {
6969
[key: string]: jasmine.Spy;
70-
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
70+
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
7171
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
72-
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register']);
72+
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register', 'clear']);
7373
const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
7474
const mockInjector = jasmine.createSpyObj('Injector', {
7575
get: mockNgControl
@@ -435,6 +435,16 @@ describe('IgxSimpleCombo', () => {
435435
combo.handleClear(spyObj);
436436
expect(combo.displayValue).toEqual(item[0]);
437437
});
438+
it('should delete the selection on destroy', () => {
439+
const selectionService = new IgxSelectionAPIService();
440+
const comboClearSpy = spyOn(mockComboService, 'clear');
441+
const selectionDeleteSpy = spyOn(selectionService, 'delete');
442+
combo = new IgxSimpleComboComponent(elementRef, mockCdr, selectionService, mockComboService,
443+
mockIconService, platformUtil, null, null, mockInjector);
444+
combo.ngOnDestroy();
445+
expect(comboClearSpy).toHaveBeenCalled();
446+
expect(selectionDeleteSpy).toHaveBeenCalled();
447+
});
438448
});
439449

440450
describe('Initialization and rendering tests: ', () => {

0 commit comments

Comments
 (0)