refactor(animations): collect parser / lookup errors in the same place

This commit is contained in:
Matias Niemelä 2016-07-09 00:43:17 -07:00
parent 6d02d2f107
commit 79eda30f0f
6 changed files with 96 additions and 15 deletions

View File

@ -15,6 +15,7 @@ import {BaseException} from '../facade/exceptions';
import {isArray, isBlank, isPresent} from '../facade/lang'; import {isArray, isBlank, isPresent} from '../facade/lang';
import {Identifiers} from '../identifiers'; import {Identifiers} from '../identifiers';
import * as o from '../output/output_ast'; import * as o from '../output/output_ast';
import {PropertyBindingType, TemplateAst, TemplateAstVisitor, NgContentAst, EmbeddedTemplateAst, ElementAst, ReferenceAst, VariableAst, BoundEventAst, BoundElementPropertyAst, AttrAst, BoundTextAst, TextAst, DirectiveAst, BoundDirectivePropertyAst, templateVisitAll,} from '../template_ast';
import {AnimationAst, AnimationAstVisitor, AnimationEntryAst, AnimationGroupAst, AnimationKeyframeAst, AnimationSequenceAst, AnimationStateAst, AnimationStateDeclarationAst, AnimationStateTransitionAst, AnimationStepAst, AnimationStylesAst} from './animation_ast'; import {AnimationAst, AnimationAstVisitor, AnimationEntryAst, AnimationGroupAst, AnimationKeyframeAst, AnimationSequenceAst, AnimationStateAst, AnimationStateDeclarationAst, AnimationStateTransitionAst, AnimationStepAst, AnimationStylesAst} from './animation_ast';
import {AnimationParseError, ParsedAnimationResult, parseAnimationEntry} from './animation_parser'; import {AnimationParseError, ParsedAnimationResult, parseAnimationEntry} from './animation_parser';
@ -27,19 +28,21 @@ export class CompiledAnimation {
} }
export class AnimationCompiler { export class AnimationCompiler {
compileComponent(component: CompileDirectiveMetadata): CompiledAnimation[] { compileComponent(component: CompileDirectiveMetadata, template: TemplateAst[]):
CompiledAnimation[] {
var compiledAnimations: CompiledAnimation[] = []; var compiledAnimations: CompiledAnimation[] = [];
var index = 0; var index = 0;
var groupedErrors: string[] = [];
component.template.animations.forEach(entry => { component.template.animations.forEach(entry => {
var result = parseAnimationEntry(entry); var result = parseAnimationEntry(entry);
if (result.errors.length > 0) { if (result.errors.length > 0) {
var errorMessage = ''; var errorMessage =
`Unable to parse the animation sequence for "${entry.name}" due to the following errors:`;
result.errors.forEach( result.errors.forEach(
(error: AnimationParseError) => { errorMessage += '\n- ' + error.msg; }); (error: AnimationParseError) => { errorMessage += '\n-- ' + error.msg; });
// todo (matsko): include the component name when throwing // todo (matsko): include the component name when throwing
throw new BaseException( groupedErrors.push(errorMessage);
`Unable to parse the animation sequence for "${entry.name}" due to the following errors: ` +
errorMessage);
} }
var factoryName = `${component.type.name}_${entry.name}_${index}`; var factoryName = `${component.type.name}_${entry.name}_${index}`;
@ -48,6 +51,18 @@ export class AnimationCompiler {
var visitor = new _AnimationBuilder(entry.name, factoryName); var visitor = new _AnimationBuilder(entry.name, factoryName);
compiledAnimations.push(visitor.build(result.ast)); compiledAnimations.push(visitor.build(result.ast));
}); });
_validateAnimationProperties(compiledAnimations, template).forEach(entry => {
groupedErrors.push(entry.msg);
});
if (groupedErrors.length > 0) {
var errorMessageStr =
`Animation parsing for ${component.type.name} has failed due to the following errors:`;
groupedErrors.forEach(error => errorMessageStr += `\n- ${error}`);
throw new BaseException(errorMessageStr);
}
return compiledAnimations; return compiledAnimations;
} }
} }
@ -360,3 +375,49 @@ function _isEndStateAnimateStep(step: AnimationAst): boolean {
function _getStylesArray(obj: any): {[key: string]: any}[] { function _getStylesArray(obj: any): {[key: string]: any}[] {
return obj.styles.styles; return obj.styles.styles;
} }
function _validateAnimationProperties(
compiledAnimations: CompiledAnimation[], template: TemplateAst[]): AnimationParseError[] {
var visitor = new _AnimationTemplatePropertyVisitor(compiledAnimations);
templateVisitAll(visitor, template);
return visitor.errors;
}
class _AnimationTemplatePropertyVisitor implements TemplateAstVisitor {
private _nodeIndex: number = 0;
private _animationRegistry: {[key: string]: boolean} = {};
public errors: AnimationParseError[] = [];
constructor(animations: CompiledAnimation[]) {
animations.forEach(entry => { this._animationRegistry[entry.name] = true; });
}
visitElement(ast: ElementAst, ctx: any): any {
ast.inputs.forEach(input => {
if (input.type == PropertyBindingType.Animation) {
var animationName = input.name;
if (!isPresent(this._animationRegistry[animationName])) {
this.errors.push(
new AnimationParseError(`couldn't find an animation entry for ${animationName}`));
}
}
});
templateVisitAll(this, ast.children);
}
visitBoundText(ast: BoundTextAst, ctx: any): any { this._nodeIndex++; }
visitText(ast: TextAst, ctx: any): any { this._nodeIndex++; }
visitEmbeddedTemplate(ast: EmbeddedTemplateAst, ctx: any): any { this._nodeIndex++; }
visitNgContent(ast: NgContentAst, ctx: any): any {}
visitAttr(ast: AttrAst, ctx: any): any {}
visitDirective(ast: DirectiveAst, ctx: any): any {}
visitEvent(ast: BoundEventAst, ctx: any): any {}
visitReference(ast: ReferenceAst, ctx: any): any {}
visitVariable(ast: VariableAst, ctx: any): any {}
visitDirectiveProperty(ast: BoundDirectivePropertyAst, ctx: any): any {}
visitElementProperty(ast: BoundElementPropertyAst, ctx: any): any {}
}

View File

@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {BaseException, SecurityContext} from '@angular/core';
import {EMPTY_STATE as EMPTY_ANIMATION_STATE, LifecycleHooks, isDefaultChangeDetectionStrategy} from '../../core_private'; import {EMPTY_STATE as EMPTY_ANIMATION_STATE, LifecycleHooks, isDefaultChangeDetectionStrategy} from '../../core_private';
import * as cdAst from '../expression_parser/ast'; import * as cdAst from '../expression_parser/ast';
import {isBlank, isPresent} from '../facade/lang'; import {isBlank, isPresent} from '../facade/lang';
@ -136,10 +138,6 @@ function bindAndWriteToRenderer(
case PropertyBindingType.Animation: case PropertyBindingType.Animation:
var animationName = boundProp.name; var animationName = boundProp.name;
var animation = view.componentView.animations.get(animationName); var animation = view.componentView.animations.get(animationName);
if (!isPresent(animation)) {
throw new BaseException(
`Internal Error: couldn't find an animation entry for ${boundProp.name}`);
}
// it's important to normalize the void value as `void` explicitly // it's important to normalize the void value as `void` explicitly
// so that the styles data can be obtained from the stringmap // so that the styles data can be obtained from the stringmap

View File

@ -275,7 +275,7 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
ast.hasViewContainer, true, ast.references); ast.hasViewContainer, true, ast.references);
this.view.nodes.push(compileElement); this.view.nodes.push(compileElement);
var compiledAnimations = this._animationCompiler.compileComponent(this.view.component); var compiledAnimations = this._animationCompiler.compileComponent(this.view.component, [ast]);
this.nestedViewCount++; this.nestedViewCount++;
var embeddedView = new CompileView( var embeddedView = new CompileView(

View File

@ -36,7 +36,7 @@ export class ViewCompiler {
component: CompileDirectiveMetadata, template: TemplateAst[], styles: o.Expression, component: CompileDirectiveMetadata, template: TemplateAst[], styles: o.Expression,
pipes: CompilePipeMetadata[]): ViewCompileResult { pipes: CompilePipeMetadata[]): ViewCompileResult {
var dependencies: Array<ViewFactoryDependency|ComponentFactoryDependency> = []; var dependencies: Array<ViewFactoryDependency|ComponentFactoryDependency> = [];
var compiledAnimations = this._animationCompiler.compileComponent(component); var compiledAnimations = this._animationCompiler.compileComponent(component, template);
var statements: o.Statement[] = []; var statements: o.Statement[] = [];
compiledAnimations.map(entry => { compiledAnimations.map(entry => {
statements.push(entry.statesMapStatement); statements.push(entry.statesMapStatement);

View File

@ -22,7 +22,7 @@ export function main() {
var compiler = new AnimationCompiler(); var compiler = new AnimationCompiler();
var compileAnimations = (component: CompileDirectiveMetadata): CompiledAnimation => { var compileAnimations = (component: CompileDirectiveMetadata): CompiledAnimation => {
return compiler.compileComponent(component)[0]; return compiler.compileComponent(component, [])[0];
}; };
var compile = (seq: AnimationMetadata) => { var compile = (seq: AnimationMetadata) => {

View File

@ -13,6 +13,7 @@ import {AnimationDriver} from '@angular/platform-browser/src/dom/animation_drive
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
import {MockAnimationDriver} from '@angular/platform-browser/testing/mock_animation_driver'; import {MockAnimationDriver} from '@angular/platform-browser/testing/mock_animation_driver';
import {BaseException} from '../../../compiler/src/facade/exceptions';
import {Component} from '../../index'; import {Component} from '../../index';
import {DEFAULT_STATE} from '../../src/animation/animation_constants'; import {DEFAULT_STATE} from '../../src/animation/animation_constants';
import {AnimationKeyframe} from '../../src/animation/animation_keyframe'; import {AnimationKeyframe} from '../../src/animation/animation_keyframe';
@ -44,12 +45,15 @@ function declareTests({useJit}: {useJit: boolean}) {
var makeAnimationCmp = var makeAnimationCmp =
(tcb: TestComponentBuilder, tpl: string, (tcb: TestComponentBuilder, tpl: string,
animationEntry: AnimationEntryMetadata | AnimationEntryMetadata[], animationEntry: AnimationEntryMetadata | AnimationEntryMetadata[],
callback: any /** TODO #9100 */ = null) => { callback: Function = null, failure: Function = null) => {
var entries = isArray(animationEntry) ? <AnimationEntryMetadata[]>animationEntry : var entries = isArray(animationEntry) ? <AnimationEntryMetadata[]>animationEntry :
[<AnimationEntryMetadata>animationEntry]; [<AnimationEntryMetadata>animationEntry];
tcb = tcb.overrideTemplate(DummyIfCmp, tpl); tcb = tcb.overrideTemplate(DummyIfCmp, tpl);
tcb = tcb.overrideAnimations(DummyIfCmp, entries); tcb = tcb.overrideAnimations(DummyIfCmp, entries);
tcb.createAsync(DummyIfCmp).then((root) => { callback(root); }); var promise = tcb.createAsync(DummyIfCmp).then((root) => { callback(root); });
if (isPresent(failure)) {
promise.catch(<any>failure);
}
tick(); tick();
}; };
@ -878,6 +882,24 @@ function declareTests({useJit}: {useJit: boolean}) {
}); });
describe('animation states', () => { describe('animation states', () => {
it('should throw an error when an animation is referenced that isn\'t defined within the component annotation',
inject(
[TestComponentBuilder, AnimationDriver],
fakeAsync((tcb: TestComponentBuilder, driver: MockAnimationDriver) => {
makeAnimationCmp(
tcb, '<div class="target" [@status]="exp"></div>', [],
() => {
throw new BaseException(
'Error: expected animations for DummyIfCmp to throw an error within this spec');
},
(e: any) => {
var message = e.message;
expect(message).toMatch(
/Animation parsing for DummyIfCmp has failed due to the following errors:/);
expect(message).toMatch(/- couldn't find an animation entry for status/);
});
})));
it('should retain the destination animation state styles once the animation is complete', it('should retain the destination animation state styles once the animation is complete',
inject( inject(
[TestComponentBuilder, AnimationDriver], [TestComponentBuilder, AnimationDriver],