From dc14f57a1e26e7778c2d98491c4dc05366422163 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 9 Sep 2020 10:49:35 -0700 Subject: [PATCH] fix(core): clear the `RefreshTransplantedView` when detached The `RefreshTransplantedView` flag is used to indicate that the view or one of its children is transplanted and dirty, so it should still be refreshed as part of change detection. This flag is set on the transplanted view itself as well setting a counter on as its parents. When a transplanted view is detached and still has this flag, it means it got detached before it was refreshed. This can happen for "backwards references" or transplanted views that are inserted at a location that was already checked. In this case, we should decrement the parent counters _and_ clear the flag on the detached view so it's not seen as "transplanted" anymore (it is detached and has no parent counters to adjust). fixes #38619 --- .../core/src/render3/node_manipulation.ts | 1 + ...change_detection_transplanted_view_spec.ts | 36 ++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 1e0dba6d780fb..890307318146c 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -300,6 +300,7 @@ function detachMovedView(declarationContainer: LContainer, lView: LView) { // would be cleared and the counter decremented), we need to decrement the view counter here // instead. if (lView[FLAGS] & LViewFlags.RefreshTransplantedView) { + lView[FLAGS] &= ~LViewFlags.RefreshTransplantedView; updateTransplantedViewCount(insertionLContainer, -1); } diff --git a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts index d3c8f99f4b2a7..815aebcb5cc4b 100644 --- a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts +++ b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts @@ -7,7 +7,7 @@ */ import {CommonModule} from '@angular/common'; -import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DoCheck, Input, TemplateRef, Type, ViewChild} from '@angular/core'; +import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DoCheck, Input, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; import {AfterViewChecked} from '@angular/core/src/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; @@ -594,6 +594,40 @@ describe('change detection for transplanted views', () => { 'SheldonSheldonSheldon', 'Expected transplanted view to be refreshed even when insertion is not dirty'); }); + + it('should not fail when change detecting detached transplanted view', () => { + @Component({template: '{{incrementChecks()}}'}) + class AppComponent { + @ViewChild(TemplateRef) templateRef!: TemplateRef<{}>; + + constructor(readonly rootVref: ViewContainerRef, readonly cdr: ChangeDetectorRef) {} + + checks = 0; + incrementChecks() { + this.checks++; + } + } + + const fixture = TestBed.configureTestingModule({declarations: [AppComponent]}) + .createComponent(AppComponent); + const component = fixture.componentInstance; + fixture.detectChanges(); + + const viewRef = component.templateRef.createEmbeddedView({}); + // This `ViewContainerRef` is for the root view + component.rootVref.insert(viewRef); + // `detectChanges` on this `ChangeDetectorRef` will refresh this view and children, not the root + // view that has the transplanted `viewRef` inserted. + component.cdr.detectChanges(); + // The template should not have been refreshed because it was inserted "above" the component so + // `detectChanges` will not refresh it. + expect(component.checks).toEqual(0); + + // Detach view, manually call `detectChanges`, and verify the template was refreshed + component.rootVref.detach(); + viewRef.detectChanges(); + expect(component.checks).toEqual(1); + }); }); function trim(text: string|null): string {