Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

feat(ngUpgrade): Auto detect ngUpgrade apps and make the ng12Hybrid f… #3847

Merged
merged 1 commit into from
Dec 21, 2016

Conversation

sjelin
Copy link
Contributor

@sjelin sjelin commented Dec 19, 2016

…lag unnecessary for most users

Closes #3836

}
if (angular.getTestability) {
angular.getTestability(el).whenStable(callback);
} else if (window.angular.version == 2) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Angular versions are going to keep advancing now, we should change this to:

else if (window.angular.version.major >= 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 (n < 1) {
if (window.angular) {
// Figure out which version of angular we're waiting on
if(!definitelyNg1 && !definitelyNg2) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: space after if

Copy link
Member

@juliemr juliemr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -599,15 +603,36 @@ functions.testForAngular = function(attempts, ng12Hybrid, asyncCallback) {
asyncCallback(args);
}, 0);
};
var definitelyNg1 = !!ng12Hybrid;
var definitelyNg2 = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably rename this as well, given that angular version thing. What about just definitelyNg

callback({ver: 1});
} else if (n < 1) {
if (window.angular) {
// Figure out which version of angular we're waiting on
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments should be /** */ style because some versions of webdriver will glomp lines together and // comments break stuff.

}
} else if (definitelyNg2) {
if (true /** ng2 has no resumeBootstrap() **/) {
return callback({ver: 2});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this as ver: 2 is fine for now but we should make this more future-proof later.

@sjelin
Copy link
Contributor Author

sjelin commented Dec 20, 2016

@juliemr All comments addressed!

@juliemr
Copy link
Member

juliemr commented Dec 21, 2016

LGTM

@sjelin sjelin merged commit 86fd569 into angular:master Dec 21, 2016
sjelin added a commit to sjelin/protractor that referenced this pull request Dec 21, 2016
@sjelin sjelin deleted the upUpgrade branch December 22, 2016 00:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants