Skip to content

Commit 15a1866

Browse files
authored
fix(select): fix memory leak in IgxSelectionAPIService (#13941)
1 parent aff0d12 commit 15a1866

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
@@ -1012,7 +1012,7 @@ export abstract class IgxComboBaseDirective extends DisplayDensityBase implement
10121012
this.destroy$.next();
10131013
this.destroy$.complete();
10141014
this.comboAPI.clear();
1015-
this.selectionService.clear(this.id);
1015+
this.selectionService.delete(this.id);
10161016
}
10171017

10181018
/**

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ describe('igxCombo', () => {
8888
const elementRef = { nativeElement: null };
8989
const mockSelection: {
9090
[key: string]: jasmine.Spy;
91-
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
91+
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
9292
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
93-
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register']);
93+
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register', 'clear']);
9494
const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
9595
const mockInjector = jasmine.createSpyObj('Injector', {
9696
get: mockNgControl
@@ -789,6 +789,13 @@ describe('igxCombo', () => {
789789
expect([...selectionService.get(combo.id)][1]).toBe(subParams.newValue);
790790
sub.unsubscribe();
791791
}));
792+
it('should delete the selection on destroy', () => {
793+
combo = new IgxComboComponent(elementRef, mockCdr, mockSelection as any, mockComboService,
794+
mockIconService, null, null, mockInjector);
795+
combo.ngOnDestroy();
796+
expect(mockComboService.clear).toHaveBeenCalled();
797+
expect(mockSelection.delete).toHaveBeenCalled();
798+
});
792799
});
793800

794801
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
@@ -46,7 +46,7 @@ describe('IgxDropDown ', () => {
4646
{ value: 'Item5', index: 5 } as IgxDropDownItemComponent];
4747
const mockSelection: {
4848
[key: string]: jasmine.Spy;
49-
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
49+
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
5050
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
5151
mockSelection.get.and.returnValue(new Set([]));
5252
const mockForOf = jasmine.createSpyObj('IgxForOfDirective', ['totalItemCount']);
@@ -181,6 +181,13 @@ describe('IgxDropDown ', () => {
181181
dropdown.toggle();
182182
expect(dropdown.close).toHaveBeenCalledTimes(1);
183183
});
184+
it('should remove selection on destroy', () => {
185+
const selectionService = new IgxSelectionAPIService();
186+
const selectionDeleteSpy = spyOn(selectionService, 'delete');
187+
dropdown = new IgxDropDownComponent({ nativeElement: null }, mockCdr, selectionService, null);
188+
dropdown.ngOnDestroy();
189+
expect(selectionDeleteSpy).toHaveBeenCalled();
190+
});
184191
});
185192
describe('User interaction tests', () => {
186193
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
@@ -406,8 +406,8 @@ export class IgxDropDownComponent extends IgxDropDownBaseDirective implements ID
406406
public ngOnDestroy() {
407407
this.destroy$.next(true);
408408
this.destroy$.complete();
409-
this.selection.clear(this.id);
410-
this.selection.clear(`${this.id}-active`);
409+
this.selection.delete(this.id);
410+
this.selection.delete(`${this.id}-active`);
411411
}
412412

413413
/** @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
@@ -2675,7 +2675,7 @@ describe('igxSelect', () => {
26752675
describe('igxSelect ControlValueAccessor Unit', () => {
26762676
let select: IgxSelectComponent;
26772677
it('Should correctly implement interface methods', () => {
2678-
const mockSelection = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'clear', 'first_item']);
2678+
const mockSelection = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'clear', 'delete', 'first_item']);
26792679
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['detectChanges']);
26802680
const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
26812681
const mockInjector = jasmine.createSpyObj('Injector', {
@@ -2716,6 +2716,10 @@ describe('igxSelect ControlValueAccessor Unit', () => {
27162716
spyOnProperty(select, 'collapsed').and.returnValue(true);
27172717
select.onBlur();
27182718
expect(mockNgControl.registerOnTouchedCb).toHaveBeenCalledTimes(2);
2719+
2720+
// destroy
2721+
select.ngOnDestroy();
2722+
expect(mockSelection.delete).toHaveBeenCalled();
27192723
});
27202724

27212725
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
@@ -539,15 +539,6 @@ export class IgxSelectComponent extends IgxDropDownComponent implements IgxSelec
539539
}
540540
}
541541

542-
/**
543-
* @hidden @internal
544-
*/
545-
public override ngOnDestroy() {
546-
this.destroy$.next(true);
547-
this.destroy$.complete();
548-
this.selection.clear(this.id);
549-
}
550-
551542
/**
552543
* @hidden @internal
553544
* 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: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ describe('IgxSimpleCombo', () => {
7272
const elementRef = { nativeElement: null };
7373
const mockSelection: {
7474
[key: string]: jasmine.Spy;
75-
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
75+
} = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
7676
const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
77-
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register']);
77+
const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register', 'clear']);
7878
const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
7979
const mockInjector = jasmine.createSpyObj('Injector', {
8080
get: mockNgControl
@@ -429,6 +429,17 @@ describe('IgxSimpleCombo', () => {
429429
combo.handleClear(spyObj);
430430
expect(combo.value).toEqual(item[0]);
431431
});
432+
433+
it('should delete the selection on destroy', () => {
434+
const selectionService = new IgxSelectionAPIService();
435+
const comboClearSpy = spyOn(mockComboService, 'clear');
436+
const selectionDeleteSpy = spyOn(selectionService, 'delete');
437+
combo = new IgxSimpleComboComponent(elementRef, mockCdr, selectionService, mockComboService,
438+
mockIconService, platformUtil, null, null, mockInjector);
439+
combo.ngOnDestroy();
440+
expect(comboClearSpy).toHaveBeenCalled();
441+
expect(selectionDeleteSpy).toHaveBeenCalled();
442+
});
432443
});
433444

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

0 commit comments

Comments
 (0)