Fix removing nested components (#1028)

* Fix removing nested components

* Add tests

* Fix lint

* Refactoring

* Test refactoring

* Small refactoring

Author: dxvladislavvolkov

src/core/component.ts

@@ -51,6 +51,7 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo
     instance: any;
     isLinked = true;
     changedOptions = {};
+    removedOptions = [];
     widgetUpdateLocked = false;
 
     private _initTemplates() {
@@ -170,6 +171,7 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo
     }
 
     protected _destroyWidget() {
+        this.removedOptions = [];
         if (this.instance) {
             let element = this.instance.element();
             events.triggerHandler(element, 'dxremove', { _angularIntegration: true });
@@ -209,6 +211,7 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo
 
     ngAfterContentChecked() {
         this.applyOptions();
+        this.resetOptions();
         this.unlockWidgetUpdate();
     }
 
@@ -226,6 +229,15 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo
         }
     }
 
+    resetOptions() {
+        if (this.instance) {
+            this.removedOptions.forEach(option => {
+                this.instance.resetOption(option);
+            });
+            this.removedOptions = [];
+        }
+    }
+
     setTemplate(template: DxTemplateDirective) {
         this.templates.push(template);
     }

src/core/nested-option.ts

@@ -11,6 +11,7 @@ const VISIBILITY_CHANGE_SELECTOR = 'dx-visibility-change-handler';
 export interface INestedOptionContainer {
     instance: any;
     isLinked: boolean;
+    removedOptions: string[];
     optionChangedHandlers: EventEmitter<any>;
 }
 
@@ -64,8 +65,10 @@ export abstract class BaseNestedOption implements INestedOptionContainer, IColle
         }
     }
 
-    protected _resetOption(name: string) {
-        this.instance.resetOption(name);
+    protected _addRemovedOption(name: string) {
+        if (this.instance && this.removedOptions) {
+            this.removedOptions.push(name);
+        }
     }
 
     setHost(host: INestedOptionContainer, optionPath: IOptionPathGetter) {
@@ -86,6 +89,10 @@ export abstract class BaseNestedOption implements INestedOptionContainer, IColle
         return this._host && this._host.instance;
     }
 
+    get removedOptions() {
+        return this._host && this._host.removedOptions;
+    }
+
     get isLinked() {
         return !!this.instance && this._host.isLinked;
     }

templates/nested-component.tst

@@ -107,7 +107,7 @@ export class <#= it.className #>Component extends <#= it.baseClass #><#? it.hasT
 <#?#>
 <#? !it.isCollection #>
     ngOnDestroy() {
-        this._resetOption(this._optionPath);
+        this._addRemovedOption(this._fullOptionPath().slice(0, -1));
     }
 <#?#>
 }

tests/src/ui/chart.spec.ts

@@ -10,7 +10,7 @@ import {
 } from '@angular/core/testing';
 
 import {
-    DxChartModule, DxChartComponent
+    DxChartModule, DxChartComponent, DxScrollViewModule
 } from 'devextreme-angular';
 
 @Component({
@@ -23,13 +23,18 @@ class TestContainerComponent {
     }];
     @ViewChild(DxChartComponent) chart: DxChartComponent;
     dataSource = [];
+    disposed = false;
     commonSeriesSettings = {
         argumentField: undefined
     };
     seriesAsArray = [];
     seriesAsObject = {
         valueField: undefined
     };
+
+    onDisposing() {
+        this.disposed = true;
+    }
 }
 
 describe('DxChart', () => {
@@ -38,7 +43,7 @@ describe('DxChart', () => {
         TestBed.configureTestingModule(
             {
                 declarations: [TestContainerComponent],
-                imports: [DxChartModule]
+                imports: [DxChartModule, DxScrollViewModule]
             });
     });
 
@@ -62,7 +67,7 @@ describe('DxChart', () => {
     it('should not repainting twice in change detection cycle after applying options directly', () => {
         TestBed.overrideComponent(TestContainerComponent, {
             set: {
-                template: `<dx-chart 
+                template: `<dx-chart
                     [dataSource]="dataSource"
                     [series]="seriesAsArray"
                     [commonSeriesSettings]="{ argumentField: 'name' }"
@@ -92,7 +97,7 @@ describe('DxChart', () => {
     it('should not repainting twice in change detection cycle after detect changes in arrays', () => {
         TestBed.overrideComponent(TestContainerComponent, {
             set: {
-                template: `<dx-chart 
+                template: `<dx-chart
                     [dataSource]="dataSource"
                     [series]="seriesAsArray"
                     [commonSeriesSettings]="{ argumentField: 'name' }"
@@ -145,4 +150,31 @@ describe('DxChart', () => {
 
         expect(instance.option('argumentAxis.strips[0].label.text')).toBe('label2');
     });
+
+    it('should remove component by dispose method', () => {
+        TestBed.overrideComponent(TestContainerComponent, {
+            set: {
+                template: `
+                <dx-chart [dataSource]="[{ ID: 1, Text: 'A', Value: '2' }]" (onDisposing)='onDisposing()'>
+                    <dxi-series argumentField='Text' valueField='Value' type='bar' color='#f9ce2d'>
+                        <dxo-label [visible]='false'> </dxo-label>
+                    </dxi-series>
+                </dx-chart>`
+            }
+        });
+
+        jasmine.clock().uninstall();
+        jasmine.clock().install();
+        let fixture = TestBed.createComponent(TestContainerComponent);
+        fixture.detectChanges();
+
+        const testComponent = fixture.componentInstance;
+        const instance = testComponent.chart.instance;
+        instance.dispose();
+        fixture.detectChanges();
+        fixture.destroy();
+        expect(testComponent.disposed);
+        jasmine.clock().uninstall();
+
+    });
 });

tests/src/ui/data-grid.spec.ts

@@ -23,6 +23,7 @@ import DxDataGrid from 'devextreme/ui/data_grid';
 })
 class TestContainerComponent {
     showComponent = true;
+    selectionCount = 0;
     dataSource = [{
         id: 1,
         string: 'String',
@@ -56,6 +57,10 @@ class TestContainerComponent {
             this.columsChanged++;
         }
     }
+
+    selectionChanged() {
+        this.selectionCount++;
+    }
 }
 
 
@@ -235,6 +240,35 @@ describe('DxDataGrid', () => {
         jasmine.clock().uninstall();
     });
 
+    it('should not call onSelectionChanged when selection is resetting', () => {
+        TestBed.overrideComponent(TestContainerComponent, {
+            set: {
+                template: `<dx-data-grid *ngIf="showComponent"
+                    [dataSource]="[{id: 1, text: 'text'}, {id: 2, text: 'text2'}]"
+                    keyExpr="id"
+                    [selectedRowKeys]="[2]"
+                    (onSelectionChanged)="selectionChanged()">
+                    <dxo-selection mode="single"></dxo-selection>
+                    <dxi-column dataField="text"></dxi-column>
+                </dx-data-grid>`
+            }
+        });
+
+        jasmine.clock().uninstall();
+        jasmine.clock().install();
+
+        let fixture = TestBed.createComponent(TestContainerComponent);
+
+        fixture.detectChanges();
+
+        jasmine.clock().tick(101);
+        let testComponent = fixture.componentInstance;
+        testComponent.showComponent = false;
+        fixture.detectChanges();
+        expect(testComponent.selectionCount).toBe(0);
+        jasmine.clock().uninstall();
+    });
+
     it('should destroy devextreme components in template correctly', () => {
         @Component({
             selector: 'test-container-component',