From 1426f680f53e9a570a1236843091475e4eed057f Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 7 Jul 2016 11:57:11 -0700 Subject: [PATCH] refactor(core): add a deprecation message for using `PLATFORM_DIRECTIVES` / `PLATFORM_PIPES` / `CompilerConfig` / `XHR` as regular providers in `bootstrap`. We still support this via a hack, but should remove this soon. This also fixes tests for module directives / pipes as they used directives / pipes that were already present in the underlying platform. --- .../integrationtest/src/module_fixtures.ts | 26 +++++--- .../integrationtest/test/app_module_spec.ts | 20 +++--- .../linker/app_module_integration_spec.ts | 53 +++++++++------- .../platform-browser-dynamic/core_private.ts | 1 + .../platform-browser-dynamic/index.ts | 31 +++++++++- .../test/browser/bootstrap_spec.ts | 62 ++++++++++++------- 6 files changed, 122 insertions(+), 71 deletions(-) diff --git a/modules/@angular/compiler-cli/integrationtest/src/module_fixtures.ts b/modules/@angular/compiler-cli/integrationtest/src/module_fixtures.ts index 0379b0df84..469055d056 100644 --- a/modules/@angular/compiler-cli/integrationtest/src/module_fixtures.ts +++ b/modules/@angular/compiler-cli/integrationtest/src/module_fixtures.ts @@ -7,7 +7,7 @@ */ import {LowerCasePipe, NgIf} from '@angular/common'; -import {ANALYZE_FOR_PRECOMPILE, AppModule, Component, ComponentFactoryResolver, Inject, Injectable, OpaqueToken} from '@angular/core'; +import {ANALYZE_FOR_PRECOMPILE, AppModule, Component, ComponentFactoryResolver, Directive, Inject, Injectable, Input, OpaqueToken, Pipe} from '@angular/core'; import {BrowserModule} from '@angular/platform-browser'; @Injectable() @@ -19,10 +19,18 @@ export class SomeService { export class NestedService { } -@Component({ - selector: 'cmp', - template: `
` -}) +@Directive({selector: '[someDir]', host: {'[title]': 'someDir'}}) +export class SomeDirective { + @Input() + someDir: string; +} + +@Pipe({name: 'somePipe'}) +export class SomePipe { + transform(value: string): any { return `transformed ${value}`; } +} + +@Component({selector: 'cmp', template: `
`}) export class SomeComp { constructor() {} } @@ -36,8 +44,8 @@ export class NestedModule { } @AppModule({ - directives: [NgIf], - pipes: [LowerCasePipe], + directives: [SomeDirective], + pipes: [SomePipe], providers: [SomeService], precompile: [SomeComp], modules: [NestedModule, BrowserModule] @@ -46,8 +54,8 @@ export class SomeModule { } @AppModule({ - directives: [NgIf], - pipes: [LowerCasePipe], + directives: [SomeDirective], + pipes: [SomePipe], precompile: [ParentComp], modules: [BrowserModule] }) diff --git a/modules/@angular/compiler-cli/integrationtest/test/app_module_spec.ts b/modules/@angular/compiler-cli/integrationtest/test/app_module_spec.ts index a965f3745b..21c646406a 100644 --- a/modules/@angular/compiler-cli/integrationtest/test/app_module_spec.ts +++ b/modules/@angular/compiler-cli/integrationtest/test/app_module_spec.ts @@ -37,25 +37,19 @@ describe('AppModule', () => { it('should support module directives and pipes', () => { var compFixture = createComponent(SomeComp, SomeModuleNgFactory); - var debugElement = compFixture.debugElement; - - // NgIf should work, is being used as module directive - expect(debugElement.children.length).toBe(1); compFixture.detectChanges(); - expect(debugElement.children.length).toBe(2); - expect(debugElement.children[0].properties['title']).toBe('hello'); + + var debugElement = compFixture.debugElement; + expect(debugElement.children[0].properties['title']).toBe('transformed someValue'); }); it('should support module directives and pipes on nested components', () => { var compFixture = createComponent(ParentComp, SomeModuleUsingParentCompNgFactory); - var debugElement = compFixture.debugElement; - - debugElement = debugElement.children[0]; - // NgIf should work, is being used as module directive - expect(debugElement.children.length).toBe(1); compFixture.detectChanges(); - expect(debugElement.children.length).toBe(2); - expect(debugElement.children[0].properties['title']).toBe('hello'); + + var debugElement = compFixture.debugElement; + debugElement = debugElement.children[0]; + expect(debugElement.children[0].properties['title']).toBe('transformed someValue'); }); it('should support child moduless', () => { diff --git a/modules/@angular/core/test/linker/app_module_integration_spec.ts b/modules/@angular/core/test/linker/app_module_integration_spec.ts index 5d3a551165..96e07b2771 100644 --- a/modules/@angular/core/test/linker/app_module_integration_spec.ts +++ b/modules/@angular/core/test/linker/app_module_integration_spec.ts @@ -1,6 +1,6 @@ import {LowerCasePipe, NgIf} from '@angular/common'; import {CompilerConfig} from '@angular/compiler'; -import {ANALYZE_FOR_PRECOMPILE, AppModule, AppModuleMetadata, Compiler, Component, ComponentFactoryResolver, ComponentRef, ComponentResolver, DebugElement, Host, Inject, Injectable, Injector, OpaqueToken, Optional, Provider, ReflectiveInjector, SelfMetadata, SkipSelf, SkipSelfMetadata, forwardRef, getDebugNode, provide} from '@angular/core'; +import {ANALYZE_FOR_PRECOMPILE, AppModule, AppModuleMetadata, Compiler, Component, ComponentFactoryResolver, ComponentRef, ComponentResolver, DebugElement, Directive, Host, Inject, Injectable, Injector, Input, OpaqueToken, Optional, Pipe, Provider, ReflectiveInjector, SelfMetadata, SkipSelf, SkipSelfMetadata, forwardRef, getDebugNode, provide} from '@angular/core'; import {ComponentFixture, configureCompiler} from '@angular/core/testing'; import {AsyncTestCompleter, beforeEach, beforeEachProviders, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '@angular/core/testing/testing_internal'; @@ -94,11 +94,18 @@ class ModuleWithPrecompile { class ModuleWithAnalyzePrecompileProvider { } +@Directive({selector: '[someDir]', host: {'[title]': 'someDir'}}) +class SomeDirective { + @Input() + someDir: string; +} -@Component({ - selector: 'comp', - template: `
` -}) +@Pipe({name: 'somePipe'}) +class SomePipe { + transform(value: string): any { return `transformed ${value}`; } +} + +@Component({selector: 'comp', template: `
`}) class CompUsingModuleDirectiveAndPipe { } @@ -108,7 +115,7 @@ class ParentCompUsingModuleDirectiveAndPipe { } @AppModule( - {directives: [NgIf], pipes: [LowerCasePipe], precompile: [CompUsingModuleDirectiveAndPipe]}) + {directives: [SomeDirective], pipes: [SomePipe], precompile: [CompUsingModuleDirectiveAndPipe]}) class ModuleWithDirectivesAndPipes { } @@ -179,36 +186,32 @@ function declareTests({useJit}: {useJit: boolean}) { return new ComponentFixture(cf.create(injector), null, false); } - function checkNgIfAndLowerCasePipe(compFixture: ComponentFixture, el: DebugElement) { - // Test that ngIf works - expect(el.children.length).toBe(1); - compFixture.detectChanges(); - expect(el.children.length).toBe(2); - - // Test that lowercase pipe works - expect(el.children[0].properties['title']).toBe('hello'); - } - it('should support module directives and pipes', () => { let compFixture = createComp(CompUsingModuleDirectiveAndPipe, ModuleWithDirectivesAndPipes); - checkNgIfAndLowerCasePipe(compFixture, compFixture.debugElement); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); }); it('should support module directives and pipes for nested modules', () => { let compFixture = createComp( CompUsingModuleDirectiveAndPipe, SomeModule, new AppModuleMetadata({modules: [ModuleWithDirectivesAndPipes]})); - checkNgIfAndLowerCasePipe(compFixture, compFixture.debugElement); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); }); it('should support module directives and pipes in nested components', () => { let compFixture = createComp(ParentCompUsingModuleDirectiveAndPipe, SomeModule, new AppModuleMetadata({ - directives: [NgIf], - pipes: [LowerCasePipe], + directives: [SomeDirective], + pipes: [SomePipe], precompile: [ParentCompUsingModuleDirectiveAndPipe] })); - checkNgIfAndLowerCasePipe(compFixture, compFixture.debugElement.children[0]); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].children[0].properties['title']) + .toBe('transformed someValue'); }); it('should provide a Compiler instance that uses the directives/pipes of the module', () => { @@ -216,7 +219,9 @@ function declareTests({useJit}: {useJit: boolean}) { let boundCompiler: Compiler = appModule.injector.get(Compiler); var cf = boundCompiler.compileComponentSync(CompUsingModuleDirectiveAndPipe); let compFixture = new ComponentFixture(cf.create(injector), null, false); - checkNgIfAndLowerCasePipe(compFixture, compFixture.debugElement); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); }); it('should provide a ComponentResolver instance that uses the directives/pipes of the module', @@ -226,7 +231,9 @@ function declareTests({useJit}: {useJit: boolean}) { let boundCompiler: ComponentResolver = appModule.injector.get(ComponentResolver); boundCompiler.resolveComponent(CompUsingModuleDirectiveAndPipe).then((cf) => { let compFixture = new ComponentFixture(cf.create(injector), null, false); - checkNgIfAndLowerCasePipe(compFixture, compFixture.debugElement); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); async.done(); }); })); diff --git a/modules/@angular/platform-browser-dynamic/core_private.ts b/modules/@angular/platform-browser-dynamic/core_private.ts index 3862d68543..34fa169aba 100644 --- a/modules/@angular/platform-browser-dynamic/core_private.ts +++ b/modules/@angular/platform-browser-dynamic/core_private.ts @@ -10,3 +10,4 @@ import {__core_private__ as r, __core_private_types__ as t} from '@angular/core' export var ReflectionCapabilities: typeof t.ReflectionCapabilities = r.ReflectionCapabilities; export var reflector: typeof t.reflector = r.reflector; +export var Console: typeof t.Console = r.Console; diff --git a/modules/@angular/platform-browser-dynamic/index.ts b/modules/@angular/platform-browser-dynamic/index.ts index 30301c6dfb..bf898f2e59 100644 --- a/modules/@angular/platform-browser-dynamic/index.ts +++ b/modules/@angular/platform-browser-dynamic/index.ts @@ -11,7 +11,7 @@ import {COMPILER_PROVIDERS, CompilerConfig, XHR} from '@angular/compiler'; import {AppModule, AppModuleRef, ApplicationRef, Compiler, ComponentRef, ComponentResolver, ExceptionHandler, PLATFORM_DIRECTIVES, PLATFORM_PIPES, ReflectiveInjector, Type, coreLoadAndBootstrap, isDevMode, lockRunMode} from '@angular/core'; import {BROWSER_APP_PROVIDERS, BrowserModule, WORKER_APP_APPLICATION_PROVIDERS, WORKER_SCRIPT, WORKER_UI_APPLICATION_PROVIDERS, bootstrapModuleFactory, browserPlatform, workerAppPlatform, workerUiPlatform} from '@angular/platform-browser'; -import {ReflectionCapabilities, reflector} from './core_private'; +import {Console, ReflectionCapabilities, reflector} from './core_private'; import {getDOM, initDomAdapter} from './platform_browser_private'; import {PromiseWrapper} from './src/facade/async'; import {ConcreteType, isPresent, stringify} from './src/facade/lang'; @@ -212,6 +212,7 @@ export function bootstrap( precompile = normalizeArray(customProvidersOrDynamicModule.precompile); compiler = customProvidersOrDynamicModule.compiler; } + let deprecationMessages: string[] = []; if (providers && providers.length > 0) { // Note: This is a hack to still support the old way // of configuring platform directives / pipes and the compiler xhr. @@ -223,20 +224,44 @@ export function bootstrap( // and provide a CompilerConfig out of it directives = directives.concat(compilerConfig.platformDirectives); pipes = pipes.concat(compilerConfig.platformPipes); + deprecationMessages.push( + `Passing a CompilerConfig to "bootstrap()" as provider is deprecated. Pass the provider to "createCompiler()" and call "bootstrap()" with the created compiler instead.`); } else { // If nobody provided a CompilerConfig, use the // PLATFORM_DIRECTIVES / PLATFORM_PIPES values directly. - directives = directives.concat(inj.get(PLATFORM_DIRECTIVES, [])); - pipes = pipes.concat(inj.get(PLATFORM_PIPES, [])); + let platformDirectives = inj.get(PLATFORM_DIRECTIVES, []); + if (platformDirectives.length > 0) { + deprecationMessages.push( + `Passing PLATFORM_DIRECTIVES to "bootstrap()" as provider is deprecated. Use the new parameter "directives" of "bootstrap()" instead.`); + } + directives = directives.concat(platformDirectives); + let platformPipes = inj.get(PLATFORM_PIPES, []); + if (platformPipes.length > 0) { + deprecationMessages.push( + `Passing PLATFORM_PIPES to "bootstrap()" as provider is deprecated. Use the new parameter "pipes" of "bootstrap()" instead.`); + } + pipes = pipes.concat(platformPipes); } let xhr = inj.get(XHR, null); if (xhr) { compilerProviders.push([{provide: XHR, useValue: xhr}]); + deprecationMessages.push( + `Passing an instance of XHR to "bootstrap()" as provider is deprecated. Pass the provider to "createCompiler()" and call "bootstrap()" with the created compiler instead.`); + } + // Need to copy console from providers to compiler + // as well so that we can test the above deprecation messages! + let console = inj.get(Console, null); + if (console) { + compilerProviders.push([{provide: Console, useValue: console}]); } } if (!compiler) { compiler = browserCompiler({providers: compilerProviders}); } + deprecationMessages.forEach((msg) => { + let console: Console = compiler.injector.get(Console); + console.warn(msg); + }); @AppModule({ providers: providers, diff --git a/modules/@angular/platform-browser/test/browser/bootstrap_spec.ts b/modules/@angular/platform-browser/test/browser/bootstrap_spec.ts index 6f73322f5e..932f0e02b0 100644 --- a/modules/@angular/platform-browser/test/browser/bootstrap_spec.ts +++ b/modules/@angular/platform-browser/test/browser/bootstrap_spec.ts @@ -8,7 +8,7 @@ import {LowerCasePipe, NgIf} from '@angular/common'; import {XHR} from '@angular/compiler'; -import {APP_INITIALIZER, Component, Directive, ExceptionHandler, Inject, OnDestroy, PLATFORM_DIRECTIVES, PLATFORM_INITIALIZER, PLATFORM_PIPES, ReflectiveInjector, coreLoadAndBootstrap, createPlatform, provide} from '@angular/core'; +import {APP_INITIALIZER, Component, Directive, ExceptionHandler, Inject, Input, OnDestroy, PLATFORM_DIRECTIVES, PLATFORM_INITIALIZER, PLATFORM_PIPES, Pipe, ReflectiveInjector, coreLoadAndBootstrap, createPlatform, provide} from '@angular/core'; import {ApplicationRef, disposePlatform} from '@angular/core/src/application_ref'; import {Console} from '@angular/core/src/console'; import {ComponentRef} from '@angular/core/src/linker/component_factory'; @@ -77,10 +77,18 @@ class HelloUrlCmp { greeting = 'hello'; } -@Component({ - selector: 'hello-app', - template: `
` -}) +@Directive({selector: '[someDir]', host: {'[title]': 'someDir'}}) +class SomeDirective { + @Input() + someDir: string; +} + +@Pipe({name: 'somePipe'}) +class SomePipe { + transform(value: string): any { return `transformed ${value}`; } +} + +@Component({selector: 'hello-app', template: `
`}) class HelloCmpUsingPlatformDirectiveAndPipe { show: boolean = false; } @@ -95,8 +103,10 @@ class _ArrayLogger { class DummyConsole implements Console { - log(message: any /** TODO #9100 */) {} - warn(message: any /** TODO #9100 */) {} + public warnings: string[] = []; + + log(message: string) {} + warn(message: string) { this.warnings.push(message); } } export function main() { @@ -104,6 +114,8 @@ export function main() { testProviders: any /** TODO #9100 */, lightDom: any /** TODO #9100 */; describe('bootstrap factory method', () => { + let compilerConsole: DummyConsole; + beforeEachProviders(() => { return [Log]; }); beforeEach(() => { @@ -117,8 +129,9 @@ export function main() { getDOM().appendChild(fakeDoc.body, el2); getDOM().appendChild(el, lightDom); getDOM().setText(lightDom, 'loading'); + compilerConsole = new DummyConsole(); testProviders = - [{provide: DOCUMENT, useValue: fakeDoc}, {provide: Console, useClass: DummyConsole}]; + [{provide: DOCUMENT, useValue: fakeDoc}, {provide: Console, useValue: compilerConsole}]; }); afterEach(disposePlatform); @@ -286,31 +299,34 @@ export function main() { it('should still allow to provide a custom xhr via the regular providers', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { let spyXhr: XHR = {get: (url: string) => Promise.resolve('{{greeting}} world!')}; - bootstrap(HelloUrlCmp, testProviders.concat([{provide: XHR, useValue: spyXhr}])) - .then((compRef) => { - expect(el).toHaveText('hello world!'); - async.done(); - }); + bootstrap(HelloUrlCmp, testProviders.concat([ + {provide: XHR, useValue: spyXhr} + ])).then((compRef) => { + expect(el).toHaveText('hello world!'); + expect(compilerConsole.warnings).toEqual([ + 'Passing an instance of XHR to "bootstrap()" as provider is deprecated. Pass the provider to "createCompiler()" and call "bootstrap()" with the created compiler instead.' + ]); + async.done(); + }); })); // Note: This will soon be deprecated as bootstrap creates a separate injector for the compiler, // i.e. such providers needs to go into that injecotr (when calling `browserCompiler`); it('should still allow to provide platform directives/pipes via the regular providers', - inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { + inject([Console, AsyncTestCompleter], (console: DummyConsole, async: AsyncTestCompleter) => { bootstrap(HelloCmpUsingPlatformDirectiveAndPipe, testProviders.concat([ - {provide: PLATFORM_DIRECTIVES, useValue: [NgIf]}, - {provide: PLATFORM_PIPES, useValue: [LowerCasePipe]} + {provide: PLATFORM_DIRECTIVES, useValue: [SomeDirective]}, + {provide: PLATFORM_PIPES, useValue: [SomePipe]} ])).then((compRef) => { let compFixture = new ComponentFixture(compRef, null, null); - let el = compFixture.debugElement; - // Test that ngIf works - expect(el.children.length).toBe(1); - compFixture.componentInstance.show = true; compFixture.detectChanges(); - expect(el.children.length).toBe(2); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); - // Test that lowercase pipe works - expect(el.children[0].properties['title']).toBe('hello'); + expect(compilerConsole.warnings).toEqual([ + 'Passing PLATFORM_DIRECTIVES to "bootstrap()" as provider is deprecated. Use the new parameter "directives" of "bootstrap()" instead.', + 'Passing PLATFORM_PIPES to "bootstrap()" as provider is deprecated. Use the new parameter "pipes" of "bootstrap()" instead.' + ]); async.done(); }); }));