From d21331e902341abe3fdec267d0acae223c801c52 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 5 Aug 2016 13:32:04 -0700 Subject: [PATCH] fix(ngUpgrade): to work with @NgModule We changed the bootstrap order: 1. create NgZone 2. bootstrap ng1 inside NgZone and upgrade ng1 components to ng2 components. 3. bootstrap ng2 with NgZone Note: Previous footgun behavior was: bootstrap ng2 first to extract NgZone, so that ng1 bootstrap can happen in NgZone. This meant that if ng2 bootstrap eagerly compiled a component which contained ng1 components, then we did not have complete metadata. --- modules/@angular/core/src/application_ref.ts | 15 +- modules/@angular/upgrade/src/angular_js.ts | 5 +- .../@angular/upgrade/src/upgrade_adapter.ts | 168 +++++++++++------- .../upgrade/src/upgrade_ng1_adapter.ts | 12 +- modules/@angular/upgrade/test/upgrade_spec.ts | 40 ++++- modules/playground/src/upgrade/index.ts | 21 +-- tools/public_api_guard/upgrade/index.d.ts | 3 +- 7 files changed, 179 insertions(+), 85 deletions(-) diff --git a/modules/@angular/core/src/application_ref.ts b/modules/@angular/core/src/application_ref.ts index 3db09c8523..fc7072de54 100644 --- a/modules/@angular/core/src/application_ref.ts +++ b/modules/@angular/core/src/application_ref.ts @@ -339,11 +339,16 @@ export class PlatformRef_ extends PlatformRef { dispose(): void { this.destroy(); } bootstrapModuleFactory(moduleFactory: NgModuleFactory): Promise> { + return this._bootstrapModuleFactoryWithZone(moduleFactory, null); + } + + private _bootstrapModuleFactoryWithZone(moduleFactory: NgModuleFactory, ngZone: NgZone): + Promise> { // Note: We need to create the NgZone _before_ we instantiate the module, // as instantiating the module creates some providers eagerly. // So we create a mini parent injector that just contains the new NgZone and // pass that as parent to the NgModuleFactory. - const ngZone = new NgZone({enableLongStackTrace: isDevMode()}); + if (!ngZone) ngZone = new NgZone({enableLongStackTrace: isDevMode()}); // Attention: Don't use ApplicationRef.run here, // as we want to be sure that all possible constructor calls are inside `ngZone.run`! return ngZone.run(() => { @@ -371,11 +376,17 @@ export class PlatformRef_ extends PlatformRef { bootstrapModule( moduleType: ConcreteType, compilerOptions: CompilerOptions|CompilerOptions[] = []): Promise> { + return this._bootstrapModuleWithZone(moduleType, compilerOptions, null); + } + + private _bootstrapModuleWithZone( + moduleType: ConcreteType, compilerOptions: CompilerOptions|CompilerOptions[] = [], + ngZone: NgZone): Promise> { const compilerFactory: CompilerFactory = this.injector.get(CompilerFactory); const compiler = compilerFactory.createCompiler( compilerOptions instanceof Array ? compilerOptions : [compilerOptions]); return compiler.compileModuleAsync(moduleType) - .then((moduleFactory) => this.bootstrapModuleFactory(moduleFactory)); + .then((moduleFactory) => this._bootstrapModuleFactoryWithZone(moduleFactory, ngZone)); } private _moduleDoBootstrap(moduleRef: NgModuleInjector) { diff --git a/modules/@angular/upgrade/src/angular_js.ts b/modules/@angular/upgrade/src/angular_js.ts index 4ea574b300..86c7c86243 100644 --- a/modules/@angular/upgrade/src/angular_js.ts +++ b/modules/@angular/upgrade/src/angular_js.ts @@ -115,7 +115,10 @@ export interface IControllerService { (controllerName: string, locals?: any): any; } -export interface IInjectorService { get(key: string): any; } +export interface IInjectorService { + get(key: string): any; + has(key: string): boolean; +} export interface ITestabilityService { findBindings(element: Element, expression: string, opt_exactMatch?: boolean): Element[]; diff --git a/modules/@angular/upgrade/src/upgrade_adapter.ts b/modules/@angular/upgrade/src/upgrade_adapter.ts index af36b2965a..3b45ee6e5d 100644 --- a/modules/@angular/upgrade/src/upgrade_adapter.ts +++ b/modules/@angular/upgrade/src/upgrade_adapter.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Compiler, CompilerFactory, ComponentFactory, ComponentResolver, Injector, NgModule, NgModuleRef, NgZone, PlatformRef, Provider, ReflectiveInjector, Testability, Type, provide} from '@angular/core'; +import {BaseException, Compiler, ComponentFactory, Injector, NgModule, NgModuleRef, NgZone, Provider, Testability, Type} from '@angular/core'; import {BrowserModule} from '@angular/platform-browser'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; @@ -59,11 +59,11 @@ var upgradeCount: number = 0; * ### Example * * ``` - * var adapter = new UpgradeAdapter(); + * var adapter = new UpgradeAdapter(forwardRef(() => MyNg2Module)); * var module = angular.module('myExample', []); - * module.directive('ng2', adapter.downgradeNg2Component(Ng2)); + * module.directive('ng2Comp', adapter.downgradeNg2Component(Ng2)); * - * module.directive('ng1', function() { + * module.directive('ng1Hello', function() { * return { * scope: { title: '=' }, * template: 'ng1[Hello {{title}}!]()' @@ -72,20 +72,28 @@ var upgradeCount: number = 0; * * * @Component({ - * selector: 'ng2', + * selector: 'ng2-comp', * inputs: ['name'], - * template: 'ng2[transclude]()', - * directives: [adapter.upgradeNg1Component('ng1')] + * template: 'ng2[transclude]()', + * directives: * }) - * class Ng2 { + * class Ng2Component { * } * - * document.body.innerHTML = 'project'; + * @NgModule({ + * declarations: [Ng2Component, adapter.upgradeNg1Component('ng1Hello')], + * imports: [BrowserModule] + * }) + * class MyNg2Module {} + * + * + * document.body.innerHTML = 'project'; * * adapter.bootstrap(document.body, ['myExample']).ready(function() { * expect(document.body.textContent).toEqual( * "ng2[ng1[Hello World!](transclude)](project)"); * }); + * * ``` * * @experimental @@ -95,14 +103,26 @@ export class UpgradeAdapter { private idPrefix: string = `NG2_UPGRADE_${upgradeCount++}_`; /* @internal */ private upgradedComponents: Type[] = []; - /* @internal */ - private downgradedComponents: {[name: string]: UpgradeNg1ComponentAdapterBuilder} = {}; + /** + * An internal map of ng1 components which need to up upgraded to ng2. + * + * We can't upgrade until injector is instantiated and we can retrieve the component metadata. + * For this reason we keep a list of components to upgrade until ng1 injector is bootstrapped. + * + * @internal + */ + private ng1ComponentsToBeUpgraded: {[name: string]: UpgradeNg1ComponentAdapterBuilder} = {}; /* @internal */ private providers: Array = []; // the ng2AppModule param should be required once the deprecated @Component.directives prop is // removed - constructor(private ng2AppModule?: Type) {} + constructor(private ng2AppModule?: Type) { + if (arguments.length && !ng2AppModule) { + throw new BaseException( + 'UpgradeAdapter constructor called with undefined instead of a ng module type'); + } + } /** * Allows Angular v2 Component to be used from AngularJS v1. @@ -132,7 +152,7 @@ export class UpgradeAdapter { * ### Example * * ``` - * var adapter = new UpgradeAdapter(); + * var adapter = new UpgradeAdapter(forwardRef(() => MyNg2Module)); * var module = angular.module('myExample', []); * module.directive('greet', adapter.downgradeNg2Component(Greeter)); * @@ -145,6 +165,12 @@ export class UpgradeAdapter { * @Input() name: string; * } * + * @NgModule({ + * declarations: [Greeter], + * imports: [BrowserModule] + * }) + * class MyNg2Module {} + * * document.body.innerHTML = * 'ng1 template: text'; * @@ -204,7 +230,7 @@ export class UpgradeAdapter { * ### Example * * ``` - * var adapter = new UpgradeAdapter(); + * var adapter = new UpgradeAdapter(forwardRef(() => MyNg2Module)); * var module = angular.module('myExample', []); * * module.directive('greet', function() { @@ -219,11 +245,16 @@ export class UpgradeAdapter { * @Component({ * selector: 'ng2', * template: 'ng2 template: text' - * directives: [adapter.upgradeNg1Component('greet')] * }) * class Ng2 { * } * + * @NgModule({ + * declarations: [Ng2, adapter.upgradeNg1Component('greet')], + * imports: [BrowserModule] + * }) + * class MyNg2Module {} + * * document.body.innerHTML = ''; * * adapter.bootstrap(document.body, ['myExample']).ready(function() { @@ -232,10 +263,11 @@ export class UpgradeAdapter { * ``` */ upgradeNg1Component(name: string): Type { - if ((this.downgradedComponents).hasOwnProperty(name)) { - return this.downgradedComponents[name].type; + if ((this.ng1ComponentsToBeUpgraded).hasOwnProperty(name)) { + return this.ng1ComponentsToBeUpgraded[name].type; } else { - return (this.downgradedComponents[name] = new UpgradeNg1ComponentAdapterBuilder(name)).type; + return (this.ng1ComponentsToBeUpgraded[name] = new UpgradeNg1ComponentAdapterBuilder(name)) + .type; } } @@ -264,12 +296,17 @@ export class UpgradeAdapter { * @Component({ * selector: 'ng2', * inputs: ['name'], - * template: 'ng2[transclude]()', - * directives: [adapter.upgradeNg1Component('ng1')] + * template: 'ng2[transclude]()' * }) * class Ng2 { * } * + * @NgModule({ + * declarations: [Ng2, adapter.upgradeNg1Component('ng1')], + * imports: [BrowserModule] + * }) + * class MyNg2Module {} + * * document.body.innerHTML = 'project'; * * adapter.bootstrap(document.body, ['myExample']).ready(function() { @@ -280,55 +317,35 @@ export class UpgradeAdapter { */ bootstrap(element: Element, modules?: any[], config?: angular.IAngularBootstrapConfig): UpgradeAdapterRef { + const ngZone = + new NgZone({enableLongStackTrace: Zone.hasOwnProperty('longStackTraceZoneSpec')}); var upgrade = new UpgradeAdapterRef(); var ng1Injector: angular.IInjectorService = null; - var platformRef: PlatformRef = platformBrowserDynamic(); - var providers = [ - {provide: NG1_INJECTOR, useFactory: () => ng1Injector}, - {provide: NG1_COMPILE, useFactory: () => ng1Injector.get(NG1_COMPILE)}, this.providers - ]; - - @NgModule({providers: providers, imports: [BrowserModule]}) - class DynamicModule { - ngDoBootstrap() {} - } - - platformRef.bootstrapModule(this.ng2AppModule || DynamicModule).then((moduleRef) => { - ng1Injector = this._afterNg2ModuleBootstrap(moduleRef, upgrade, element, modules, config); - }); - return upgrade; - } - - private _afterNg2ModuleBootstrap( - moduleRef: NgModuleRef, upgrade: UpgradeAdapterRef, element: Element, modules?: any[], - config?: angular.IAngularBootstrapConfig): angular.IInjectorService { - const boundCompiler: Compiler = moduleRef.injector.get(Compiler); - var ng1Injector: angular.IInjectorService = null; - var injector: Injector = moduleRef.injector; - var ngZone: NgZone = injector.get(NgZone); + var moduleRef: NgModuleRef = null; var delayApplyExps: Function[] = []; var original$applyFn: Function; var rootScopePrototype: any; var rootScope: angular.IRootScopeService; var componentFactoryRefMap: ComponentFactoryRefMap = {}; var ng1Module = angular.module(this.idPrefix, modules); - var ng1BootstrapPromise: Promise = null; - var ng1compilePromise: Promise = null; - ng1Module.value(NG2_INJECTOR, injector) + var ng1BootstrapPromise: Promise; + var ng1compilePromise: Promise; + ng1Module.factory(NG2_INJECTOR, () => moduleRef.injector.get(Injector)) .value(NG2_ZONE, ngZone) - .value(NG2_COMPILER, boundCompiler) + .factory(NG2_COMPILER, () => moduleRef.injector.get(Compiler)) .value(NG2_COMPONENT_FACTORY_REF_MAP, componentFactoryRefMap) .config([ '$provide', '$injector', - (provide: any /** TODO #???? */, ng1Injector: any /** TODO #???? */) => { + (provide: any /** TODO #???? */, ng1Injector: angular.IInjectorService) => { provide.decorator(NG1_ROOT_SCOPE, [ '$delegate', function(rootScopeDelegate: angular.IRootScopeService) { + // Capture the root apply so that we can delay first call to $apply until we + // bootstrap Angular 2 and then we replay and restore the $apply. rootScopePrototype = rootScopeDelegate.constructor.prototype; if (rootScopePrototype.hasOwnProperty('$apply')) { original$applyFn = rootScopePrototype.$apply; - rootScopePrototype.$apply = (exp: any /** TODO #???? */) => - delayApplyExps.push(exp); + rootScopePrototype.$apply = (exp: any) => delayApplyExps.push(exp); } else { throw new Error('Failed to find \'$apply\' on \'$rootScope\'!'); } @@ -339,12 +356,12 @@ export class UpgradeAdapter { provide.decorator(NG1_TESTABILITY, [ '$delegate', function(testabilityDelegate: angular.ITestabilityService) { - var ng2Testability: Testability = injector.get(Testability); - var origonalWhenStable: Function = testabilityDelegate.whenStable; + var originalWhenStable: Function = testabilityDelegate.whenStable; var newWhenStable = (callback: Function): void => { var whenStableContext: any = this; - origonalWhenStable.call(this, function() { + originalWhenStable.call(this, function() { + var ng2Testability: Testability = moduleRef.injector.get(Testability); if (ng2Testability.isStable()) { callback.apply(this, arguments); } else { @@ -366,12 +383,31 @@ export class UpgradeAdapter { '$injector', '$rootScope', (injector: angular.IInjectorService, rootScope: angular.IRootScopeService) => { ng1Injector = injector; - ngZone.onMicrotaskEmpty.subscribe({ - next: (_: any /** TODO #???? */) => - ngZone.runOutsideAngular(() => rootScope.$evalAsync()) - }); - UpgradeNg1ComponentAdapterBuilder.resolve(this.downgradedComponents, injector) - .then(resolve, reject); + UpgradeNg1ComponentAdapterBuilder.resolve(this.ng1ComponentsToBeUpgraded, injector) + .then(() => { + // At this point we have ng1 injector and we have lifted ng1 components into ng2, we + // now can bootstrap ng2. + var DynamicNgUpgradeModule = + NgModule({ + providers: [ + {provide: NG1_INJECTOR, useFactory: () => ng1Injector}, + {provide: NG1_COMPILE, useFactory: () => ng1Injector.get(NG1_COMPILE)}, + this.providers + ], + imports: this.ng2AppModule ? [this.ng2AppModule] : [BrowserModule] + }).Class({constructor: function() {}, ngDoBootstrap: function() {}}); + (platformBrowserDynamic() as any) + ._bootstrapModuleWithZone(DynamicNgUpgradeModule, undefined, ngZone) + .then((ref: NgModuleRef) => { + moduleRef = ref; + angular.element(element).data( + controllerKey(NG2_INJECTOR), moduleRef.injector); + ngZone.onMicrotaskEmpty.subscribe({ + next: (_: any) => ngZone.runOutsideAngular(() => rootScope.$evalAsync()) + }); + }) + .then(resolve, reject); + }); } ]); }); @@ -380,7 +416,6 @@ export class UpgradeAdapter { var windowAngular = (window as any /** TODO #???? */)['angular']; windowAngular.resumeBootstrap = undefined; - angular.element(element).data(controllerKey(NG2_INJECTOR), injector); ngZone.run(() => { angular.bootstrap(element, [this.idPrefix], config); }); ng1BootstrapPromise = new Promise((resolve, reject) => { if (windowAngular.resumeBootstrap) { @@ -396,9 +431,12 @@ export class UpgradeAdapter { }); Promise.all([ng1BootstrapPromise, ng1compilePromise]) - .then(() => { return this.compileNg2Components(boundCompiler, componentFactoryRefMap); }) .then(() => { - ngZone.run(() => { + return this.compileNg2Components( + moduleRef.injector.get(Compiler), componentFactoryRefMap); + }) + .then(() => { + moduleRef.injector.get(NgZone).run(() => { if (rootScopePrototype) { rootScopePrototype.$apply = original$applyFn; // restore original $apply while (delayApplyExps.length) { @@ -409,7 +447,7 @@ export class UpgradeAdapter { } }); }, onError); - return ng1Injector; + return upgrade; } /** @@ -528,7 +566,7 @@ export class UpgradeAdapter { var promises: Array>> = []; var types = this.upgradedComponents; for (var i = 0; i < types.length; i++) { - promises.push(compiler.compileComponentAsync(types[i])); + promises.push(compiler.compileComponentAsync(types[i], this.ng2AppModule)); } return Promise.all(promises).then((componentFactories: Array>) => { var types = this.upgradedComponents; diff --git a/modules/@angular/upgrade/src/upgrade_ng1_adapter.ts b/modules/@angular/upgrade/src/upgrade_ng1_adapter.ts index c48a28ba75..a0665e8639 100644 --- a/modules/@angular/upgrade/src/upgrade_ng1_adapter.ts +++ b/modules/@angular/upgrade/src/upgrade_ng1_adapter.ts @@ -127,7 +127,7 @@ export class UpgradeNg1ComponentAdapterBuilder { compileTemplate( compile: angular.ICompileService, templateCache: angular.ITemplateCacheService, - httpBackend: angular.IHttpBackendService): Promise { + httpBackend: angular.IHttpBackendService): Promise { if (this.directive.template !== undefined) { this.linkFn = compileHtml( typeof this.directive.template === 'function' ? this.directive.template() : @@ -162,10 +162,13 @@ export class UpgradeNg1ComponentAdapterBuilder { } } + /** + * Upgrade ng1 components into Angular 2. + */ static resolve( exportedComponents: {[name: string]: UpgradeNg1ComponentAdapterBuilder}, - injector: angular.IInjectorService): Promise { - var promises: any[] /** TODO #9100 */ = []; + injector: angular.IInjectorService): Promise { + var promises: Promise[] = []; var compile: angular.ICompileService = injector.get(NG1_COMPILE); var templateCache: angular.ITemplateCacheService = injector.get(NG1_TEMPLATE_CACHE); var httpBackend: angular.IHttpBackendService = injector.get(NG1_HTTP_BACKEND); @@ -176,7 +179,8 @@ export class UpgradeNg1ComponentAdapterBuilder { exportedComponent.directive = exportedComponent.extractDirective(injector); exportedComponent.$controller = $controller; exportedComponent.extractBindings(); - var promise = exportedComponent.compileTemplate(compile, templateCache, httpBackend); + var promise: Promise = + exportedComponent.compileTemplate(compile, templateCache, httpBackend); if (promise) promises.push(promise); } } diff --git a/modules/@angular/upgrade/test/upgrade_spec.ts b/modules/@angular/upgrade/test/upgrade_spec.ts index 75bac45b05..d03f9a1714 100644 --- a/modules/@angular/upgrade/test/upgrade_spec.ts +++ b/modules/@angular/upgrade/test/upgrade_spec.ts @@ -6,8 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {Class, Component, EventEmitter, Testability, disposePlatform, provide} from '@angular/core'; +import {Class, Component, EventEmitter, Inject, NgModule, OpaqueToken, Testability, disposePlatform} from '@angular/core'; +import {async} from '@angular/core/testing'; import {AsyncTestCompleter, ddescribe, describe, expect, iit, inject, it} from '@angular/core/testing/testing_internal'; +import {BrowserModule} from '@angular/platform-browser'; import {UpgradeAdapter} from '@angular/upgrade'; import * as angular from '@angular/upgrade/src/angular_js'; @@ -60,7 +62,6 @@ export function main() { async.done(); }); })); - describe('scope/component change-detection', () => { it('should interleave scope and component expressions', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { @@ -269,6 +270,41 @@ export function main() { async.done(); }); })); + + + // this test should be removed ones the deprecated @Components.directives prop is removed + // we should rewrite all of the tests in this file to use modules instead. + it('should downgrade ng2 component that is part of a module', async(() => { + const ng1Module = angular.module('ng1', []); + + ng1Module.component('ng1Component', {template: ''}); + + const SpecialValue = new OpaqueToken('special test value'); + + const Ng2Component = Component({ + selector: 'ng2-component', + template: 'test: {{value}}' + }).Class({ + constructor: [ + Inject(SpecialValue), function Ng2Component(value: number) { this.value = value; } + ] + }); + const Ng2AppModule = + NgModule({ + declarations: [Ng2Component], + imports: [BrowserModule], + providers: [{provide: SpecialValue, useValue: 23}] + }).Class({constructor: function Ng2AppModule() {}, ngDoBootstrap: function() {}}); + + const adapter = new UpgradeAdapter(Ng2AppModule); + ng1Module.directive('ng2Component', adapter.downgradeNg2Component(Ng2Component)); + var element = html(''); + adapter.bootstrap(element, ['ng1']).ready((ref) => { + expect(multiTrim(document.body.getElementsByTagName('ng2-component')[0].innerHTML)) + .toEqual('test: 23'); + ref.dispose(); + }); + })); }); describe('upgrade ng1 component', () => { diff --git a/modules/playground/src/upgrade/index.ts b/modules/playground/src/upgrade/index.ts index 980ea7fef0..8be1a3e2f6 100644 --- a/modules/playground/src/upgrade/index.ts +++ b/modules/playground/src/upgrade/index.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, EventEmitter, Input, NgModule, Output} from '@angular/core'; +import {Component, EventEmitter, Input, NgModule, Output, forwardRef} from '@angular/core'; +import {BrowserModule} from '@angular/platform-browser'; import {UpgradeAdapter} from '@angular/upgrade'; declare var angular: any; @@ -26,13 +27,12 @@ var styles = [` } `]; -var adapter: UpgradeAdapter = new UpgradeAdapter(Ng2AppModule); - +var adapter = new UpgradeAdapter(forwardRef(() => Ng2AppModule)); var ng1module = angular.module('myExample', []); ng1module.controller('Index', function($scope: any /** TODO #9100 */) { $scope.name = 'World'; }); -ng1module.directive('user', function() { +ng1module.directive('ng1User', function() { return { scope: {handle: '@', reset: '&'}, template: ` @@ -62,13 +62,12 @@ class Pane { - +
`, - styles: styles, - directives: [Pane, adapter.upgradeNg1Component('user')] + styles: styles }) class UpgradeApp { @Input() user: string; @@ -77,10 +76,12 @@ class UpgradeApp { } @NgModule({ - declarations: [Pane, UpgradeApp], - entryComponents: [UpgradeApp] + declarations: [Pane, UpgradeApp, adapter.upgradeNg1Component('ng1User')], + imports: [BrowserModule] }) -class Ng2AppModule {} +class Ng2AppModule { +} + ng1module.directive('upgradeApp', adapter.downgradeNg2Component(UpgradeApp)); diff --git a/tools/public_api_guard/upgrade/index.d.ts b/tools/public_api_guard/upgrade/index.d.ts index 054a819ea2..fca7a582b0 100644 --- a/tools/public_api_guard/upgrade/index.d.ts +++ b/tools/public_api_guard/upgrade/index.d.ts @@ -1,6 +1,7 @@ /** @experimental */ export declare class UpgradeAdapter { - addProvider(provider: Type | Provider | any[] | any): void; + constructor(ng2AppModule?: Type); + /** @deprecated */ addProvider(provider: Type | Provider | any[] | any): void; bootstrap(element: Element, modules?: any[], config?: angular.IAngularBootstrapConfig): UpgradeAdapterRef; downgradeNg2Component(type: Type): Function; downgradeNg2Provider(token: any): Function;