diff --git a/goldens/circular-deps/packages.json b/goldens/circular-deps/packages.json index 3e9bf12faa..06e4b0d60b 100644 --- a/goldens/circular-deps/packages.json +++ b/goldens/circular-deps/packages.json @@ -108,45 +108,6 @@ "packages/compiler/src/render3/view/styling_builder.ts", "packages/compiler/src/render3/view/template.ts" ], - [ - "packages/core/src/change_detection/change_detection.ts", - "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_ref.ts", - "packages/core/src/linker/view_container_ref.ts", - "packages/core/src/linker/component_factory.ts" - ], - [ - "packages/core/src/change_detection/change_detection.ts", - "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_ref.ts", - "packages/core/src/linker/view_container_ref.ts", - "packages/core/src/linker/ng_module_factory.ts", - "packages/core/src/linker/component_factory_resolver.ts", - "packages/core/src/linker/component_factory.ts" - ], - [ - "packages/core/src/change_detection/change_detection.ts", - "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_ref.ts", - "packages/core/src/linker/view_container_ref.ts", - "packages/core/src/linker/template_ref.ts", - "packages/core/src/render3/instructions/shared.ts", - "packages/core/src/error_handler.ts", - "packages/core/src/errors.ts", - "packages/core/src/view/types.ts", - "packages/core/src/linker/component_factory.ts" - ], - [ - "packages/core/src/change_detection/change_detection.ts", - "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_ref.ts", - "packages/core/src/linker/view_container_ref.ts", - "packages/core/src/render3/instructions/shared.ts", - "packages/core/src/error_handler.ts", - "packages/core/src/errors.ts", - "packages/core/src/view/types.ts", - "packages/core/src/linker/component_factory.ts" - ], [ "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", @@ -164,8 +125,6 @@ [ "packages/core/src/change_detection/change_detector_ref.ts", "packages/core/src/render3/view_ref.ts", - "packages/core/src/linker/view_container_ref.ts", - "packages/core/src/linker/template_ref.ts", "packages/core/src/linker/view_ref.ts" ], [ @@ -205,18 +164,28 @@ "packages/core/src/errors.ts", "packages/core/src/view/types.ts" ], + [ + "packages/core/src/error_handler.ts", + "packages/core/src/errors.ts", + "packages/core/src/view/types.ts", + "packages/core/src/linker/template_ref.ts", + "packages/core/src/render3/instructions/shared.ts" + ], + [ + "packages/core/src/error_handler.ts", + "packages/core/src/errors.ts", + "packages/core/src/view/types.ts", + "packages/core/src/linker/view_container_ref.ts", + "packages/core/src/render3/instructions/shared.ts" + ], [ "packages/core/src/linker/component_factory_resolver.ts", + "packages/core/src/linker/component_factory.ts", "packages/core/src/linker/ng_module_factory.ts" ], [ - "packages/core/src/linker/template_ref.ts", - "packages/core/src/render3/view_ref.ts", - "packages/core/src/linker/view_container_ref.ts" - ], - [ - "packages/core/src/linker/view_container_ref.ts", - "packages/core/src/render3/view_ref.ts" + "packages/core/src/linker/component_factory_resolver.ts", + "packages/core/src/linker/ng_module_factory.ts" ], [ "packages/core/src/metadata/directives.ts", diff --git a/packages/core/src/linker/view_container_ref.ts b/packages/core/src/linker/view_container_ref.ts index b009bf0b11..b37e423efc 100644 --- a/packages/core/src/linker/view_container_ref.ts +++ b/packages/core/src/linker/view_container_ref.ts @@ -307,7 +307,7 @@ const R3ViewContainerRef = class ViewContainerRef extends VE_ViewContainerRef { addViewToContainer(tView, lContainer[T_HOST], renderer, lView, parentRNode, beforeNode); } - (viewRef as R3ViewRef).attachToViewContainerRef(this); + (viewRef as R3ViewRef).attachToViewContainerRef(); addToArray(getOrCreateViewRefs(lContainer), adjustedIdx, viewRef); return viewRef; diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index ec2bfff716..b88e8b51ba 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -7,12 +7,15 @@ */ import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detection/change_detector_ref'; -import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef, ViewRefTracker} from '../linker/view_ref'; +import {removeFromArray} from '../util/array_utils'; +import {assertEqual} from '../util/assert'; import {collectNativeNodes} from './collect_native_nodes'; import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupWithContext} from './instructions/shared'; -import {CONTEXT, FLAGS, LView, LViewFlags, TVIEW} from './interfaces/view'; -import {destroyLView, renderDetachView} from './node_manipulation'; +import {CONTAINER_HEADER_OFFSET, VIEW_REFS} from './interfaces/container'; +import {isLContainer} from './interfaces/type_checks'; +import {CONTEXT, FLAGS, LView, LViewFlags, PARENT, TVIEW} from './interfaces/view'; +import {destroyLView, detachView, renderDetachView} from './node_manipulation'; @@ -24,7 +27,7 @@ export interface viewEngine_ChangeDetectorRef_interface extends viewEngine_Chang export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_InternalViewRef, viewEngine_ChangeDetectorRef_interface { private _appRef: ViewRefTracker|null = null; - private _viewContainerRef: viewEngine_ViewContainerRef|null = null; + private _attachedToViewContainer = false; get rootNodes(): any[] { const lView = this._lView; @@ -65,14 +68,21 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_Int destroy(): void { if (this._appRef) { this._appRef.detachView(this); - } else if (this._viewContainerRef) { - const index = this._viewContainerRef.indexOf(this); - - if (index > -1) { - this._viewContainerRef.detach(index); + } else if (this._attachedToViewContainer) { + const parent = this._lView[PARENT]; + if (isLContainer(parent)) { + const viewRefs = parent[VIEW_REFS] as ViewRef[] | null; + const index = viewRefs ? viewRefs.indexOf(this) : -1; + if (index > -1) { + ngDevMode && + assertEqual( + index, parent.indexOf(this._lView) - CONTAINER_HEADER_OFFSET, + 'An attached view should be in the same position within its container as its ViewRef in the VIEW_REFS array.'); + detachView(parent, index); + removeFromArray(viewRefs!, index); + } } - - this._viewContainerRef = null; + this._attachedToViewContainer = false; } destroyLView(this._lView[TVIEW], this._lView); } @@ -271,11 +281,11 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_Int checkNoChangesInternal(this._lView[TVIEW], this._lView, this.context); } - attachToViewContainerRef(vcRef: viewEngine_ViewContainerRef) { + attachToViewContainerRef() { if (this._appRef) { throw new Error('This view is already attached directly to the ApplicationRef!'); } - this._viewContainerRef = vcRef; + this._attachedToViewContainer = true; } detachFromAppRef() { @@ -284,7 +294,7 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_Int } attachToAppRef(appRef: ViewRefTracker) { - if (this._viewContainerRef) { + if (this._attachedToViewContainer) { throw new Error('This view is already attached to a ViewContainer!'); } this._appRef = appRef; diff --git a/packages/core/test/acceptance/view_ref_spec.ts b/packages/core/test/acceptance/view_ref_spec.ts index 21b82a17fb..3c3f6bd8c1 100644 --- a/packages/core/test/acceptance/view_ref_spec.ts +++ b/packages/core/test/acceptance/view_ref_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ApplicationRef, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, ElementRef, Injector, NgModule} from '@angular/core'; +import {ApplicationRef, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, ElementRef, EmbeddedViewRef, Injector, NgModule, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core'; import {InternalViewRef} from '@angular/core/src/linker/view_ref'; import {TestBed} from '@angular/core/testing'; @@ -72,4 +72,80 @@ describe('ViewRef', () => { expect(called).toBe(true); }); + + it('should remove view ref from view container when destroyed', () => { + @Component({template: ''}) + class DynamicComponent { + constructor(public viewContainerRef: ViewContainerRef) {} + } + + @Component({template: `Hello`}) + class App { + @ViewChild(TemplateRef) templateRef!: TemplateRef; + componentRef!: ComponentRef; + viewRef!: EmbeddedViewRef; + constructor( + private _viewContainerRef: ViewContainerRef, + private _componentFactoryResolver: ComponentFactoryResolver) {} + + create() { + const factory = this._componentFactoryResolver.resolveComponentFactory(DynamicComponent); + this.viewRef = this.templateRef.createEmbeddedView(null); + this.componentRef = this._viewContainerRef.createComponent(factory); + this.componentRef.instance.viewContainerRef.insert(this.viewRef); + } + } + + @NgModule({declarations: [App, DynamicComponent], entryComponents: [DynamicComponent]}) + class MyTestModule { + } + + TestBed.configureTestingModule({imports: [MyTestModule]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + fixture.componentInstance.create(); + const viewContainerRef = fixture.componentInstance.componentRef.instance.viewContainerRef; + + expect(viewContainerRef.length).toBe(1); + fixture.componentInstance.viewRef.destroy(); + expect(viewContainerRef.length).toBe(0); + }); + + it('should mark a ViewRef as destroyed when the host view is destroyed', () => { + @Component({template: ''}) + class DynamicComponent { + constructor(public viewContainerRef: ViewContainerRef) {} + } + + @Component({template: `Hello`}) + class App { + @ViewChild(TemplateRef) templateRef!: TemplateRef; + componentRef!: ComponentRef; + viewRef!: EmbeddedViewRef; + constructor( + private _viewContainerRef: ViewContainerRef, + private _componentFactoryResolver: ComponentFactoryResolver) {} + + create() { + const factory = this._componentFactoryResolver.resolveComponentFactory(DynamicComponent); + this.viewRef = this.templateRef.createEmbeddedView(null); + this.componentRef = this._viewContainerRef.createComponent(factory); + this.componentRef.instance.viewContainerRef.insert(this.viewRef); + } + } + + @NgModule({declarations: [App, DynamicComponent], entryComponents: [DynamicComponent]}) + class MyTestModule { + } + + TestBed.configureTestingModule({imports: [MyTestModule]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + fixture.componentInstance.create(); + const {componentRef, viewRef} = fixture.componentInstance; + + expect(viewRef.destroyed).toBe(false); + componentRef.hostView.destroy(); + expect(viewRef.destroyed).toBe(true); + }); });