From 8f5c396a7c49132b77488844331c1420a86a5b1a Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 13 Jun 2019 17:41:45 +0200 Subject: [PATCH] fix(ivy): don't throw when attempting to destroy a destroyed ComponentRef (#31022) Currently in Ivy we throw when attempting to destroy a `ComponentRef` that has been destroyed, however in ViewEngine we didn't which can cause some tests to break. These changes remove the error to match ViewEngine. These changes resolve FW-1379. PR Close #31022 --- packages/core/src/render3/component_ref.ts | 16 +++++++------ .../acceptance/view_container_ref_spec.ts | 24 +++++++++++++++---- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 803ab65891..13da2b898d 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -18,7 +18,6 @@ import {ElementRef as viewEngine_ElementRef} from '../linker/element_ref'; import {NgModuleRef as viewEngine_NgModuleRef} from '../linker/ng_module_factory'; import {RendererFactory2} from '../render/api'; import {Sanitizer} from '../sanitization/security'; -import {assertDefined} from '../util/assert'; import {VERSION} from '../version'; import {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from '../view/provider'; @@ -258,13 +257,16 @@ export class ComponentRef extends viewEngine_ComponentRef { get injector(): Injector { return new NodeInjector(this._tNode, this._rootLView); } destroy(): void { - ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed'); - this.destroyCbs !.forEach(fn => fn()); - this.destroyCbs = null; - !this.hostView.destroyed && this.hostView.destroy(); + if (this.destroyCbs) { + this.destroyCbs.forEach(fn => fn()); + this.destroyCbs = null; + !this.hostView.destroyed && this.hostView.destroy(); + } } + onDestroy(callback: () => void): void { - ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed'); - this.destroyCbs !.push(callback); + if (this.destroyCbs) { + this.destroyCbs.push(callback); + } } } diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index 97eb4f8db0..0db09a7467 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -1035,6 +1035,20 @@ describe('ViewContainerRef', () => { .toEqual( '

12
34
'); }); + + it('should not throw when calling destroy() multiple times for a ComponentRef', () => { + @Component({template: ''}) + class App { + } + + TestBed.configureTestingModule({declarations: [App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + fixture.componentRef.destroy(); + expect(() => fixture.componentRef.destroy()).not.toThrow(); + }); + }); describe('insertion points and declaration points', () => { @@ -1061,7 +1075,7 @@ describe('ViewContainerRef', () => {
{{name}}
- + ` }) @@ -1105,10 +1119,10 @@ describe('ViewContainerRef', () => {
{{cell}} - {{row.value}} - {{name}}
- + - + `, }) @@ -1401,7 +1415,7 @@ describe('ViewContainerRef', () => { {{name}} - +
blah
` @@ -1780,7 +1794,7 @@ class ConstructorDir { @Component({ selector: 'constructor-app', - template: ` + template: `