From 3651d8d6738c319705ee0e68ec8520ff5cc7cf9e Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 6 Mar 2017 17:15:08 -0800 Subject: [PATCH] fix: throw for synthetic properties / listeners by default (#14880) This allows to detect the case that the animation module is not loaded. --- .../integrationtest/src/module.ts | 24 ++++++++++-- .../animation/animation_integration_spec.ts | 38 ++++++++++++++++++- .../examples/_common/system-config.ts | 2 +- .../animation/ts/dsl/animation_example.ts | 5 ++- .../platform-browser/src/dom/dom_renderer.ts | 14 ++++++- .../platform-server/src/private_export.ts | 1 + .../platform-server/src/server_renderer.ts | 10 +++++ 7 files changed, 85 insertions(+), 9 deletions(-) diff --git a/modules/@angular/compiler-cli/integrationtest/src/module.ts b/modules/@angular/compiler-cli/integrationtest/src/module.ts index 2d1b6d94e3..9506dda314 100644 --- a/modules/@angular/compiler-cli/integrationtest/src/module.ts +++ b/modules/@angular/compiler-cli/integrationtest/src/module.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {ApplicationRef, NgModule} from '@angular/core'; +import {ApplicationRef, NgModule, NgZone, Provider, RendererFactoryV2} from '@angular/core'; import {FormsModule} from '@angular/forms'; -import {NoopAnimationsModule} from '@angular/platform-browser/animations'; -import {ServerModule} from '@angular/platform-server'; +import {NoopAnimationsModule, ɵAnimationEngine, ɵAnimationRendererFactory} from '@angular/platform-browser/animations'; +import {ServerModule, ɵServerRendererFactoryV2} from '@angular/platform-server'; import {MdButtonModule} from '@angular2-material/button'; // Note: don't refer to third_party_src as we want to test that // we can compile components from node_modules! @@ -25,6 +25,19 @@ import {CompUsingRootModuleDirectiveAndPipe, SomeDirectiveInRootModule, SomeLibM import {CompWithNgContent, ProjectingComp} from './projection'; import {CompForChildQuery, CompWithChildQuery, CompWithDirectiveChild, DirectiveForQuery} from './queries'; +export function instantiateServerRendererFactory( + renderer: RendererFactoryV2, engine: ɵAnimationEngine, zone: NgZone) { + return new ɵAnimationRendererFactory(renderer, engine, zone); +} + +// TODO(matsko): create a server module for animations and use +// that instead of these manual providers here. +export const SERVER_ANIMATIONS_PROVIDERS: Provider[] = [{ + provide: RendererFactoryV2, + useFactory: instantiateServerRendererFactory, + deps: [ɵServerRendererFactoryV2, ɵAnimationEngine, NgZone] +}]; + @NgModule({ declarations: [ AnimateCmp, @@ -58,7 +71,10 @@ import {CompForChildQuery, CompWithChildQuery, CompWithDirectiveChild, Directive SomeLibModule.withProviders(), ThirdpartyModule, ], - providers: [SomeService], + providers: [ + SomeService, + SERVER_ANIMATIONS_PROVIDERS, + ], entryComponents: [ AnimateCmp, BasicComp, diff --git a/modules/@angular/core/test/animation/animation_integration_spec.ts b/modules/@angular/core/test/animation/animation_integration_spec.ts index 52bcdbe64d..281f0ed828 100644 --- a/modules/@angular/core/test/animation/animation_integration_spec.ts +++ b/modules/@angular/core/test/animation/animation_integration_spec.ts @@ -6,10 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ import {AUTO_STYLE, AnimationEvent, animate, keyframes, state, style, transition, trigger} from '@angular/animations'; -import {Component, HostBinding, HostListener, ViewChild} from '@angular/core'; +import {Component, HostBinding, HostListener, RendererFactoryV2, ViewChild} from '@angular/core'; +import {ɵDomRendererFactoryV2} from '@angular/platform-browser'; import {AnimationDriver, BrowserAnimationsModule, ɵAnimationEngine} from '@angular/platform-browser/animations'; import {MockAnimationDriver, MockAnimationPlayer} from '@angular/platform-browser/animations/testing'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; + import {TestBed} from '../../testing'; export function main() { @@ -549,6 +551,40 @@ export function main() { expect(cmp.event.toState).toEqual('TRUE'); }); }); + + describe('errors for not using the animation module', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [{provide: RendererFactoryV2, useExisting: ɵDomRendererFactoryV2}], + }); + }); + + it('should throw when using an @prop binding without the animation module', () => { + @Component({template: `
`}) + class Cmp { + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const comp = TestBed.createComponent(Cmp); + expect(() => comp.detectChanges()) + .toThrowError( + 'Found the synthetic property @myAnimation. Please include either "BrowserAnimationsModule" or "NoopAnimationsModule" in your application.'); + }); + + it('should throw when using an @prop listener without the animation module', () => { + @Component({template: `
`}) + class Cmp { + a: any; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + expect(() => TestBed.createComponent(Cmp)) + .toThrowError( + 'Found the synthetic listener @myAnimation.start. Please include either "BrowserAnimationsModule" or "NoopAnimationsModule" in your application.'); + + }); + }); }); } diff --git a/modules/@angular/examples/_common/system-config.ts b/modules/@angular/examples/_common/system-config.ts index 7bfa87e5b1..d99a44038c 100644 --- a/modules/@angular/examples/_common/system-config.ts +++ b/modules/@angular/examples/_common/system-config.ts @@ -12,7 +12,7 @@ System.config({ '@angular/compiler': '/vendor/@angular/compiler/bundles/compiler.umd.js', '@angular/animations': '/vendor/@angular/animations/bundles/animations.umd.js', '@angular/platform-browser/animations': - '/vendor/@angular/platform-browser/animations/bundles/platform-browser-animations.umd.js', + '/vendor/@angular/platform-browser/bundles/platform-browser-animations.umd.js', '@angular/core': '/vendor/@angular/core/bundles/core.umd.js', '@angular/forms': '/vendor/@angular/forms/bundles/forms.umd.js', '@angular/http': '/vendor/@angular/forms/bundles/http.umd.js', diff --git a/modules/@angular/examples/core/animation/ts/dsl/animation_example.ts b/modules/@angular/examples/core/animation/ts/dsl/animation_example.ts index 91a71c6d86..d0cec6075b 100644 --- a/modules/@angular/examples/core/animation/ts/dsl/animation_example.ts +++ b/modules/@angular/examples/core/animation/ts/dsl/animation_example.ts @@ -8,7 +8,7 @@ // #docregion Component import {Component, NgModule, animate, state, style, transition, trigger} from '@angular/core'; -import {BrowserModule} from '@angular/platform-browser'; +import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; @Component({ selector: 'example-app', @@ -48,7 +48,8 @@ export class MyExpandoCmp { collapse() { this.stateExpression = 'collapsed'; } } -@NgModule({imports: [BrowserModule], declarations: [MyExpandoCmp], bootstrap: [MyExpandoCmp]}) +@NgModule( + {imports: [BrowserAnimationsModule], declarations: [MyExpandoCmp], bootstrap: [MyExpandoCmp]}) export class AppModule { } diff --git a/modules/@angular/platform-browser/src/dom/dom_renderer.ts b/modules/@angular/platform-browser/src/dom/dom_renderer.ts index 814df0df54..ad5640c101 100644 --- a/modules/@angular/platform-browser/src/dom/dom_renderer.ts +++ b/modules/@angular/platform-browser/src/dom/dom_renderer.ts @@ -182,12 +182,16 @@ class DefaultDomRendererV2 implements RendererV2 { } } - setProperty(el: any, name: string, value: any): void { el[name] = value; } + setProperty(el: any, name: string, value: any): void { + checkNoSyntheticProp(name, 'property'); + el[name] = value; + } setValue(node: any, value: string): void { node.nodeValue = value; } listen(target: 'window'|'document'|'body'|any, event: string, callback: (event: any) => boolean): () => void { + checkNoSyntheticProp(event, 'listener'); if (typeof target === 'string') { return <() => void>this.eventManager.addGlobalEventListener( target, event, decoratePreventDefault(callback)); @@ -197,6 +201,14 @@ class DefaultDomRendererV2 implements RendererV2 { } } +const AT_CHARCODE = '@'.charCodeAt(0); +function checkNoSyntheticProp(name: string, nameKind: string) { + if (name.charCodeAt(0) === AT_CHARCODE) { + throw new Error( + `Found the synthetic ${nameKind} ${name}. Please include either "BrowserAnimationsModule" or "NoopAnimationsModule" in your application.`); + } +} + class EmulatedEncapsulationDomRendererV2 extends DefaultDomRendererV2 { private contentAttr: string; private hostAttr: string; diff --git a/modules/@angular/platform-server/src/private_export.ts b/modules/@angular/platform-server/src/private_export.ts index e4ba93a683..ac4f7ccd41 100644 --- a/modules/@angular/platform-server/src/private_export.ts +++ b/modules/@angular/platform-server/src/private_export.ts @@ -8,3 +8,4 @@ export {INTERNAL_SERVER_PLATFORM_PROVIDERS as ɵINTERNAL_SERVER_PLATFORM_PROVIDERS, SERVER_RENDER_PROVIDERS as ɵSERVER_RENDER_PROVIDERS} from './server'; +export {ServerRendererFactoryV2 as ɵServerRendererFactoryV2} from './server_renderer'; \ No newline at end of file diff --git a/modules/@angular/platform-server/src/server_renderer.ts b/modules/@angular/platform-server/src/server_renderer.ts index b3d076fe45..45db70746a 100644 --- a/modules/@angular/platform-server/src/server_renderer.ts +++ b/modules/@angular/platform-server/src/server_renderer.ts @@ -146,6 +146,7 @@ class DefaultServerRendererV2 implements RendererV2 { } setProperty(el: any, name: string, value: any): void { + checkNoSyntheticProp(name, 'property'); getDOM().setProperty(el, name, value); // Mirror property values for known HTML element properties in the attributes. const tagName = (el.tagName as string).toLowerCase(); @@ -164,6 +165,7 @@ class DefaultServerRendererV2 implements RendererV2 { callback: (event: any) => boolean): () => void { // Note: We are not using the EventsPlugin here as this is not needed // to run our tests. + checkNoSyntheticProp(eventName, 'listener'); const el = typeof target === 'string' ? getDOM().getGlobalEventTarget(this.document, target) : target; const outsideHandler = (event: any) => this.ngZone.runGuarded(() => callback(event)); @@ -171,6 +173,14 @@ class DefaultServerRendererV2 implements RendererV2 { } } +const AT_CHARCODE = '@'.charCodeAt(0); +function checkNoSyntheticProp(name: string, nameKind: string) { + if (name.charCodeAt(0) === AT_CHARCODE) { + throw new Error( + `Found the synthetic ${nameKind} ${name}. Please include either "BrowserAnimationsModule" or "NoopAnimationsModule" in your application.`); + } +} + class EmulatedEncapsulationServerRendererV2 extends DefaultServerRendererV2 { private contentAttr: string; private hostAttr: string;