fix(ivy): don't detect changes on detached child embedded views (#34846)

Fixes Ivy detecting changes inside child embedded views, even though they're detached.

Note that there's on subtlety here: I made the changes inside `refreshDynamicEmbeddedViews` rather than `refreshView`, because we support detecting changes on a detached view (evidenced by a couple of unit tests), but only if it's triggered directly from the view's `ChangeDetectorRef`, however we shouldn't be detecting changes in the detached child view when something happens in the parent.

Fixes #34816.

PR Close #34846

integration/_payload-limits.json

@@ -3,7 +3,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 141476,
+        "main-es2015": 141985,
         "polyfills-es2015": 36808
       }
     }
@@ -21,7 +21,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 147610,
+        "main-es2015": 148115,
         "polyfills-es2015": 36808
       }
     }

packages/core/src/render3/instructions/shared.ts

@@ -1525,7 +1525,10 @@ function refreshDynamicEmbeddedViews(lView: LView) {
         const embeddedLView = viewOrContainer[i];
         const embeddedTView = embeddedLView[TVIEW];
         ngDevMode && assertDefined(embeddedTView, 'TView must be allocated');
-        refreshView(embeddedLView, embeddedTView, embeddedTView.template, embeddedLView[CONTEXT] !);
+        if (viewAttachedToChangeDetector(embeddedLView)) {
+          refreshView(
+              embeddedLView, embeddedTView, embeddedTView.template, embeddedLView[CONTEXT] !);
+        }
       }
       if ((activeIndexFlag & ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS) !== 0) {
         // We should only CD moved views if the component where they were inserted does not match

packages/core/test/acceptance/change_detection_spec.ts

@@ -22,10 +22,10 @@ describe('change detection', () => {
     @Directive({selector: '[viewManipulation]', exportAs: 'vm'})
     class ViewManipulation {
       constructor(
-          private _tplRef: TemplateRef<{}>, private _vcRef: ViewContainerRef,
+          private _tplRef: TemplateRef<{}>, public vcRef: ViewContainerRef,
           private _appRef: ApplicationRef) {}
 
-      insertIntoVcRef() { this._vcRef.createEmbeddedView(this._tplRef); }
+      insertIntoVcRef() { return this.vcRef.createEmbeddedView(this._tplRef); }
 
       insertIntoAppRef(): EmbeddedViewRef<{}> {
         const viewRef = this._tplRef.createEmbeddedView({});
@@ -43,11 +43,8 @@ describe('change detection', () => {
     class TestCmpt {
     }
 
-    beforeEach(() => {
-      TestBed.configureTestingModule({declarations: [TestCmpt, ViewManipulation]});
-    });
-
     it('should detect changes for embedded views inserted through ViewContainerRef', () => {
+      TestBed.configureTestingModule({declarations: [TestCmpt, ViewManipulation]});
       const fixture = TestBed.createComponent(TestCmpt);
       const vm = fixture.debugElement.childNodes[0].references['vm'] as ViewManipulation;
 
@@ -58,6 +55,7 @@ describe('change detection', () => {
     });
 
     it('should detect changes for embedded views attached to ApplicationRef', () => {
+      TestBed.configureTestingModule({declarations: [TestCmpt, ViewManipulation]});
       const fixture = TestBed.createComponent(TestCmpt);
       const vm = fixture.debugElement.childNodes[0].references['vm'] as ViewManipulation;
 
@@ -70,6 +68,109 @@ describe('change detection', () => {
       expect(viewRef.rootNodes[0]).toHaveText('change-detected');
     });
 
+    it('should not detect changes in child embedded views while they are detached', () => {
+      const counters = {componentView: 0, embeddedView: 0};
+
+      @Component({
+        template: `
+          <button (click)="noop()">Trigger change detection</button>
+          <div>{{increment('componentView')}}</div>
+          <ng-template #vm="vm" viewManipulation>{{increment('embeddedView')}}</ng-template>
+        `,
+        changeDetection: ChangeDetectionStrategy.OnPush
+      })
+      class App {
+        increment(counter: 'componentView'|'embeddedView') { counters[counter]++; }
+        noop() {}
+      }
+
+      TestBed.configureTestingModule({declarations: [App, ViewManipulation]});
+      const fixture = TestBed.createComponent(App);
+      const vm: ViewManipulation = fixture.debugElement.childNodes[2].references['vm'];
+      const button = fixture.nativeElement.querySelector('button');
+      const viewRef = vm.insertIntoVcRef();
+      fixture.detectChanges();
+
+      expect(counters).toEqual({componentView: 1, embeddedView: 1});
+
+      button.click();
+      fixture.detectChanges();
+      expect(counters).toEqual({componentView: 2, embeddedView: 2});
+
+      viewRef.detach();
+      button.click();
+      fixture.detectChanges();
+
+      expect(counters).toEqual({componentView: 3, embeddedView: 2});
+
+      // Re-attach the view to ensure that the process can be reversed.
+      viewRef.reattach();
+      button.click();
+      fixture.detectChanges();
+
+      expect(counters).toEqual({componentView: 4, embeddedView: 3});
+    });
+
+    it('should not detect changes in child component views while they are detached', () => {
+      let counter = 0;
+
+      @Component({
+        template: `<ng-template #vm="vm" viewManipulation></ng-template>`,
+        changeDetection: ChangeDetectionStrategy.OnPush
+      })
+      class App {
+      }
+
+      @Component({
+        template: `
+          <button (click)="noop()">Trigger change detection</button>
+          <div>{{increment()}}</div>
+        `,
+        changeDetection: ChangeDetectionStrategy.OnPush
+      })
+      class DynamicComp {
+        increment() { counter++; }
+        noop() {}
+      }
+
+      // We need to declare a module so that we can specify `entryComponents`
+      // for when the test is run against ViewEngine.
+      @NgModule({
+        declarations: [App, ViewManipulation, DynamicComp],
+        exports: [App, ViewManipulation, DynamicComp],
+        entryComponents: [DynamicComp]
+      })
+      class AppModule {
+      }
+
+      TestBed.configureTestingModule({imports: [AppModule]});
+      const fixture = TestBed.createComponent(App);
+      const vm: ViewManipulation = fixture.debugElement.childNodes[0].references['vm'];
+      const factory = TestBed.get(ComponentFactoryResolver).resolveComponentFactory(DynamicComp);
+      const componentRef = vm.vcRef.createComponent(factory);
+      const button = fixture.nativeElement.querySelector('button');
+      fixture.detectChanges();
+
+      expect(counter).toBe(1);
+
+      button.click();
+      fixture.detectChanges();
+      expect(counter).toBe(2);
+
+      componentRef.changeDetectorRef.detach();
+      button.click();
+      fixture.detectChanges();
+
+      expect(counter).toBe(2);
+
+      // Re-attach the change detector to ensure that the process can be reversed.
+      componentRef.changeDetectorRef.reattach();
+      button.click();
+      fixture.detectChanges();
+
+      expect(counter).toBe(3);
+    });
+
   });
 
   describe('markForCheck', () => {