diff --git a/packages/animations/browser/src/dsl/animation_transition_factory.ts b/packages/animations/browser/src/dsl/animation_transition_factory.ts index 35697f7da5..390f77d32f 100644 --- a/packages/animations/browser/src/dsl/animation_transition_factory.ts +++ b/packages/animations/browser/src/dsl/animation_transition_factory.ts @@ -29,12 +29,16 @@ export class AnimationTransitionFactory { build( driver: AnimationDriver, element: any, currentState: any, nextState: any, options?: AnimationOptions, - subInstructions?: ElementInstructionMap): AnimationTransitionInstruction|undefined { + subInstructions?: ElementInstructionMap): AnimationTransitionInstruction { const animationOptions = mergeAnimationOptions(this.ast.options || {}, options || {}); const backupStateStyles = this._stateStyles['*'] || {}; const currentStateStyles = this._stateStyles[currentState] || backupStateStyles; const nextStateStyles = this._stateStyles[nextState] || backupStateStyles; + const queriedElements = new Set(); + const preStyleMap = new Map(); + const postStyleMap = new Map(); + const isRemoval = nextState === 'void'; const errors: any[] = []; const timelines = buildAnimationTimelines( @@ -42,13 +46,11 @@ export class AnimationTransitionFactory { subInstructions, errors); if (errors.length) { - const errorMessage = `animation building failed:\n${errors.join("\n")}`; - throw new Error(errorMessage); + return createTransitionInstruction( + element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles, + nextStateStyles, [], [], preStyleMap, postStyleMap, errors); } - const preStyleMap = new Map(); - const postStyleMap = new Map(); - const queriedElements = new Set(); timelines.forEach(tl => { const elm = tl.element; const preProps = getOrSetAsInMap(preStyleMap, elm, {}); @@ -64,9 +66,8 @@ export class AnimationTransitionFactory { const queriedElementsList = iteratorToArray(queriedElements.values()); return createTransitionInstruction( - element, this._triggerName, currentState, nextState, nextState === 'void', - currentStateStyles, nextStateStyles, timelines, queriedElementsList, preStyleMap, - postStyleMap); + element, this._triggerName, currentState, nextState, isRemoval, currentStateStyles, + nextStateStyles, timelines, queriedElementsList, preStyleMap, postStyleMap); } } diff --git a/packages/animations/browser/src/dsl/animation_transition_instruction.ts b/packages/animations/browser/src/dsl/animation_transition_instruction.ts index dbd963fadd..f1062b322b 100644 --- a/packages/animations/browser/src/dsl/animation_transition_instruction.ts +++ b/packages/animations/browser/src/dsl/animation_transition_instruction.ts @@ -21,6 +21,7 @@ export interface AnimationTransitionInstruction extends AnimationEngineInstructi queriedElements: any[]; preStyleProps: Map; postStyleProps: Map; + errors?: any[]; } export function createTransitionInstruction( @@ -28,7 +29,8 @@ export function createTransitionInstruction( isRemovalTransition: boolean, fromStyles: ɵStyleData, toStyles: ɵStyleData, timelines: AnimationTimelineInstruction[], queriedElements: any[], preStyleProps: Map, - postStyleProps: Map): AnimationTransitionInstruction { + postStyleProps: Map, + errors?: any[]): AnimationTransitionInstruction { return { type: AnimationTransitionInstructionType.TransitionAnimation, element, @@ -41,6 +43,7 @@ export function createTransitionInstruction( timelines, queriedElements, preStyleProps, - postStyleProps + postStyleProps, + errors }; } diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 29e5b43b48..0d2bd44d6f 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -745,6 +745,7 @@ export class TransitionAnimationEngine { const queriedElements = new Map(); const allPreStyleElements = new Map>(); const allPostStyleElements = new Map>(); + const cleanupFns: Function[] = []; const bodyNode = getBodyNode(); const allEnterNodes: any[] = this.collectedEnterElements.length ? @@ -772,10 +773,21 @@ export class TransitionAnimationEngine { } } + cleanupFns.push(() => { + allEnterNodes.forEach(element => removeClass(element, ENTER_CLASSNAME)); + allLeaveNodes.forEach(element => { + removeClass(element, LEAVE_CLASSNAME); + this.processLeaveNode(element); + }); + }); + + const allPlayers: TransitionAnimationPlayer[] = []; + const erroneousTransitions: AnimationTransitionInstruction[] = []; for (let i = this._namespaceList.length - 1; i >= 0; i--) { const ns = this._namespaceList[i]; ns.drainQueuedTransitions(microtaskId).forEach(entry => { const player = entry.player; + allPlayers.push(player); const element = entry.element; if (!bodyNode || !this.driver.containsElement(bodyNode, element)) { @@ -783,8 +795,11 @@ export class TransitionAnimationEngine { return; } - const instruction = this._buildInstruction(entry, subTimelines); - if (!instruction) return; + const instruction = this._buildInstruction(entry, subTimelines) !; + if (instruction.errors && instruction.errors.length) { + erroneousTransitions.push(instruction); + return; + } // if a unmatched transition is queued to go then it SHOULD NOT render // an animation and cancel the previously running animations. @@ -833,6 +848,18 @@ export class TransitionAnimationEngine { }); } + if (erroneousTransitions.length) { + let msg = `Unable to process animations due to the following failed trigger transitions\n`; + erroneousTransitions.forEach(instruction => { + msg += `@${instruction.triggerName} has failed due to:\n`; + instruction.errors !.forEach(error => { msg += `- ${error}\n`; }); + }); + + cleanupFns.forEach(fn => fn()); + allPlayers.forEach(player => player.destroy()); + throw new Error(msg); + } + // these can only be detected here since we have a map of all the elements // that have animations attached to them... const enterNodesWithoutAnimations: any[] = []; @@ -937,6 +964,7 @@ export class TransitionAnimationEngine { for (let i = 0; i < allLeaveNodes.length; i++) { const element = allLeaveNodes[i]; const details = element[REMOVAL_FLAG] as ElementAnimationState; + removeClass(element, LEAVE_CLASSNAME); // this means the element has a removal animation that is being // taken care of and therefore the inner elements will hang around @@ -969,6 +997,9 @@ export class TransitionAnimationEngine { } } + // this is required so the cleanup method doesn't remove them + allLeaveNodes.length = 0; + rootPlayers.forEach(player => { this.players.push(player); player.onDone(() => { @@ -980,8 +1011,7 @@ export class TransitionAnimationEngine { player.play(); }); - allEnterNodes.forEach(element => removeClass(element, ENTER_CLASSNAME)); - + cleanupFns.forEach(fn => fn()); return rootPlayers; } diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index b94ffabf30..d8fe04024a 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -1826,6 +1826,64 @@ export function main() { /only state\(\) and transition\(\) definitions can sit inside of a trigger\(\)/); }); + it('should combine multiple errors together into one exception when an animation fails to be built', + () => { + @Component({ + selector: 'if-cmp', + template: ` +
+ `, + animations: [ + trigger( + 'foo', + [ + transition(':enter', []), + transition( + '* => *', + [ + query('foo', animate(1000, style({background: 'red'}))), + ]), + ]), + trigger( + 'bar', + [ + transition(':enter', []), + transition( + '* => *', + [ + query('bar', animate(1000, style({background: 'blue'}))), + ]), + ]), + ] + }) + class Cmp { + fooExp: any = false; + barExp: any = false; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + + cmp.fooExp = 'go'; + cmp.barExp = 'go'; + + let errorMsg: string = ''; + try { + fixture.detectChanges(); + } catch (e) { + errorMsg = e.message; + } + + expect(errorMsg).toMatch(/@foo has failed due to:/); + expect(errorMsg).toMatch(/`query\("foo"\)` returned zero elements/); + expect(errorMsg).toMatch(/@bar has failed due to:/); + expect(errorMsg).toMatch(/`query\("bar"\)` returned zero elements/); + }); + it('should not throw an error if styles overlap in separate transitions', () => { @Component({ selector: 'if-cmp', diff --git a/packages/core/test/animation/animation_query_integration_spec.ts b/packages/core/test/animation/animation_query_integration_spec.ts index 981e3b0bee..4577a51cf5 100644 --- a/packages/core/test/animation/animation_query_integration_spec.ts +++ b/packages/core/test/animation/animation_query_integration_spec.ts @@ -8,6 +8,7 @@ import {AUTO_STYLE, AnimationPlayer, animate, animateChild, query, stagger, state, style, transition, trigger, ɵAnimationGroupPlayer as AnimationGroupPlayer} from '@angular/animations'; import {AnimationDriver, ɵAnimationEngine} from '@angular/animations/browser'; import {matchesElement} from '@angular/animations/browser/src/render/shared'; +import {ENTER_CLASSNAME, LEAVE_CLASSNAME} from '@angular/animations/browser/src/util'; import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/browser/testing'; import {CommonModule} from '@angular/common'; import {Component, HostBinding, ViewChild} from '@angular/core'; @@ -749,6 +750,69 @@ export function main() { }); }); + it('should cleanup :enter and :leave artifacts from nodes when any animation sequences fail to be built', + () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+
+ {{ item }} +
+
Leave!
+
+ `, + animations: [ + trigger( + 'myAnimation', + [ + transition('* => 0', []), + transition( + '* => *', + [ + query( + '.child:enter', + [ + style({opacity: 0}), + animate(1000, style({opacity: 1})), + ]), + query( + '.incorrect-child:leave', + [ + animate(1000, style({opacity: 0})), + ]), + ]), + ]), + ] + }) + class Cmp { + @ViewChild('container') public container: any; + public items: any[] = []; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.items = []; + fixture.detectChanges(); + + cmp.items = [0, 1, 2, 3, 4]; + + expect(() => { fixture.detectChanges(); }).toThrow(); + + const children = cmp.container.nativeElement.querySelectorAll('.child'); + expect(children.length).toEqual(5); + + for (let i = 0; i < children.length; i++) { + let child = children[i]; + expect(child.classList.contains(ENTER_CLASSNAME)).toBe(false); + expect(child.classList.contains(LEAVE_CLASSNAME)).toBe(false); + } + }); + it('should find elements that have been removed via :leave', () => { @Component({ selector: 'ani-cmp',