fix(filters): remove Filters from DOM after header row gets destroyed

ghiscoding · Aug 29, 2021 · b08d4ba · b08d4ba
1 parent b0d9908
commit b08d4ba
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 0 deletions.
diff --git a/packages/common/src/services/__tests__/filter.service.spec.ts b/packages/common/src/services/__tests__/filter.service.spec.ts
@@ -92,6 +92,7 @@ const gridStub = {
   invalidate: jest.fn(),
   onLocalSortChanged: jest.fn(),
   onSort: new Slick.Event(),
+  onBeforeHeaderRowCellDestroy: new Slick.Event(),
   onHeaderRowCellRendered: new Slick.Event(),
   render: jest.fn(),
   setColumns: jest.fn(),
@@ -172,11 +173,15 @@ describe('FilterService', () => {
       gridStub.onHeaderRowCellRendered.notify(mockArgs as any, new Slick.EventData(), gridStub);
       const columnFilters = service.getColumnFilters();
       const filterMetadataArray = service.getFiltersMetadata();
+      const destroySpy = jest.spyOn(filterMetadataArray[0], 'destroy');
 
       expect(columnFilters).toEqual({});
       expect(filterMetadataArray.length).toBe(1);
       expect(filterMetadataArray[0] instanceof InputFilter).toBeTruthy();
       expect(filterMetadataArray[0]).toContainEntry(['searchTerms', []]);
+
+      gridStub.onBeforeHeaderRowCellDestroy.notify(mockArgs as any, new Slick.EventData(), gridStub);
+      expect(destroySpy).toHaveBeenCalled();
     });
 
     it('should call the same filter twice but expect the filter to be rendered only once', () => {
@@ -194,6 +199,7 @@ describe('FilterService', () => {
       gridStub.onHeaderRowCellRendered.notify(mockArgs as any, new Slick.EventData(), gridStub);
       const columnFilters = service.getColumnFilters();
       const filterMetadataArray = service.getFiltersMetadata();
+      const destroySpy = jest.spyOn(filterMetadataArray[0], 'destroy');
 
       expect(service.isFilterFirstRender).toBe(false);
       expect(columnFilters).toEqual({
@@ -202,6 +208,9 @@ describe('FilterService', () => {
       expect(filterMetadataArray.length).toBe(1);
       expect(filterMetadataArray[0] instanceof NativeSelectFilter).toBeTruthy();
       expect(filterMetadataArray[0]).toContainEntry(['searchTerms', [true]]);
+
+      gridStub.onBeforeHeaderRowCellDestroy.notify(mockArgs as any, new Slick.EventData(), gridStub);
+      expect(destroySpy).toHaveBeenCalled();
     });
 
     it('should call "onBackendFilterChange" when "onSearchChange" event is triggered', (done) => {
@@ -258,11 +267,15 @@ describe('FilterService', () => {
       gridStub.onHeaderRowCellRendered.notify(mockArgs as any, new Slick.EventData(), gridStub);
       const columnFilters = service.getColumnFilters();
       const filterMetadataArray = service.getFiltersMetadata();
+      const destroySpy = jest.spyOn(filterMetadataArray[0], 'destroy');
 
       expect(columnFilters).toEqual({});
       expect(filterMetadataArray.length).toBe(1);
       expect(filterMetadataArray[0] instanceof InputFilter).toBeTruthy();
       expect(filterMetadataArray[0]).toContainEntry(['searchTerms', []]);
+
+      gridStub.onBeforeHeaderRowCellDestroy.notify(mockArgs as any, new Slick.EventData(), gridStub);
+      expect(destroySpy).toHaveBeenCalled();
     });
 
     it('should call "onFilterChanged" when "onSearchChange" event is triggered', (done) => {

diff --git a/packages/common/src/services/filter.service.ts b/packages/common/src/services/filter.service.ts
@@ -180,6 +180,9 @@ export class FilterService {
       }
     });
 
+    // destroy Filter(s) to avoid leak and not keep orphan filters in the DOM tree
+    this.subscribeToOnHeaderRowCellRendered(grid);
+
     // subscribe to the SlickGrid event and call the backend execution
     if (this._onSearchChange) {
       const onSearchChangeHandler = this._onSearchChange;
@@ -232,6 +235,9 @@ export class FilterService {
     (this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onHeaderRowCellRenderedHandler>>).subscribe(onHeaderRowCellRenderedHandler, (_e, args) => {
       this.addFilterTemplateToHeaderRow(args);
     });
+
+    // destroy Filter(s) to avoid leak and not keep orphan filters
+    this.subscribeToOnHeaderRowCellRendered(grid);
   }
 
   async clearFilterByColumnId(event: Event, columnId: number | string): Promise<boolean> {
@@ -1095,6 +1101,18 @@ export class FilterService {
     return columnDefinitions;
   }
 
+  /**
+   * Subscribe to `onBeforeHeaderRowCellDestroy` to destroy Filter(s) to avoid leak and not keep orphan filters
+   * @param {Object} grid - Slick Grid object
+   */
+  protected subscribeToOnHeaderRowCellRendered(grid: SlickGrid) {
+    const onBeforeHeaderRowCellDestroyHandler = grid.onBeforeHeaderRowCellDestroy;
+    (this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onBeforeHeaderRowCellDestroyHandler>>).subscribe(onBeforeHeaderRowCellDestroyHandler, (_e, args) => {
+      const colFilter: Filter | undefined = this._filtersMetadata.find((filter: Filter) => filter.columnDef.id === args.column.id);
+      colFilter?.destroy?.();
+    });
+  }
+
   protected updateColumnFilters(searchTerms: SearchTerm[] | undefined, columnDef: any, operator?: OperatorType | OperatorString) {
     const fieldType = columnDef.filter?.type ?? columnDef.type ?? FieldType.string;
     const parsedSearchTerms = getParsedSearchTermsByFieldType(searchTerms, fieldType); // parsed term could a single value or an array of values