fix(animations): throw errors when duplicate component trigger names …

…are registered
angular · Jul 12, 2016 · 5af1e89 · 5af1e89
1 parent 79eda30
commit 5af1e89
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 13 deletions.
diff --git a/modules/@angular/compiler/src/animation/animation_compiler.ts b/modules/@angular/compiler/src/animation/animation_compiler.ts
@@ -33,23 +33,33 @@ export class AnimationCompiler {
     var compiledAnimations: CompiledAnimation[] = [];
     var index = 0;
     var groupedErrors: string[] = [];
+    var triggerLookup: {[key: string]: CompiledAnimation} = {};
+    var componentName = component.type.name;
 
     component.template.animations.forEach(entry => {
       var result = parseAnimationEntry(entry);
+      var triggerName = entry.name;
       if (result.errors.length > 0) {
         var errorMessage =
-            `Unable to parse the animation sequence for "${entry.name}" due to the following errors:`;
+            `Unable to parse the animation sequence for "${triggerName}" due to the following errors:`;
         result.errors.forEach(
             (error: AnimationParseError) => { errorMessage += '\n-- ' + error.msg; });
         // todo (matsko): include the component name when throwing
         groupedErrors.push(errorMessage);
       }
 
-      var factoryName = `${component.type.name}_${entry.name}_${index}`;
-      index++;
-
-      var visitor = new _AnimationBuilder(entry.name, factoryName);
-      compiledAnimations.push(visitor.build(result.ast));
+      if (triggerLookup[triggerName]) {
+        groupedErrors.push(
+            `The animation trigger "${triggerName}" has already been registered on "${componentName}"`);
+      } else {
+        var factoryName = `${component.type.name}_${entry.name}_${index}`;
+        index++;
+
+        var visitor = new _AnimationBuilder(triggerName, factoryName);
+        var compileResult = visitor.build(result.ast)
+        compiledAnimations.push(compileResult);
+        triggerLookup[entry.name] = compileResult;
+      }
     });
 
     _validateAnimationProperties(compiledAnimations, template).forEach(entry => {

diff --git a/modules/@angular/compiler/test/animation/animation_compiler_spec.ts b/modules/@angular/compiler/test/animation/animation_compiler_spec.ts
@@ -9,8 +9,9 @@
 import {AnimationMetadata, animate, group, sequence, style, transition, trigger} from '@angular/core';
 import {AsyncTestCompleter, beforeEach, beforeEachProviders, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '@angular/core/testing/testing_internal';
 
+import {StringMapWrapper} from '../../../platform-browser-dynamic/src/facade/collection';
 import {AnimationCompiler, CompiledAnimation} from '../../src/animation/animation_compiler';
-import {CompileDirectiveMetadata, CompileTemplateMetadata, CompileTypeMetadata} from '../../src/compile_metadata';
+import {CompileAnimationEntryMetadata, CompileDirectiveMetadata, CompileTemplateMetadata, CompileTypeMetadata} from '../../src/compile_metadata';
 import {CompileMetadataResolver} from '../../src/metadata_resolver';
 
 export function main() {
@@ -25,18 +26,24 @@ export function main() {
       return compiler.compileComponent(component, [])[0];
     };
 
-    var compile = (seq: AnimationMetadata) => {
-      var entry = trigger('myAnimation', [transition('state1 => state2', seq)]);
+    var compileTriggers = (input: any[]) => {
+      var entries: CompileAnimationEntryMetadata[] = input.map(entry => {
+        var animationTriggerData = trigger(entry[0], entry[1]);
+        return resolver.getAnimationEntryMetadata(animationTriggerData);
+      });
 
-      var compiledAnimationEntry = resolver.getAnimationEntryMetadata(entry);
       var component = CompileDirectiveMetadata.create({
-        type: new CompileTypeMetadata({name: 'something'}),
-        template: new CompileTemplateMetadata({animations: [compiledAnimationEntry]})
+        type: new CompileTypeMetadata({name: 'myCmp'}),
+        template: new CompileTemplateMetadata({animations: entries})
       });
 
       return compileAnimations(component);
     };
 
+    var compileSequence = (seq: AnimationMetadata) => {
+      return compileTriggers([['myAnimation', [transition('state1 => state2', seq)]]]);
+    };
+
     it('should throw an exception containing all the inner animation parser errors', () => {
       var animation = sequence([
         style({'color': 'red'}), animate(1000, style({'font-size': '100px'})),
@@ -46,7 +53,7 @@ export function main() {
 
       var capturedErrorMessage: string;
       try {
-        compile(animation);
+        compileSequence(animation);
       } catch (e) {
         capturedErrorMessage = e.message;
       }
@@ -57,5 +64,23 @@ export function main() {
       expect(capturedErrorMessage)
           .toMatch(/Animation states via styles must be prefixed with a ":"/);
     });
+
+    it('should throw an error when two or more animation triggers contain the same name', () => {
+      var doCompile = () => {
+        var t1Data: any[] = [];
+        var t2Data: any[] = [];
+        compileTriggers([['myTrigger', t1Data], ['myTrigger', t2Data]]);
+      };
+
+      var capturedErrorMessage: string;
+      try {
+        doCompile();
+      } catch (e) {
+        capturedErrorMessage = e.message;
+      }
+
+      expect(capturedErrorMessage)
+          .toMatch(/The animation trigger "myTrigger" has already been registered on "myCmp"/);
+    });
   });
 }