From f12d51992d25b1ebdf6bc73d8d109b1bce78afd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 17 Aug 2016 08:00:49 -0700 Subject: [PATCH] fix(animations): report errors for missing host-level referenced animations (#10650) Closes #10650 --- .../src/animation/animation_compiler.ts | 32 ++++++++++++++++--- .../animation/animation_integration_spec.ts | 25 +++++++++++++-- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/modules/@angular/compiler/src/animation/animation_compiler.ts b/modules/@angular/compiler/src/animation/animation_compiler.ts index 344fee298c..4786a98e3b 100644 --- a/modules/@angular/compiler/src/animation/animation_compiler.ts +++ b/modules/@angular/compiler/src/animation/animation_compiler.ts @@ -19,6 +19,8 @@ import * as t from '../template_parser/template_ast'; import {AnimationAst, AnimationAstVisitor, AnimationEntryAst, AnimationGroupAst, AnimationKeyframeAst, AnimationSequenceAst, AnimationStateAst, AnimationStateDeclarationAst, AnimationStateTransitionAst, AnimationStepAst, AnimationStylesAst} from './animation_ast'; import {AnimationParseError, ParsedAnimationResult, parseAnimationEntry} from './animation_parser'; +const animationCompilationCache = new Map(); + export class CompiledAnimation { constructor( public name: string, public statesMapStatement: o.Statement, @@ -68,6 +70,7 @@ export class AnimationCompiler { throw new BaseException(errorMessageStr); } + animationCompilationCache.set(component, compiledAnimations); return compiledAnimations; } } @@ -389,24 +392,43 @@ function _validateAnimationProperties( } class _AnimationTemplatePropertyVisitor implements t.TemplateAstVisitor { - private _animationRegistry: {[key: string]: boolean} = {}; - + private _animationRegistry: {[key: string]: boolean}; public errors: AnimationParseError[] = []; constructor(animations: CompiledAnimation[]) { - animations.forEach(entry => { this._animationRegistry[entry.name] = true; }); + this._animationRegistry = this._buildCompileAnimationLookup(animations); + } + + private _buildCompileAnimationLookup(animations: CompiledAnimation[]): {[key: string]: boolean} { + var map: {[key: string]: boolean} = {}; + animations.forEach(entry => { map[entry.name] = true; }); + return map; } visitElement(ast: t.ElementAst, ctx: any): any { - ast.inputs.forEach(input => { + var inputAsts: t.BoundElementPropertyAst[] = ast.inputs; + var componentAnimationRegistry = this._animationRegistry; + + var componentOnElement: t.DirectiveAst = + ast.directives.find(directive => directive.directive.isComponent); + if (componentOnElement) { + inputAsts = componentOnElement.hostProperties; + let cachedComponentAnimations = animationCompilationCache.get(componentOnElement.directive); + if (cachedComponentAnimations) { + componentAnimationRegistry = this._buildCompileAnimationLookup(cachedComponentAnimations); + } + } + + inputAsts.forEach(input => { if (input.type == t.PropertyBindingType.Animation) { var animationName = input.name; - if (!isPresent(this._animationRegistry[animationName])) { + if (!isPresent(componentAnimationRegistry[animationName])) { this.errors.push( new AnimationParseError(`couldn't find an animation entry for ${animationName}`)); } } }); + t.templateVisitAll(this, ast.children); } diff --git a/modules/@angular/core/test/animation/animation_integration_spec.ts b/modules/@angular/core/test/animation/animation_integration_spec.ts index 9e51948b3d..7c5f112932 100644 --- a/modules/@angular/core/test/animation/animation_integration_spec.ts +++ b/modules/@angular/core/test/animation/animation_integration_spec.ts @@ -19,7 +19,7 @@ import {AnimationPlayer} from '../../src/animation/animation_player'; import {AnimationStyles} from '../../src/animation/animation_styles'; import {AUTO_STYLE, AnimationEntryMetadata, animate, group, keyframes, sequence, state, style, transition, trigger} from '../../src/animation/metadata'; import {isArray, isPresent} from '../../src/facade/lang'; -import {TestBed, fakeAsync, flushMicrotasks, tick} from '../../testing'; +import {TestBed, async, fakeAsync, flushMicrotasks, tick} from '../../testing'; import {MockAnimationPlayer} from '../../testing/mock_animation_player'; import {AsyncTestCompleter, TestComponentBuilder, beforeEach, beforeEachProviders, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '../../testing/testing_internal'; @@ -32,8 +32,10 @@ function declareTests({useJit}: {useJit: boolean}) { describe('animation tests', function() { beforeEachProviders(() => { TestBed.configureCompiler({useJit: useJit}); - TestBed.configureTestingModule( - {providers: [{provide: AnimationDriver, useClass: MockAnimationDriver}]}); + TestBed.configureTestingModule({ + declarations: [DummyLoadingCmp, DummyIfCmp], + providers: [{provide: AnimationDriver, useClass: MockAnimationDriver}] + }); }); var makeAnimationCmp = @@ -1043,6 +1045,21 @@ function declareTests({useJit}: {useJit: boolean}) { tick(); }))); + + it('should throw an error if a host-level referenced animation is not defined within the component', + () => { + TestBed.overrideComponent(DummyLoadingCmp, {set: {animations: []}}); + + var failureMessage = ''; + try { + inject([AnimationDriver], (driver: AnimationDriver) => {})(); + } catch (e) { + failureMessage = e.message; + } + + expect(failureMessage).toMatch(/- couldn't find an animation entry for loading/); + }); + it('should retain the destination animation state styles once the animation is complete', inject( [TestComponentBuilder, AnimationDriver], @@ -1361,6 +1378,7 @@ class InnerContentTrackingAnimationPlayer extends MockAnimationPlayer { @Component({ selector: 'if-cmp', directives: [NgIf], + animations: [trigger('myAnimation', [])], template: `
` @@ -1375,6 +1393,7 @@ class DummyIfCmp { selector: 'if-cmp', host: {'[@loading]': 'exp'}, directives: [NgIf], + animations: [trigger('loading', [])], template: `
loading...
`