fix(upgrade): fix/improve support for lifecycle hooks (angular#13020)

With the exception of `$onChanges()`, all lifecycle hooks in ng1 are called on the controller, regardless if it is the binding destination or not (i.e. regardless of the value of `bindToController`). This change makes `upgrade` mimic that behavior when calling lifecycle hooks. Additionally, calling the `$onInit()` hook has been moved before calling the linking functions, which also mimics the ng1 behavior.
IgorMinar · Jan 6, 2017 · 21942a8 · 21942a8
1 parent 018865e
commit 21942a8
Show file tree

Hide file tree

Showing 2 changed files with 567 additions and 145 deletions.
diff --git a/modules/@angular/upgrade/src/upgrade_ng1_adapter.ts b/modules/@angular/upgrade/src/upgrade_ng1_adapter.ts
@@ -12,6 +12,21 @@ import * as angular from './angular_js';
 import {NG1_COMPILE, NG1_CONTROLLER, NG1_HTTP_BACKEND, NG1_SCOPE, NG1_TEMPLATE_CACHE} from './constants';
 import {controllerKey} from './util';
 
+interface IBindingDestination {
+  [key: string]: any;
+  $onChanges?: (changes: SimpleChanges) => void;
+}
+
+interface IControllerInstance extends IBindingDestination {
+  $doCheck?: () => void;
+  $onDestroy?: () => void;
+  $onInit?: () => void;
+  $postLink?: () => void;
+}
+
+type LifecycleHook = '$doCheck' | '$onChanges' | '$onDestroy' | '$onInit' | '$postLink';
+
+
 const CAMEL_CASE = /([A-Z])/g;
 const INITIAL_VALUE = {
   __UNINITIALIZED__: true
@@ -134,11 +149,11 @@ export class UpgradeNg1ComponentAdapterBuilder {
       httpBackend: angular.IHttpBackendService): Promise<angular.ILinkFn> {
     if (this.directive.template !== undefined) {
       this.linkFn = compileHtml(
-          typeof this.directive.template === 'function' ? this.directive.template() :
-                                                          this.directive.template);
+          isFunction(this.directive.template) ? this.directive.template() :
+                                                this.directive.template);
     } else if (this.directive.templateUrl) {
-      const url = typeof this.directive.templateUrl === 'function' ? this.directive.templateUrl() :
-                                                                     this.directive.templateUrl;
+      const url = isFunction(this.directive.templateUrl) ? this.directive.templateUrl() :
+                                                           this.directive.templateUrl;
       const html = templateCache.get(url);
       if (html !== undefined) {
         this.linkFn = compileHtml(html);
@@ -193,7 +208,8 @@ export class UpgradeNg1ComponentAdapterBuilder {
 }
 
 class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
-  destinationObj: any = null;
+  private controllerInstance: IControllerInstance = null;
+  destinationObj: IBindingDestination = null;
   checkLastValues: any[] = [];
   componentScope: angular.IScope;
   element: Element;
@@ -209,7 +225,8 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
     this.$element = angular.element(this.element);
     const controllerType = directive.controller;
     if (directive.bindToController && controllerType) {
-      this.destinationObj = this.buildController(controllerType);
+      this.controllerInstance = this.buildController(controllerType);
+      this.destinationObj = this.controllerInstance;
     } else {
       this.destinationObj = this.componentScope;
     }
@@ -231,8 +248,13 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
 
   ngOnInit() {
     if (!this.directive.bindToController && this.directive.controller) {
-      this.buildController(this.directive.controller);
+      this.controllerInstance = this.buildController(this.directive.controller);
+    }
+
+    if (this.controllerInstance && isFunction(this.controllerInstance.$onInit)) {
+      this.controllerInstance.$onInit();
     }
+
     let link = this.directive.link;
     if (typeof link == 'object') link = (<angular.IDirectivePrePost>link).pre;
     if (link) {
@@ -257,8 +279,9 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
       parentBoundTranscludeFn: (scope: any /** TODO #9100 */,
                                 cloneAttach: any /** TODO #9100 */) => { cloneAttach(childNodes); }
     });
-    if (this.destinationObj.$onInit) {
-      this.destinationObj.$onInit();
+
+    if (this.controllerInstance && isFunction(this.controllerInstance.$postLink)) {
+      this.controllerInstance.$postLink();
     }
   }
 
@@ -269,7 +292,8 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
       this.setComponentProperty(name, change.currentValue);
       ng1Changes[this.propertyMap[name]] = change;
     });
-    if (this.destinationObj.$onChanges) {
+
+    if (isFunction(this.destinationObj.$onChanges)) {
       this.destinationObj.$onChanges(ng1Changes);
     }
   }
@@ -290,14 +314,15 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
         }
       }
     }
-    if (this.destinationObj.$doCheck && this.directive.controller) {
-      this.destinationObj.$doCheck();
+
+    if (this.controllerInstance && isFunction(this.controllerInstance.$doCheck)) {
+      this.controllerInstance.$doCheck();
     }
   }
 
   ngOnDestroy() {
-    if (this.destinationObj.$onDestroy && this.directive.controller) {
-      this.destinationObj.$onDestroy();
+    if (this.controllerInstance && isFunction(this.controllerInstance.$onDestroy)) {
+      this.controllerInstance.$onDestroy();
     }
   }
 
@@ -353,3 +378,7 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
         `Directive '${this.directive.name}' require syntax unrecognized: ${this.directive.require}`);
   }
 }
+
+function isFunction(value: any): value is Function {
+  return typeof value === 'function';
+}