X Tutup
Skip to content

fix(upgrade): make ngUpgrade work with testability API #7603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sjelin wants to merge 1 commit into from
Closed

fix(upgrade): make ngUpgrade work with testability API #7603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions modules/angular2/src/upgrade/angular_js.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,16 +97,33 @@ export interface IControllerService {

export interface IInjectorService { get(key: string): any; }

export interface ITestabilityService {
findBindings(element: Element, expression: string, opt_exactMatch?: boolean): Element[];
findModels(element: Element, expression: string, opt_exactMatch?: boolean): Element[];
getLocation(): string;
setLocation(url: string): void;
whenStable(callback: Function): void;
}

function noNg() {
throw new Error('AngularJS v1.x is not loaded!');
}

var angular: {
bootstrap: (e: Element, modules: string[], config: IAngularBootstrapConfig) => void,
module: (prefix: string, dependencies?: string[]) => IModule,
element: (e: Element) => IAugmentedJQuery,
version: {major: number}
} = <any>{bootstrap: noNg, module: noNg, element: noNg, version: noNg};
var angular:
{
bootstrap: (e: Element, modules: string[], config: IAngularBootstrapConfig) => void,
module: (prefix: string, dependencies?: string[]) => IModule,
element: (e: Element) => IAugmentedJQuery,
version: {major: number}, resumeBootstrap?: () => void,
getTestability: (e: Element) => ITestabilityService
} = <any>{
bootstrap: noNg,
module: noNg,
element: noNg,
version: noNg,
resumeBootstrap: noNg,
Copy link
Copy Markdown
Member

@gkalpak gkalpak Mar 15, 2016

Choose a reason for hiding this comment

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

Why not add resumeBooystrap in the type above ?

Copy link
Copy Markdown
Contributor Author

@sjelin sjelin Mar 15, 2016

Choose a reason for hiding this comment

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

It's there. clang just put it on the same line as version

Copy link
Copy Markdown
Member

@gkalpak gkalpak Mar 15, 2016

Choose a reason for hiding this comment

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

Wow ! Did clang-format really do that ?
That's pretty unexpected !

(But then again you can't argue with clang-format.)

Copy link
Copy Markdown
Contributor Author

@sjelin sjelin Mar 15, 2016

Choose a reason for hiding this comment

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

I mean, I didn't put it on that line, so I think it must have been clang

getTestability: noNg
};


try {
Expand All @@ -121,3 +138,5 @@ export var bootstrap = angular.bootstrap;
export var module = angular.module;
export var element = angular.element;
export var version = angular.version;
export var resumeBootstrap = angular.resumeBootstrap;
export var getTestability = angular.getTestability;
1 change: 1 addition & 0 deletions modules/angular2/src/upgrade/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ export const NG1_HTTP_BACKEND = '$httpBackend';
export const NG1_INJECTOR = '$injector';
export const NG1_PARSE = '$parse';
export const NG1_TEMPLATE_CACHE = '$templateCache';
export const NG1_TESTABILITY = '$$testability';
export const REQUIRE_INJECTOR = '^' + NG2_INJECTOR;
73 changes: 61 additions & 12 deletions modules/angular2/src/upgrade/upgrade_adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import {
HostViewFactoryRef,
Provider,
Type,
Testability,
APPLICATION_COMMON_PROVIDERS
} from 'angular2/core';
import {global} from 'angular2/src/facade/lang';
import {ObservableWrapper} from 'angular2/src/facade/async';
import {BROWSER_PROVIDERS, BROWSER_APP_PROVIDERS} from 'angular2/platform/browser';

Expand All @@ -23,6 +25,7 @@ import {
NG1_PARSE,
NG1_ROOT_SCOPE,
NG1_SCOPE,
NG1_TESTABILITY,
NG2_APP_VIEW_MANAGER,
NG2_COMPILER,
NG2_INJECTOR,
Expand Down Expand Up @@ -309,6 +312,7 @@ export class UpgradeAdapter {
var rootScope: angular.IRootScopeService;
var hostViewFactoryRefMap: HostViewFactoryRefMap = {};
var ng1Module = angular.module(this.idPrefix, modules);
var ng1BootstrapPromise: Promise<any> = null;
var ng1compilePromise: Promise<any> = null;
ng1Module.value(NG2_INJECTOR, injector)
.value(NG2_ZONE, ngZone)
Expand All @@ -331,23 +335,68 @@ export class UpgradeAdapter {
return rootScope = rootScopeDelegate;
}
]);
}
])
.run([
'$injector',
'$rootScope',
(injector: angular.IInjectorService, rootScope: angular.IRootScopeService) => {
ng1Injector = injector;
ObservableWrapper.subscribe(ngZone.onMicrotaskEmpty,
(_) => ngZone.runOutsideAngular(() => rootScope.$apply()));
ng1compilePromise =
UpgradeNg1ComponentAdapterBuilder.resolve(this.downgradedComponents, injector);
provide.decorator(NG1_TESTABILITY, [
'$delegate',
function(testabilityDelegate: angular.ITestabilityService) {
var ng2Testability: Testability = injector.get(Testability);

var origonalWhenStable: Function = testabilityDelegate.whenStable;
var newWhenStable = (callback: Function): void => {
var whenStableContext: any = this;
origonalWhenStable.call(this, function() {
if (ng2Testability.isStable()) {
Copy link
Copy Markdown
Member

@juliemr juliemr Mar 14, 2016

Choose a reason for hiding this comment

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

I don't think this is necessary - can we remove the if check and just always do:

ng2Testability.whenStable(newWhenStable.bind(whenStableContext, callback));?

Copy link
Copy Markdown
Contributor Author

@sjelin sjelin Mar 14, 2016

Choose a reason for hiding this comment

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

No that would be an infinite loop

Copy link
Copy Markdown
Member

@juliemr juliemr Mar 14, 2016

Choose a reason for hiding this comment

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

Maybe I'm being dense, but how? ng2 whenStable doesn't call ng1 whenStable.

Copy link
Copy Markdown
Contributor Author

@sjelin sjelin Mar 15, 2016

Choose a reason for hiding this comment

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

You're not being dense, it's weird code.

ng2 whenStable is calling newWhenStable, which in turn calls ng1 whenStable, hence the loop

The easiest way to see this is by looking at the function as you say I should write it:

var newWhenStable = (callback: Function): void => {
  var whenStableContext: any = this;
  origonalWhenStable.call(this, function() {
    ng2Testability.whenStable(newWhenStable.bind(whenStableContext, callback));
  });
});

Note that once newWhenStable is called, all code paths lead to newWhenStable being called again, and nothing actually invokes callback.

I think you're thinking of

var newWhenStable = (callback: Function): void => {
  var whenStableContext: any = this;
  origonalWhenStable.call(this, function() {
    ng2Testability.whenStable(callback);
  });
});

Which wouldn't infinite loop, but would have a race condition where ng1 could become destabilized again while ng2 is stabilizing

Copy link
Copy Markdown
Member

@juliemr juliemr Mar 15, 2016

Choose a reason for hiding this comment

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

Gotcha, I see it now. Thanks!

callback.apply(this, arguments);
} else {
ng2Testability.whenStable(newWhenStable.bind(whenStableContext, callback));
}
});
};

testabilityDelegate.whenStable = newWhenStable;
return testabilityDelegate;
}
]);
}
]);

ng1compilePromise = new Promise((resolve, reject) => {
ng1Module.run([
'$injector',
'$rootScope',
(injector: angular.IInjectorService, rootScope: angular.IRootScopeService) => {
ng1Injector = injector;
ObservableWrapper.subscribe(ngZone.onMicrotaskEmpty,
(_) => ngZone.runOutsideAngular(() => rootScope.$apply()));
UpgradeNg1ComponentAdapterBuilder.resolve(this.downgradedComponents, injector)
.then(resolve, reject);
}
]);
});

// Make sure resumeBootstrap() only exists if the current bootstrap is deferred
var windowAngular = (<any>global).angular;
windowAngular.resumeBootstrap = undefined;

angular.element(element).data(controllerKey(NG2_INJECTOR), injector);
ngZone.run(() => { angular.bootstrap(element, [this.idPrefix], config); });
Promise.all([this.compileNg2Components(compiler, hostViewFactoryRefMap), ng1compilePromise])
ng1BootstrapPromise = new Promise((resolve, reject) => {
if (windowAngular.resumeBootstrap) {
var originalResumeBootstrap: () => void = windowAngular.resumeBootstrap;
windowAngular.resumeBootstrap = function() {
windowAngular.resumeBootstrap = originalResumeBootstrap;
windowAngular.resumeBootstrap.apply(this, arguments);
resolve();
};
} else {
resolve();
}
});

Promise.all([
this.compileNg2Components(compiler, hostViewFactoryRefMap),
ng1BootstrapPromise,
ng1compilePromise
])
.then(() => {
ngZone.run(() => {
if (rootScopePrototype) {
Expand Down
58 changes: 56 additions & 2 deletions modules/angular2/test/upgrade/upgrade_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,16 @@ import {
} from 'angular2/testing_internal';
import {DOM} from 'angular2/src/platform/dom/dom_adapter';

import {Component, Class, Inject, EventEmitter, ApplicationRef, provide} from 'angular2/core';
import {global} from 'angular2/src/facade/lang';
import {
Component,
Class,
Inject,
EventEmitter,
ApplicationRef,
provide,
Testability,
} from 'angular2/core';
import {UpgradeAdapter} from 'angular2/upgrade';
import * as angular from 'angular2/src/upgrade/angular_js';

Expand Down Expand Up @@ -559,6 +568,52 @@ export function main() {
}));
});

describe('testability', () => {
it('should handle deferred bootstrap', inject([AsyncTestCompleter], (async) => {
var adapter: UpgradeAdapter = new UpgradeAdapter();
var ng1Module = angular.module('ng1', []);
var bootstrapResumed: boolean = false;

var element = html("<div></div>");
window.name = 'NG_DEFER_BOOTSTRAP!' + window.name;

adapter.bootstrap(element, ['ng1'])
.ready((ref) => {
expect(bootstrapResumed).toEqual(true);
ref.dispose();
async.done();
});

setTimeout(() => {
bootstrapResumed = true;
(<any>global).angular.resumeBootstrap();
}, 100);
}));

it('should wait for ng2 testability', inject([AsyncTestCompleter], (async) => {
var adapter: UpgradeAdapter = new UpgradeAdapter();
var ng1Module = angular.module('ng1', []);
var element = html("<div></div>");
adapter.bootstrap(element, ['ng1'])
.ready((ref) => {
var ng2Testability: Testability = ref.ng2Injector.get(Testability);
ng2Testability.increasePendingRequestCount();
var ng2Stable = false;

angular.getTestability(element).whenStable(function() {
expect(ng2Stable).toEqual(true);
ref.dispose();
async.done();
});

setTimeout(() => {
ng2Stable = true;
ng2Testability.decreasePendingRequestCount();
}, 100);
});
}));
});

describe('examples', () => {
it('should verify UpgradeAdapter example', inject([AsyncTestCompleter], (async) => {
var adapter = new UpgradeAdapter();
Expand Down Expand Up @@ -594,7 +649,6 @@ export function main() {
});
}));
});

});
}

Expand Down
X Tutup