Skip to content

Commit

Permalink
refactor(animations): collect parser / lookup errors in the same place
Browse files Browse the repository at this point in the history
  • Loading branch information
matsko committed Jul 11, 2016
1 parent 57473e7 commit d266def
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 26 deletions.
73 changes: 67 additions & 6 deletions modules/@angular/compiler/src/animation/animation_compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {BaseException} from '../facade/exceptions';
import {isArray, isBlank, isPresent} from '../facade/lang';
import {Identifiers} from '../identifiers';
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 {AnimationParseError, ParsedAnimationResult, parseAnimationEntry} from './animation_parser';
Expand All @@ -27,19 +28,21 @@ export class CompiledAnimation {
}

export class AnimationCompiler {
compileComponent(component: CompileDirectiveMetadata): CompiledAnimation[] {
compileComponent(component: CompileDirectiveMetadata, template: TemplateAst[]):
CompiledAnimation[] {
var compiledAnimations: CompiledAnimation[] = [];
var index = 0;
var groupedErrors: string[] = [];

component.template.animations.forEach(entry => {
var result = parseAnimationEntry(entry);
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(
(error: AnimationParseError) => { errorMessage += '\n- ' + error.msg; });
(error: AnimationParseError) => { errorMessage += '\n-- ' + error.msg; });
// todo (matsko): include the component name when throwing
throw new BaseException(
`Unable to parse the animation sequence for "${entry.name}" due to the following errors: ` +
errorMessage);
groupedErrors.push(errorMessage);
}

var factoryName = `${component.type.name}_${entry.name}_${index}`;
Expand All @@ -48,6 +51,18 @@ export class AnimationCompiler {
var visitor = new _AnimationBuilder(entry.name, factoryName);
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;
}
}
Expand Down Expand Up @@ -360,3 +375,49 @@ function _isEndStateAnimateStep(step: AnimationAst): boolean {
function _getStylesArray(obj: any): {[key: string]: any}[] {
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 {}
}
22 changes: 7 additions & 15 deletions modules/@angular/compiler/src/view_compiler/property_binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,23 @@
* 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 * as cdAst from '../expression_parser/ast';
import {isBlank, isPresent} from '../facade/lang';
import {Identifiers} from '../identifiers';
import * as o from '../output/output_ast';
import {BoundElementPropertyAst, BoundTextAst, DirectiveAst, PropertyBindingType} from '../template_ast';
import {camelCaseToDashCase} from '../util';

import {DetectChangesVars, ViewProperties} from './constants';

import {BoundTextAst, BoundElementPropertyAst, DirectiveAst, PropertyBindingType,} from '../template_ast';

import {CompileView} from './compile_view';
import {CompileBinding} from './compile_binding';
import {CompileElement, CompileNode} from './compile_element';
import {CompileMethod} from './compile_method';

import {camelCaseToDashCase} from '../util';

import {CompileView} from './compile_view';
import {DetectChangesVars, ViewProperties} from './constants';
import {convertCdExpressionToIr} from './expression_converter';

import {CompileBinding} from './compile_binding';
import {BaseException, SecurityContext} from '@angular/core';


function createBindFieldExpr(exprIndex: number): o.ReadPropExpr {
return o.THIS_EXPR.prop(`_expr_${exprIndex}`);
Expand Down Expand Up @@ -142,10 +138,6 @@ function bindAndWriteToRenderer(
case PropertyBindingType.Animation:
var animationName = boundProp.name;
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
// so that the styles data can be obtained from the stringmap
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
ast.hasViewContainer, true, ast.references);
this.view.nodes.push(compileElement);

var compiledAnimations = this._animationCompiler.compileComponent(this.view.component);
var compiledAnimations = this._animationCompiler.compileComponent(this.view.component, [ast]);

this.nestedViewCount++;
var embeddedView = new CompileView(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class ViewCompiler {
component: CompileDirectiveMetadata, template: TemplateAst[], styles: o.Expression,
pipes: CompilePipeMetadata[]): ViewCompileResult {
var dependencies: Array<ViewFactoryDependency|ComponentFactoryDependency> = [];
var compiledAnimations = this._animationCompiler.compileComponent(component);
var compiledAnimations = this._animationCompiler.compileComponent(component, template);
var statements: o.Statement[] = [];
compiledAnimations.map(entry => {
statements.push(entry.statesMapStatement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function main() {
var compiler = new AnimationCompiler();

var compileAnimations = (component: CompileDirectiveMetadata): CompiledAnimation => {
return compiler.compileComponent(component)[0];
return compiler.compileComponent(component, [])[0];
};

var compile = (seq: AnimationMetadata) => {
Expand Down
20 changes: 18 additions & 2 deletions modules/@angular/core/test/animation/animation_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,15 @@ function declareTests({useJit}: {useJit: boolean}) {
var makeAnimationCmp =
(tcb: TestComponentBuilder, tpl: string,
animationEntry: AnimationEntryMetadata | AnimationEntryMetadata[],
callback: any /** TODO #9100 */ = null) => {
callback: Function = null, failure: Function = null) => {
var entries = isArray(animationEntry) ? <AnimationEntryMetadata[]>animationEntry :
[<AnimationEntryMetadata>animationEntry];
tcb = tcb.overrideTemplate(DummyIfCmp, tpl);
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();
};

Expand Down Expand Up @@ -878,6 +881,19 @@ function declareTests({useJit}: {useJit: boolean}) {
});

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>', [], () => {}, (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',
inject(
[TestComponentBuilder, AnimationDriver],
Expand Down

0 comments on commit d266def

Please sign in to comment.