feat(ngUpgrade): Auto detect ngUpgrade apps and make the ng12Hybrid flag unnecessary for most users (#3847)

Author: sjelin

lib/browser.ts

@@ -303,9 +303,25 @@ export class ProtractorBrowser extends Webdriver {
     this.ready = null;
     this.plugins_ = new Plugins({});
     this.resetUrl = DEFAULT_RESET_URL;
-    this.ng12Hybrid = false;
     this.debugHelper = new DebugHelper(this);
 
+    var ng12Hybrid_ = false;
+    Object.defineProperty(this, 'ng12Hybrid', {
+      get: function() {
+        return ng12Hybrid_;
+      },
+      set: function(ng12Hybrid) {
+        if (ng12Hybrid) {
+          logger.warn(
+              'You have set ng12Hybrid.  As of Protractor 4.1.0, ' +
+              'Protractor can automatically infer if you are using an ' +
+              'ngUpgrade app (as long as ng1 is loaded before you call ' +
+              'platformBrowserDynamic()), and this flag is no longer needed ' +
+              'for most users');
+        }
+        ng12Hybrid_ = ng12Hybrid;
+      }
+    });
     this.driver.getCapabilities().then((caps: Capabilities) => {
       // Internet Explorer does not accept data URLs, which are the default
       // reset URL for Protractor.
@@ -451,7 +467,7 @@ export class ProtractorBrowser extends Webdriver {
       } else if (this.rootEl) {
         return this.executeAsyncScript_(
             clientSideScripts.waitForAngular, 'Protractor.waitForAngular()' + description,
-            this.rootEl, this.ng12Hybrid);
+            this.rootEl);
       } else {
         return this.executeAsyncScript_(
             clientSideScripts.waitForAllAngular2, 'Protractor.waitForAngular()' + description);

lib/clientsidescripts.js

@@ -44,33 +44,38 @@ function wrapWithHelpers(fun) {
  * Asynchronous.
  *
  * @param {string} rootSelector The selector housing an ng-app
- * @param {boolean} ng12Hybrid Flag set if app is a hybrid of angular 1 and 2
  * @param {function(string)} callback callback. If a failure occurs, it will
  *     be passed as a parameter.
  */
-functions.waitForAngular = function(rootSelector, ng12Hybrid, callback) {
+functions.waitForAngular = function(rootSelector, callback) {
   var el = document.querySelector(rootSelector);
 
   try {
-    if (!ng12Hybrid && window.getAngularTestability) {
+    if (window.angular && !(window.angular.version &&
+        window.angular.version.major > 1)) {
+      if (angular.getTestability) {
+        angular.getTestability(el).whenStable(callback);
+      } else if (angular.element(el).injector()) {
+        angular.element(el).injector().get('$browser').
+            notifyWhenNoOutstandingRequests(callback);
+      } else {
+        throw new Error('root element (' + rootSelector + ') has no injector.' +
+           ' this may mean it is not inside ng-app.');
+      }
+    } else if (window.getAngularTestability) {
       window.getAngularTestability(el).whenStable(callback);
-      return;
-    }
-    if (!window.angular) {
+    } else if (!window.angular) {
       throw new Error('window.angular is undefined.  This could be either ' +
           'because this is a non-angular page or because your test involves ' +
           'client-side navigation, which can interfere with Protractor\'s ' +
           'bootstrapping.  See http://git.io/v4gXM for details');
-    }
-    if (angular.getTestability) {
-      angular.getTestability(el).whenStable(callback);
+    } else if (window.angular.version >= 2) {
+      throw new Error('You appear to be using angular, but window.' +
+          'getAngularTestability was never set.  This may be due to bad ' +
+          'obfuscation.');
     } else {
-      if (!angular.element(el).injector()) {
-        throw new Error('root element (' + rootSelector + ') has no injector.' +
-           ' this may mean it is not inside ng-app.');
-      }
-      angular.element(el).injector().get('$browser').
-          notifyWhenNoOutstandingRequests(callback);
+      throw new Error('Cannot get testability API for unknown angular ' +
+            'version "' + window.angular.version + '"');
     }
   } catch (err) {
     callback(err.message);
@@ -599,15 +604,36 @@ functions.testForAngular = function(attempts, ng12Hybrid, asyncCallback) {
       asyncCallback(args);
     }, 0);
   };
+  var definitelyNg1 = !!ng12Hybrid;
+  var definitelyNg2OrNewer = false;
   var check = function(n) {
     try {
-      if (!ng12Hybrid && window.getAllAngularTestabilities) {
-        callback({ver: 2});
-      } else if (window.angular && window.angular.resumeBootstrap) {
-        callback({ver: 1});
-      } else if (n < 1) {
-        if (window.angular) {
+      /* Figure out which version of angular we're waiting on */
+      if (!definitelyNg1 && !definitelyNg2OrNewer) {
+        if (window.angular && !(window.angular.version && window.angular.version.major > 1)) {
+          definitelyNg1 = true;
+        } else if (window.getAllAngularTestabilities) {
+          definitelyNg2OrNewer = true;
+        }
+      }
+      /* See if our version of angular is ready */
+      if (definitelyNg1) {
+        if (window.angular && window.angular.resumeBootstrap) {
+          return callback({ver: 1});
+        }
+      } else if (definitelyNg2OrNewer) {
+        if (true /* ng2 has no resumeBootstrap() */) {
+          return callback({ver: 2});
+        }
+      }
+      /* Try again (or fail) */
+      if (n < 1) {
+        if (definitelyNg1 && window.angular) {
           callback({message: 'angular never provided resumeBootstrap'});
+        } else if (ng12Hybrid && !window.angular) { 
+          callback({message: 'angular 1 never loaded' +
+              window.getAllAngularTestabilities ? ' (are you sure this app ' +
+              'uses ngUpgrade?  Try un-setting ng12Hybrid)' : ''});
         } else {
           callback({message: 'retries looking for angular exceeded'});
         }

spec/hybrid/async_spec.js

@@ -1,14 +1,8 @@
 describe('async angular1/2 hybrid using ngUpgrade application', function() {
   describe('@angular/upgrade/static', function() {
-    beforeEach(function() {
+    it('should be able to click buttons and wait for $timeout', function() {
       browser.get('/upgrade');
-    });
-
-    it('should set browser flag via config', function() {
-      expect(browser.ng12Hybrid).toBe(true);
-    });
 
-    it('should be able to click buttons and wait for $timeout', function() {
       var rootBtn = $$('my-app button').first();
       expect(rootBtn.getText()).toEqual('Click Count: 0');
       rootBtn.click();
@@ -25,23 +19,26 @@ describe('async angular1/2 hybrid using ngUpgrade application', function() {
       expect(ng1Btn.getText()).toEqual('Click Count: 1');
     });
 
-    it('should use the flag on the browser object', function() {
-      browser.ng12Hybrid = false;
-      browser.get('/ng2'); // will time out if Protractor expects hybrid
-      browser.ng12Hybrid = true;
+    it('should be able to automatically infer ng1/ng2/ngUpgrade', function() {
+      browser.get('/upgrade');
+      expect($('h1').getText()).toBe('My App');
+      browser.get('/ng1');
+      expect($$('h4').first().getText()).toBe('Bindings');
+      browser.get('/upgrade');
+      expect($('h1').getText()).toBe('My App');
+      browser.useAllAngular2AppRoots();
+      browser.get('/ng2');
+      expect($('h1').getText()).toBe('Test App for Angular 2');
+      browser.rootEl = 'body';
+      browser.get('/upgrade');
+      expect($('h1').getText()).toBe('My App');
     });
   });
 
   describe('@angular/upgrade (not static)', function() {
-    beforeEach(function() {
+    it('should be able to click buttons and wait for $timeout', function() {
       browser.get('/upgrade?no_static');
-    });
 
-    it('should set browser flag via config', function() {
-      expect(browser.ng12Hybrid).toBe(true);
-    });
-
-    it('should be able to click buttons and wait for $timeout', function() {
       var rootBtn = $$('my-app button').first();
       expect(rootBtn.getText()).toEqual('Click Count: 0');
       rootBtn.click();

spec/hybridConf.js

@@ -14,7 +14,5 @@ exports.config = {
 
   baseUrl: env.baseUrl,
 
-  rootElement: 'body',
-
-  ng12Hybrid: true
+  rootElement: 'body'
 };