perf(upgrade): unregister `$doCheck` watcher when destroying upgraded component (#14303)

PR Close #14303

modules/@angular/upgrade/src/static/upgrade_component.ts

@@ -100,6 +100,8 @@ export class UpgradeComponent implements OnInit, OnChanges, DoCheck, OnDestroy {
   private controllerInstance: IControllerInstance = null;
   private bindingDestination: IBindingDestination = null;
 
+  private unregisterDoCheckWatcher: Function;
+
   /**
    * Create a new `UpgradeComponent` instance. You should not normally need to do this.
    * Instead you should derive a new class from this one and call the super constructor
@@ -173,7 +175,7 @@ export class UpgradeComponent implements OnInit, OnChanges, DoCheck, OnDestroy {
     if (this.controllerInstance && isFunction(this.controllerInstance.$doCheck)) {
       const callDoCheck = () => this.controllerInstance.$doCheck();
 
-      this.$componentScope.$parent.$watch(callDoCheck);
+      this.unregisterDoCheckWatcher = this.$componentScope.$parent.$watch(callDoCheck);
       callDoCheck();
     }
 
@@ -236,6 +238,9 @@ export class UpgradeComponent implements OnInit, OnChanges, DoCheck, OnDestroy {
   }
 
   ngOnDestroy() {
+    if (isFunction(this.unregisterDoCheckWatcher)) {
+      this.unregisterDoCheckWatcher();
+    }
     if (this.controllerInstance && isFunction(this.controllerInstance.$onDestroy)) {
       this.controllerInstance.$onDestroy();
     }

modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts

@@ -6,11 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, ElementRef, EventEmitter, Injector, Input, NO_ERRORS_SCHEMA, NgModule, Output, SimpleChanges, destroyPlatform} from '@angular/core';
+import {Component, Directive, ElementRef, EventEmitter, Inject, Injector, Input, NO_ERRORS_SCHEMA, NgModule, Output, SimpleChanges, destroyPlatform} from '@angular/core';
 import {async, fakeAsync, tick} from '@angular/core/testing';
 import {BrowserModule} from '@angular/platform-browser';
 import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
 import * as angular from '@angular/upgrade/src/common/angular1';
+import {$SCOPE} from '@angular/upgrade/src/common/constants';
 import {UpgradeComponent, UpgradeModule, downgradeComponent} from '@angular/upgrade/static';
 
 import {$digest, bootstrap, html, multiTrim} from '../test_helpers';
@@ -2735,64 +2736,130 @@ export function main() {
          }));
     });
 
-    it('should destroy `$componentScope` when destroying the upgraded component', async(() => {
+    describe('destroying the upgraded component', () => {
-         const scopeDestroyListener = jasmine.createSpy('scopeDestroyListener');
+      it('should destroy `$componentScope`', async(() => {
-         let ng2ComponentAInstance: Ng2ComponentA;
+           const scopeDestroyListener = jasmine.createSpy('scopeDestroyListener');
+           let ng2ComponentAInstance: Ng2ComponentA;
 
-         // Define `ng1Component`
+           // Define `ng1Component`
-         const ng1Component: angular.IComponent = {
+           const ng1Component: angular.IComponent = {
-           controller: class {
+             controller: class {
-             constructor($scope: angular.IScope) { $scope.$on('$destroy', scopeDestroyListener); }
+               constructor($scope: angular.IScope) { $scope.$on('$destroy', scopeDestroyListener); }
+             }
+           };
+
+           // Define `Ng1ComponentFacade`
+           @Directive({selector: 'ng1'})
+           class Ng1ComponentFacade extends UpgradeComponent {
+             constructor(elementRef: ElementRef, injector: Injector) {
+               super('ng1', elementRef, injector);
+             }
            }
-         };
 
-         // Define `Ng1ComponentFacade`
+           // Define `Ng2Component`
-         @Directive({selector: 'ng1'})
+           @Component({selector: 'ng2A', template: '<ng2B *ngIf="!destroyIt"></ng2B>'})
-         class Ng1ComponentFacade extends UpgradeComponent {
+           class Ng2ComponentA {
-           constructor(elementRef: ElementRef, injector: Injector) {
+             destroyIt = false;
-             super('ng1', elementRef, injector);
+
+             constructor() { ng2ComponentAInstance = this; }
            }
-         }
 
-         // Define `Ng2Component`
+           @Component({selector: 'ng2B', template: '<ng1></ng1>'})
-         @Component({selector: 'ng2A', template: '<ng2B *ngIf="!destroyIt"></ng2B>'})
+           class Ng2ComponentB {
-         class Ng2ComponentA {
+           }
-           destroyIt = false;
 
-           constructor() { ng2ComponentAInstance = this; }
+           // Define `ng1Module`
-         }
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2A', downgradeComponent({component: Ng2ComponentA}));
 
-         @Component({selector: 'ng2B', template: '<ng1></ng1>'})
+           // Define `Ng2Module`
-         class Ng2ComponentB {
+           @NgModule({
-         }
+             declarations: [Ng1ComponentFacade, Ng2ComponentA, Ng2ComponentB],
+             entryComponents: [Ng2ComponentA],
+             imports: [BrowserModule, UpgradeModule]
+           })
+           class Ng2Module {
+             ngDoBootstrap() {}
+           }
 
-         // Define `ng1Module`
+           // Bootstrap
-         const ng1Module = angular.module('ng1Module', [])
+           const element = html(`<ng2-a></ng2-a>`);
-                               .component('ng1', ng1Component)
-                               .directive('ng2A', downgradeComponent({component: Ng2ComponentA}));
 
-         // Define `Ng2Module`
+           bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(adapter => {
-         @NgModule({
+             expect(scopeDestroyListener).not.toHaveBeenCalled();
-           declarations: [Ng1ComponentFacade, Ng2ComponentA, Ng2ComponentB],
-           entryComponents: [Ng2ComponentA],
-           imports: [BrowserModule, UpgradeModule]
-         })
-         class Ng2Module {
-           ngDoBootstrap() {}
-         }
 
-         // Bootstrap
+             ng2ComponentAInstance.destroyIt = true;
-         const element = html(`<ng2-a></ng2-a>`);
+             $digest(adapter);
 
-         bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(adapter => {
+             expect(scopeDestroyListener).toHaveBeenCalled();
-           expect(scopeDestroyListener).not.toHaveBeenCalled();
+           });
+         }));
 
-           ng2ComponentAInstance.destroyIt = true;
+      it('should clean up `$doCheck()` watchers from the parent scope', async(() => {
-           $digest(adapter);
+           let ng2Component: Ng2Component;
 
-           expect(scopeDestroyListener).toHaveBeenCalled();
+           // Define `ng1Component`
-         });
+           const ng1Component:
-       }));
+               angular.IComponent = {template: 'ng1', controller: class {$doCheck() {}}};
+
+           // Define `Ng1ComponentFacade`
+           @Directive({selector: 'ng1'})
+           class Ng1ComponentFacade extends UpgradeComponent {
+             constructor(elementRef: ElementRef, injector: Injector) {
+               super('ng1', elementRef, injector);
+             }
+           }
+
+           // Define `Ng2Component`
+           @Component({selector: 'ng2', template: '<ng1 *ngIf="doShow"></ng1>'})
+           class Ng2Component {
+             doShow: boolean = false;
+             constructor(@Inject($SCOPE) public $scope: angular.IScope) { ng2Component = this; }
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2', downgradeComponent({component: Ng2Component}));
+
+           // Define `Ng2Module`
+           @NgModule({
+             imports: [BrowserModule, UpgradeModule],
+             declarations: [Ng1ComponentFacade, Ng2Component],
+             entryComponents: [Ng2Component]
+           })
+           class Ng2Module {
+             ngDoBootstrap() {}
+           }
+
+           // Bootstrap
+           const element = html(`<ng2></ng2>`);
+
+           bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(adapter => {
+             const getWatcherCount: () => number = () =>
+                 (ng2Component.$scope as any).$$watchers.length;
+             const baseWatcherCount = getWatcherCount();
+
+             expect(multiTrim(document.body.textContent)).toBe('');
+
+             ng2Component.doShow = true;
+             $digest(adapter);
+             expect(multiTrim(document.body.textContent)).toBe('ng1');
+             expect(getWatcherCount()).toBe(baseWatcherCount + 1);
+
+             ng2Component.doShow = false;
+             $digest(adapter);
+             expect(multiTrim(document.body.textContent)).toBe('');
+             expect(getWatcherCount()).toBe(baseWatcherCount);
+
+             ng2Component.doShow = true;
+             $digest(adapter);
+             expect(multiTrim(document.body.textContent)).toBe('ng1');
+             expect(getWatcherCount()).toBe(baseWatcherCount + 1);
+           });
+         }));
+    });
 
     it('should support ng2 > ng1 > ng2 (no inputs/outputs)', async(() => {
          // Define `ng1Component`