fix(select): fix memory leak in IgxSelectionAPIService (#13929)

Co-authored-by: Nikolay Alipiev <[email protected]>
IgniteUI · Feb 23, 2024 · 2d6f238 · 2d6f238
1 parent 240c006
commit 2d6f238
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 18 deletions.
diff --git a/projects/igniteui-angular/src/lib/combo/combo.common.ts b/projects/igniteui-angular/src/lib/combo/combo.common.ts
@@ -1025,7 +1025,7 @@ export abstract class IgxComboBaseDirective extends DisplayDensityBase implement
         this.destroy$.next();
         this.destroy$.complete();
         this.comboAPI.clear();
-        this.selectionService.clear(this.id);
+        this.selectionService.delete(this.id);
     }
 
     /**

diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts
@@ -80,9 +80,9 @@ describe('igxCombo', () => {
         const elementRef = { nativeElement: null };
         const mockSelection: {
             [key: string]: jasmine.Spy;
-        } = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
+        } = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
         const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
-        const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register']);
+        const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register', 'clear']);
         const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
         const mockInjector = jasmine.createSpyObj('Injector', {
             get: mockNgControl
@@ -801,6 +801,13 @@ describe('igxCombo', () => {
             expect([...selectionService.get(combo.id)][1]).toBe(subParams.newValue);
             sub.unsubscribe();
         }));
+        it('should delete the selection on destroy', () => {
+            combo = new IgxComboComponent(elementRef, mockCdr, mockSelection as any, mockComboService,
+                mockIconService, null, null, mockInjector);
+            combo.ngOnDestroy();
+            expect(mockComboService.clear).toHaveBeenCalled();
+            expect(mockSelection.delete).toHaveBeenCalled();
+        });
     });
 
     describe('Combo feature tests: ', () => {

diff --git a/projects/igniteui-angular/src/lib/core/selection.ts b/projects/igniteui-angular/src/lib/core/selection.ts
@@ -42,6 +42,14 @@ export class IgxSelectionAPIService {
         this.selection.set(componentID, this.get_empty());
     }
 
+    /**
+     * Removes selection for a component.
+     * @param componentID
+     */
+    public delete(componentID: string) {
+        this.selection.delete(componentID);
+    }
+
     /**
      * Get current component selection length.
      *

diff --git a/projects/igniteui-angular/src/lib/drop-down/drop-down.component.spec.ts b/projects/igniteui-angular/src/lib/drop-down/drop-down.component.spec.ts
@@ -44,7 +44,7 @@ describe('IgxDropDown ', () => {
             { value: 'Item5', index: 5 } as IgxDropDownItemComponent];
         const mockSelection: {
             [key: string]: jasmine.Spy;
-        } = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
+        } = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
         const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
         mockSelection.get.and.returnValue(new Set([]));
         const mockForOf = jasmine.createSpyObj('IgxForOfDirective', ['totalItemCount']);
@@ -179,6 +179,13 @@ describe('IgxDropDown ', () => {
             dropdown.toggle();
             expect(dropdown.close).toHaveBeenCalledTimes(1);
         });
+        it('should remove selection on destroy', () => {
+            const selectionService = new IgxSelectionAPIService();
+            const selectionDeleteSpy = spyOn(selectionService, 'delete');
+            dropdown = new IgxDropDownComponent({ nativeElement: null }, mockCdr, selectionService, null);
+            dropdown.ngOnDestroy();
+            expect(selectionDeleteSpy).toHaveBeenCalled();
+        });
     });
     describe('User interaction tests', () => {
         describe('Selection & key navigation', () => {

diff --git a/projects/igniteui-angular/src/lib/drop-down/drop-down.component.ts b/projects/igniteui-angular/src/lib/drop-down/drop-down.component.ts
@@ -409,8 +409,8 @@ export class IgxDropDownComponent extends IgxDropDownBaseDirective implements ID
     public ngOnDestroy() {
         this.destroy$.next(true);
         this.destroy$.complete();
-        this.selection.clear(this.id);
-        this.selection.clear(`${this.id}-active`);
+        this.selection.delete(this.id);
+        this.selection.delete(`${this.id}-active`);
     }
 
     /** @hidden @internal */

diff --git a/projects/igniteui-angular/src/lib/select/select.component.spec.ts b/projects/igniteui-angular/src/lib/select/select.component.spec.ts
@@ -2666,7 +2666,7 @@ describe('igxSelect', () => {
 describe('igxSelect ControlValueAccessor Unit', () => {
     let select: IgxSelectComponent;
     it('Should correctly implement interface methods', () => {
-        const mockSelection = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'clear', 'first_item']);
+        const mockSelection = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'clear', 'delete', 'first_item']);
         const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['detectChanges']);
         const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
         const mockInjector = jasmine.createSpyObj('Injector', {
@@ -2707,6 +2707,10 @@ describe('igxSelect ControlValueAccessor Unit', () => {
         spyOnProperty(select, 'collapsed').and.returnValue(true);
         select.onBlur();
         expect(mockNgControl.registerOnTouchedCb).toHaveBeenCalledTimes(2);
+
+        // destroy
+        select.ngOnDestroy();
+        expect(mockSelection.delete).toHaveBeenCalled();
     });
 
     it('Should correctly handle ngControl validity', () => {

diff --git a/projects/igniteui-angular/src/lib/select/select.component.ts b/projects/igniteui-angular/src/lib/select/select.component.ts
@@ -550,15 +550,6 @@ export class IgxSelectComponent extends IgxDropDownComponent implements IgxSelec
         }
     }
 
-    /**
-     * @hidden @internal
-     */
-    public override ngOnDestroy() {
-        this.destroy$.next(true);
-        this.destroy$.complete();
-        this.selection.clear(this.id);
-    }
-
     /**
      * @hidden @internal
      * Prevent input blur - closing the items container on Header/Footer Template click.

diff --git a/projects/igniteui-angular/src/lib/simple-combo/simple-combo.component.spec.ts b/projects/igniteui-angular/src/lib/simple-combo/simple-combo.component.spec.ts
@@ -66,9 +66,9 @@ describe('IgxSimpleCombo', () => {
         const elementRef = { nativeElement: null };
         const mockSelection: {
             [key: string]: jasmine.Spy;
-        } = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items']);
+        } = jasmine.createSpyObj('IgxSelectionAPIService', ['get', 'set', 'add_items', 'select_items', 'delete']);
         const mockCdr = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck', 'detectChanges']);
-        const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register']);
+        const mockComboService = jasmine.createSpyObj('IgxComboAPIService', ['register', 'clear']);
         const mockNgControl = jasmine.createSpyObj('NgControl', ['registerOnChangeCb', 'registerOnTouchedCb']);
         const mockInjector = jasmine.createSpyObj('Injector', {
             get: mockNgControl
@@ -430,6 +430,17 @@ describe('IgxSimpleCombo', () => {
             combo.handleClear(spyObj);
             expect(combo.displayValue).toEqual([item[0]]);
         });
+
+        it('should delete the selection on destroy', () => {
+            const selectionService = new IgxSelectionAPIService();
+            const comboClearSpy = spyOn(mockComboService, 'clear');
+            const selectionDeleteSpy = spyOn(selectionService, 'delete');
+            combo = new IgxSimpleComboComponent(elementRef, mockCdr, selectionService, mockComboService,
+                mockIconService, platformUtil, null, null, mockInjector);
+            combo.ngOnDestroy();
+            expect(comboClearSpy).toHaveBeenCalled();
+            expect(selectionDeleteSpy).toHaveBeenCalled();
+        });
     });
 
     describe('Initialization and rendering tests: ', () => {