feat(animations): report errors when invalid CSS properties are detected (angular#18718)

Closes angular#18701

PR Close angular#18718

Author: matsko

packages/animations/browser/src/dsl/animation.ts

@@ -20,7 +20,7 @@ export class Animation {
   private _animationAst: Ast;
   constructor(private _driver: AnimationDriver, input: AnimationMetadata|AnimationMetadata[]) {
     const errors: any[] = [];
-    const ast = buildAnimationAst(input, errors);
+    const ast = buildAnimationAst(_driver, input, errors);
     if (errors.length) {
       const errorMessage = `animation validation failed:\n${errors.join("\n")}`;
       throw new Error(errorMessage);

packages/animations/browser/src/dsl/animation_ast_builder.ts

@@ -7,6 +7,7 @@
  */
 import {AUTO_STYLE, AnimateTimings, AnimationAnimateChildMetadata, AnimationAnimateMetadata, AnimationAnimateRefMetadata, AnimationGroupMetadata, AnimationKeyframesSequenceMetadata, AnimationMetadata, AnimationMetadataType, AnimationOptions, AnimationQueryMetadata, AnimationQueryOptions, AnimationReferenceMetadata, AnimationSequenceMetadata, AnimationStaggerMetadata, AnimationStateMetadata, AnimationStyleMetadata, AnimationTransitionMetadata, AnimationTriggerMetadata, style, ɵStyleData} from '@angular/animations';
 
+import {AnimationDriver} from '../render/animation_driver';
 import {getOrSetAsInMap} from '../render/shared';
 import {ENTER_SELECTOR, LEAVE_SELECTOR, NG_ANIMATING_SELECTOR, NG_TRIGGER_SELECTOR, SUBSTITUTION_EXPR_START, copyObj, extractStyleParams, iteratorToArray, normalizeAnimationEntry, resolveTiming, validateStyleParams} from '../util';
 
@@ -54,8 +55,9 @@ const SELF_TOKEN_REGEX = new RegExp(`\s*${SELF_TOKEN}\s*,?`, 'g');
  * Otherwise an error will be thrown.
  */
 export function buildAnimationAst(
-    metadata: AnimationMetadata | AnimationMetadata[], errors: any[]): Ast {
-  return new AnimationAstBuilderVisitor().build(metadata, errors);
+    driver: AnimationDriver, metadata: AnimationMetadata | AnimationMetadata[],
+    errors: any[]): Ast {
+  return new AnimationAstBuilderVisitor(driver).build(metadata, errors);
 }
 
 const LEAVE_TOKEN = ':leave';
@@ -65,6 +67,8 @@ const ENTER_TOKEN_REGEX = new RegExp(ENTER_TOKEN, 'g');
 const ROOT_SELECTOR = '';
 
 export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
+  constructor(private _driver: AnimationDriver) {}
+
   build(metadata: AnimationMetadata|AnimationMetadata[], errors: any[]): Ast {
     const context = new AnimationAstBuilderContext(errors);
     this._resetContextStyleTimingState(context);
@@ -273,6 +277,12 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
       if (typeof tuple == 'string') return;
 
       Object.keys(tuple).forEach(prop => {
+        if (!this._driver.validateStyleProperty(prop)) {
+          context.errors.push(
+              `The provided animation property "${prop}" is not a supported CSS property for animations`);
+          return;
+        }
+
         const collectedStyles = context.collectedStyles[context.currentQuerySelector !];
         const collectedEntry = collectedStyles[prop];
         let updateCollectedStyle = true;

packages/animations/browser/src/dsl/animation_timeline_builder.ts

@@ -447,7 +447,7 @@ export class AnimationTimelineContext {
       private _driver: AnimationDriver, public element: any,
       public subInstructions: ElementInstructionMap, public errors: any[],
       public timelines: TimelineBuilder[], initialTimeline?: TimelineBuilder) {
-    this.currentTimeline = initialTimeline || new TimelineBuilder(element, 0);
+    this.currentTimeline = initialTimeline || new TimelineBuilder(this._driver, element, 0);
     timelines.push(this.currentTimeline);
   }
 
@@ -530,7 +530,7 @@ export class AnimationTimelineContext {
       easing: ''
     };
     const builder = new SubTimelineBuilder(
-        instruction.element, instruction.keyframes, instruction.preStyleProps,
+        this._driver, instruction.element, instruction.keyframes, instruction.preStyleProps,
         instruction.postStyleProps, updatedTimings, instruction.stretchStartingKeyframe);
     this.timelines.push(builder);
     return updatedTimings;
@@ -582,7 +582,7 @@ export class TimelineBuilder {
   private _currentEmptyStepKeyframe: ɵStyleData|null = null;
 
   constructor(
-      public element: any, public startTime: number,
+      private _driver: AnimationDriver, public element: any, public startTime: number,
       private _elementTimelineStylesLookup?: Map<any, ɵStyleData>) {
     if (!this._elementTimelineStylesLookup) {
       this._elementTimelineStylesLookup = new Map<any, ɵStyleData>();
@@ -632,7 +632,7 @@ export class TimelineBuilder {
   fork(element: any, currentTime?: number): TimelineBuilder {
     this.applyStylesToKeyframe();
     return new TimelineBuilder(
-        element, currentTime || this.currentTime, this._elementTimelineStylesLookup);
+        this._driver, element, currentTime || this.currentTime, this._elementTimelineStylesLookup);
   }
 
   private _loadKeyframe() {
@@ -796,10 +796,10 @@ class SubTimelineBuilder extends TimelineBuilder {
   public timings: AnimateTimings;
 
   constructor(
-      public element: any, public keyframes: ɵStyleData[], public preStyleProps: string[],
-      public postStyleProps: string[], timings: AnimateTimings,
+      driver: AnimationDriver, public element: any, public keyframes: ɵStyleData[],
+      public preStyleProps: string[], public postStyleProps: string[], timings: AnimateTimings,
       private _stretchStartingKeyframe: boolean = false) {
-    super(element, timings.delay);
+    super(driver, element, timings.delay);
     this.timings = {duration: timings.duration, delay: timings.delay, easing: timings.easing};
   }
 

packages/animations/browser/src/render/animation_driver.ts

@@ -7,13 +7,14 @@
  */
 import {AnimationPlayer, NoopAnimationPlayer} from '@angular/animations';
 
-import {containsElement, invokeQuery, matchesElement} from './shared';
-
+import {containsElement, invokeQuery, matchesElement, validateStyleProperty} from './shared';
 
 /**
  * @experimental
  */
 export class NoopAnimationDriver implements AnimationDriver {
+  validateStyleProperty(prop: string): boolean { return validateStyleProperty(prop); }
+
   matchesElement(element: any, selector: string): boolean {
     return matchesElement(element, selector);
   }
@@ -41,6 +42,8 @@ export class NoopAnimationDriver implements AnimationDriver {
 export abstract class AnimationDriver {
   static NOOP: AnimationDriver = new NoopAnimationDriver();
 
+  abstract validateStyleProperty(prop: string): boolean;
+
   abstract matchesElement(element: any, selector: string): boolean;
 
   abstract containsElement(elm1: any, elm2: any): boolean;

packages/animations/browser/src/render/animation_engine_next.ts

@@ -25,9 +25,9 @@ export class AnimationEngine {
   // this method is designed to be overridden by the code that uses this engine
   public onRemovalComplete = (element: any, context: any) => {};
 
-  constructor(driver: AnimationDriver, normalizer: AnimationStyleNormalizer) {
-    this._transitionEngine = new TransitionAnimationEngine(driver, normalizer);
-    this._timelineEngine = new TimelineAnimationEngine(driver, normalizer);
+  constructor(private _driver: AnimationDriver, normalizer: AnimationStyleNormalizer) {
+    this._transitionEngine = new TransitionAnimationEngine(_driver, normalizer);
+    this._timelineEngine = new TimelineAnimationEngine(_driver, normalizer);
 
     this._transitionEngine.onRemovalComplete = (element: any, context: any) =>
         this.onRemovalComplete(element, context);
@@ -40,7 +40,8 @@ export class AnimationEngine {
     let trigger = this._triggerCache[cacheKey];
     if (!trigger) {
       const errors: any[] = [];
-      const ast = buildAnimationAst(metadata as AnimationMetadata, errors) as TriggerAst;
+      const ast =
+          buildAnimationAst(this._driver, metadata as AnimationMetadata, errors) as TriggerAst;
       if (errors.length) {
         throw new Error(
             `The animation trigger "${name}" has failed to build due to the following errors:\n - ${errors.join("\n - ")}`);

packages/animations/browser/src/render/shared.ts

@@ -166,6 +166,21 @@ if (typeof Element != 'undefined') {
   };
 }
 
+let _CACHED_BODY: {style: any}|null = null;
+export function validateStyleProperty(prop: string): boolean {
+  if (!_CACHED_BODY) {
+    _CACHED_BODY = getBodyNode() || {};
+  }
+  return _CACHED_BODY !.style ? prop in _CACHED_BODY !.style : true;
+}
+
+export function getBodyNode(): any|null {
+  if (typeof document != 'undefined') {
+    return document.body;
+  }
+  return null;
+}
+
 export const matchesElement = _matches;
 export const containsElement = _contains;
 export const invokeQuery = _query;

packages/animations/browser/src/render/timeline_animation_engine.ts

@@ -28,7 +28,7 @@ export class TimelineAnimationEngine {
 
   register(id: string, metadata: AnimationMetadata|AnimationMetadata[]) {
     const errors: any[] = [];
-    const ast = buildAnimationAst(metadata, errors);
+    const ast = buildAnimationAst(this._driver, metadata, errors);
     if (errors.length) {
       throw new Error(
           `Unable to build the animation due to the following errors: ${errors.join("\n")}`);

packages/animations/browser/src/render/transition_animation_engine.ts

@@ -16,7 +16,7 @@ import {AnimationStyleNormalizer} from '../dsl/style_normalization/animation_sty
 import {ENTER_CLASSNAME, LEAVE_CLASSNAME, NG_ANIMATING_CLASSNAME, NG_ANIMATING_SELECTOR, NG_TRIGGER_CLASSNAME, NG_TRIGGER_SELECTOR, copyObj, eraseStyles, setStyles} from '../util';
 
 import {AnimationDriver} from './animation_driver';
-import {getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared';
+import {getBodyNode, getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared';
 
 const QUEUED_CLASSNAME = 'ng-animate-queued';
 const QUEUED_SELECTOR = '.ng-animate-queued';
@@ -1525,13 +1525,6 @@ function removeClass(element: any, className: string) {
   }
 }
 
-function getBodyNode(): any|null {
-  if (typeof document != 'undefined') {
-    return document.body;
-  }
-  return null;
-}
-
 function removeNodesAfterAnimationDone(
     engine: TransitionAnimationEngine, element: any, players: AnimationPlayer[]) {
   optimizeGroupPlayer(players).onDone(() => engine.processLeaveNode(element));

packages/animations/browser/src/render/web_animations/web_animations_driver.ts

@@ -8,11 +8,13 @@
 import {AnimationPlayer, ɵStyleData} from '@angular/animations';
 
 import {AnimationDriver} from '../animation_driver';
-import {containsElement, invokeQuery, matchesElement} from '../shared';
+import {containsElement, invokeQuery, matchesElement, validateStyleProperty} from '../shared';
 
 import {WebAnimationsPlayer} from './web_animations_player';
 
 export class WebAnimationsDriver implements AnimationDriver {
+  validateStyleProperty(prop: string): boolean { return validateStyleProperty(prop); }
+
   matchesElement(element: any, selector: string): boolean {
     return matchesElement(element, selector);
   }

packages/animations/browser/test/dsl/animation_spec.ts

@@ -177,10 +177,12 @@ export function main() {
 
       it('should throw if dynamic style substitutions are used without defaults within state() definitions',
          () => {
-           const steps = [state('final', style({
-                                  'width': '{{ one }}px',
-                                  'borderRadius': '{{ two }}px {{ three }}px',
-                                }))];
+           const steps = [
+             state('final', style({
+                     'width': '{{ one }}px',
+                     'borderRadius': '{{ two }}px {{ three }}px',
+                   })),
+           ];
 
            expect(() => { validateAndThrowAnimationSequence(steps); })
                .toThrowError(
@@ -198,6 +200,14 @@ export function main() {
                .toThrowError(
                    /state\("panfinal", ...\) must define default values for all the following style substitutions: greyColor/);
          });
+
+      it('should throw an error if an invalid CSS property is used in the animation', () => {
+        const steps = [animate(1000, style({abc: '500px'}))];
+
+        expect(() => { validateAndThrowAnimationSequence(steps); })
+            .toThrowError(
+                /The provided animation property "abc" is not a supported CSS property for animations/);
+      });
     });
 
     describe('keyframe building', () => {
@@ -388,14 +398,13 @@ export function main() {
 
         it('should allow multiple substitutions to occur within the same style value', () => {
           const steps = [
-            style({transform: ''}),
-            animate(1000, style({transform: 'translateX({{ x }}) translateY({{ y }})'}))
+            style({borderRadius: '100px 100px'}),
+            animate(1000, style({borderRadius: '{{ one }}px {{ two }}'})),
           ];
           const players =
-              invokeAnimationSequence(rootElement, steps, buildParams({x: '200px', y: '400px'}));
+              invokeAnimationSequence(rootElement, steps, buildParams({one: '200', two: '400px'}));
           expect(players[0].keyframes).toEqual([
-            {offset: 0, transform: ''},
-            {offset: 1, transform: 'translateX(200px) translateY(400px)'}
+            {offset: 0, borderRadius: '100px 100px'}, {offset: 1, borderRadius: '200px 400px'}
           ]);
         });
 
@@ -571,18 +580,12 @@ export function main() {
            () => {
              const steps = [
                animate(1000, style({height: '50px'})),
-               animate(
-                   2000, keyframes([
-                     style({left: '0', transform: 'rotate(0deg)', offset: 0}),
-                     style({
-                       left: '40%',
-                       transform: 'rotate(250deg) translateY(-200px)',
-                       offset: .33
-                     }),
-                     style(
-                         {left: '60%', transform: 'rotate(180deg) translateY(200px)', offset: .66}),
-                     style({left: 'calc(100% - 100px)', transform: 'rotate(0deg)', offset: 1}),
-                   ])),
+               animate(2000, keyframes([
+                         style({left: '0', top: '0', offset: 0}),
+                         style({left: '40%', top: '50%', offset: .33}),
+                         style({left: '60%', top: '80%', offset: .66}),
+                         style({left: 'calc(100% - 100px)', top: '100%', offset: 1}),
+                       ])),
                group([animate('2s', style({width: '200px'}))]),
                animate('2s', style({height: '300px'})),
                group([animate('2s', style({height: '500px', width: '500px'}))])
@@ -987,8 +990,9 @@ function invokeAnimationSequence(
 }
 
 function validateAndThrowAnimationSequence(steps: AnimationMetadata | AnimationMetadata[]) {
+  const driver = new MockAnimationDriver();
   const errors: any[] = [];
-  const ast = buildAnimationAst(steps, errors);
+  const ast = buildAnimationAst(driver, steps, errors);
   if (errors.length) {
     throw new Error(errors.join('\n'));
   }