fix(core): check if transplanted views are attached to change detector (#46974)

edusperoni · dylhunn · commit 5ff715c5497f · 2022-08-08T11:33:29.000-07:00
Prevents change detection on views transplanted in OnPush components that have been detached from change detection. PR Close #46974
diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts
@@ -1670,12 +1670,16 @@ function refreshContainsDirtyView(lView: LView) {
        lContainer = getNextLContainer(lContainer)) {
     for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) {
       const embeddedLView = lContainer[i];
-      if (embeddedLView[FLAGS] & LViewFlags.RefreshTransplantedView) {
-        const embeddedTView = embeddedLView[TVIEW];
-        ngDevMode && assertDefined(embeddedTView, 'TView must be allocated');
-        refreshView(embeddedTView, embeddedLView, embeddedTView.template, embeddedLView[CONTEXT]!);
-      } else if (embeddedLView[TRANSPLANTED_VIEWS_TO_REFRESH] > 0) {
-        refreshContainsDirtyView(embeddedLView);
+      if (viewAttachedToChangeDetector(embeddedLView)) {
+        if (embeddedLView[FLAGS] & LViewFlags.RefreshTransplantedView) {
+          const embeddedTView = embeddedLView[TVIEW];
+          ngDevMode && assertDefined(embeddedTView, 'TView must be allocated');
+          refreshView(
+              embeddedTView, embeddedLView, embeddedTView.template, embeddedLView[CONTEXT]!);
+
+        } else if (embeddedLView[TRANSPLANTED_VIEWS_TO_REFRESH] > 0) {
+          refreshContainsDirtyView(embeddedLView);
+        }
       }
     }
   }
diff --git a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts
@@ -627,6 +627,124 @@ describe('change detection for transplanted views', () => {
     viewRef.detectChanges();
     expect(component.checks).toEqual(1);
   });
+
+  describe('when detached', () => {
+    @Component({
+      selector: 'on-push-component',
+      template: `
+          <ng-container #vc></ng-container>
+        `,
+      changeDetection: ChangeDetectionStrategy.OnPush
+    })
+    class OnPushComponent {
+      @ViewChild('vc', {read: ViewContainerRef}) viewContainer!: ViewContainerRef;
+      @Input() template!: TemplateRef<{}>;
+
+      createTemplate() {
+        return this.viewContainer.createEmbeddedView(this.template);
+      }
+    }
+
+    @Component({
+      selector: 'check-always-component',
+      template: `
+          <ng-container #vc></ng-container>
+        `,
+    })
+    class CheckAlwaysComponent {
+      @ViewChild('vc', {read: ViewContainerRef}) viewContainer!: ViewContainerRef;
+      @Input() template!: TemplateRef<{}>;
+
+      createTemplate() {
+        return this.viewContainer.createEmbeddedView(this.template);
+      }
+    }
+    let fixture: ComponentFixture<App>;
+    let appComponent: App;
+    let onPushComponent: OnPushComponent;
+    let checkAlwaysComponent: CheckAlwaysComponent;
+
+    @Component({
+      template: `
+      <ng-template #transplantedTemplate>{{ incrementChecks() }}</ng-template>
+      <on-push-component [template]="transplantedTemplate"></on-push-component>
+      <check-always-component [template]="transplantedTemplate"></check-always-component>
+        `
+    })
+    class App {
+      @ViewChild(OnPushComponent) onPushComponent!: OnPushComponent;
+      @ViewChild(CheckAlwaysComponent) checkAlwaysComponent!: CheckAlwaysComponent;
+      transplantedViewRefreshCount = 0;
+      incrementChecks() {
+        this.transplantedViewRefreshCount++;
+      }
+    }
+    beforeEach(() => {
+      TestBed.configureTestingModule({declarations: [App, OnPushComponent, CheckAlwaysComponent]});
+      fixture = TestBed.createComponent(App);
+      fixture.detectChanges();
+      appComponent = fixture.componentInstance;
+      onPushComponent = appComponent.onPushComponent;
+      checkAlwaysComponent = appComponent.checkAlwaysComponent;
+    });
+    describe('inside OnPush components', () => {
+      it('should detect changes when attached', () => {
+        onPushComponent.createTemplate();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(1);
+      });
+
+      it('should not detect changes', () => {
+        const viewRef = onPushComponent.createTemplate();
+        viewRef.detach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(0);
+        viewRef.reattach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(1);
+      });
+
+      it('should not detect changes on mixed detached/attached refs', () => {
+        onPushComponent.createTemplate();
+        const viewRef = onPushComponent.createTemplate();
+        viewRef.detach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(1);
+        viewRef.reattach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(3);
+      });
+    });
+
+    describe('inside CheckAlways component', () => {
+      it('should detect changes when attached', () => {
+        checkAlwaysComponent.createTemplate();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(1);
+      });
+
+      it('should not detect changes', () => {
+        const viewRef = checkAlwaysComponent.createTemplate();
+        viewRef.detach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(0);
+        viewRef.reattach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(1);
+      });
+
+      it('should not detect changes on mixed detached/attached refs', () => {
+        checkAlwaysComponent.createTemplate();
+        const viewRef = checkAlwaysComponent.createTemplate();
+        viewRef.detach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(1);
+        viewRef.reattach();
+        fixture.detectChanges(false);
+        expect(appComponent.transplantedViewRefreshCount).toEqual(3);
+      });
+    });
+  });
 });
 
 function trim(text: string|null): string {
