Automatically detect hybrid apps most of the time

Before the ng12Hybrid flag, our wait logic was as follows:

if (window.getAngularTestability) {
  // We are in an ng2 app and should use ng2 testability
} else {
  // We are in an ng1 app and should use ng1 testability
}

This causes problems because sometimes, but not consistently, the ngUpgrade API would publish both testability APIs, causing us to use the ng2 testability API, but really we wanted the ng1 testability API (which is what you use in hybrid apps). So we introduced the ng12Hybrid flag to resolve this ambiguity. But was it really ambiguous? Instead we should have just reversed the original logic:

if (window.angular && !(window.angular.version && window.angular.version.major > 1)) {
  // We are in either an ng1 app or an ngUpgrade app and should use ng1 testability
} else {
  // We are in an ng2 app and should use ng2 testability
}

I propose we do the following:

In master, if ng12Hybrid is not set, used the new check (which makes ng12Hybrid needless)
In beta, rebase the above change in and then remove the ng12Hybrid flag entierly

I like this, and it's a good time to do this kind of breaking change. It would also be nice to just have a single waitForAngular function that detects the Angular version and does the right thing.

@mgiambalvo discussed this a bit offline, and this works fine for waitForAngular, but there's one snag during the bootstrap process: If you see the ng2 testability API, but not the ng1 API, it is possible that ng1 simply hasn't loaded yet. This is extremely unlikely, and would require the app to be loading ng1 asynchronously after platformBrowserDynamic() was called, as opposed to just having a <script> tag at the top of the page with ng1. So this is the new proposed solution:

Use the new wait logic instead of the ng12Hybrid flag everywhere except browser.get's testForAngular.

Change testForAngular's check() logic to the following:

var definitelyNg1 = !!ng12Hybrid;
var definitelyNg2 = false;
function check(n) {
  // Figure out which version of angular we're waiting on
  if(!definitelyNg1 && !definitelyNg2) {
    if (window.angular && !(window.angular.version && window.angular.version.major > 1)) {
      definitelyNg1 = true;
    } else if (window.getAllAngularTestabilities) {
      definitelyNg2 = true;
    }
  }
  // See if our version of angular is ready
  if (definitelyNg1) {
    if (window.angular && window.angular.resumeBootstrap) {
      return callback({ver: 1});
    }
  } else if (definitelyNg2) {
    if (window.getAllAngularTestabilities) {
       return callback({ver: 2});
    }
  }
  window.setTimeout(function() {check(n - 1);}, 1000);
}

So in English: If ng12Hybrid is set, we know we're looking for ng1 testability. Otherwise, we use the logic from my first comment to determine if we're looking for ng1 or ng2. Once we know which API we're looking for, we check only for that API. This is convoluted, but needed because we don't want protractor to conclude we're on an ng2 app simply because resumeBootstrap is taking a little while to show up. As long as the user has included ng1 in a <script> tag above all their code, this should always correctly differentiate between ng1, ng2, and upgrade apps.

If the ng12Hybrid flag is set, print a warning. Almost zero users will need this flag, and that small group can deal with a warning.

Because the ng12Hybrid will always be needed for some users, it will never go away, so there is no longer any breaking change, and this can all be done in master

SGTM.

Navigation Menu

Repository navigation

Automatically detect hybrid apps most of the time #3836

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Automatically detect hybrid apps most of the time #3836

Description

Activity

heathkit commented on Dec 17, 2016

sjelin commented on Dec 19, 2016

juliemr commented on Dec 19, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions