Skip to content

Commit f8dc660

Browse files
dario-piotrowiczatscott
dario-piotrowicz
authored and
atscott
committed
fix(animations): allow animations with unsupported CSS properties (#44729)
currently animations with unsupported CSS properties cause a hard error and the crash of the animation itself, instead of this behaviour just ignore such properties and provide a warning for the developer in the console (only in dev mode) this change also introduces a general way to present warnings in the animations code resolves #23195 PR Close #44729
1 parent d65706a commit f8dc660

8 files changed

Lines changed: 116 additions & 20 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {AnimationMetadata, AnimationMetadataType, AnimationOptions, ɵStyleDataM
1010
import {buildingFailed, validationFailed} from '../error_helpers';
1111
import {AnimationDriver} from '../render/animation_driver';
1212
import {ENTER_CLASSNAME, LEAVE_CLASSNAME, normalizeStyles} from '../util';
13+
import {warnValidation} from '../warning_helpers';
1314

1415
import {Ast} from './animation_ast';
1516
import {buildAnimationAst} from './animation_ast_builder';
@@ -21,10 +22,14 @@ export class Animation {
2122
private _animationAst: Ast<AnimationMetadataType>;
2223
constructor(private _driver: AnimationDriver, input: AnimationMetadata|AnimationMetadata[]) {
2324
const errors: Error[] = [];
24-
const ast = buildAnimationAst(_driver, input, errors);
25+
const warnings: string[] = [];
26+
const ast = buildAnimationAst(_driver, input, errors, warnings);
2527
if (errors.length) {
2628
throw validationFailed(errors);
2729
}
30+
if (warnings.length) {
31+
warnValidation(warnings);
32+
}
2833
this._animationAst = ast;
2934
}
3035

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {invalidDefinition, invalidKeyframes, invalidOffset, invalidParallelAnima
1111
import {AnimationDriver} from '../render/animation_driver';
1212
import {getOrSetDefaultValue} from '../render/shared';
1313
import {convertToMap, copyObj, extractStyleParams, iteratorToArray, NG_ANIMATING_SELECTOR, NG_TRIGGER_SELECTOR, normalizeAnimationEntry, resolveTiming, SUBSTITUTION_EXPR_START, validateStyleParams, visitDslNode} from '../util';
14+
import {pushUnrecognizedPropertiesWarning} from '../warning_helpers';
1415

1516
import {AnimateAst, AnimateChildAst, AnimateRefAst, Ast, DynamicTimingAst, GroupAst, KeyframesAst, QueryAst, ReferenceAst, SequenceAst, StaggerAst, StateAst, StyleAst, TimingAst, TransitionAst, TriggerAst} from './animation_ast';
1617
import {AnimationDslVisitor} from './animation_dsl_visitor';
@@ -56,22 +57,30 @@ const SELF_TOKEN_REGEX = new RegExp(`\s*${SELF_TOKEN}\s*,?`, 'g');
5657
* Otherwise an error will be thrown.
5758
*/
5859
export function buildAnimationAst(
59-
driver: AnimationDriver, metadata: AnimationMetadata|AnimationMetadata[],
60-
errors: Error[]): Ast<AnimationMetadataType> {
61-
return new AnimationAstBuilderVisitor(driver).build(metadata, errors);
60+
driver: AnimationDriver, metadata: AnimationMetadata|AnimationMetadata[], errors: Error[],
61+
warnings: string[]): Ast<AnimationMetadataType> {
62+
return new AnimationAstBuilderVisitor(driver).build(metadata, errors, warnings);
6263
}
6364

6465
const ROOT_SELECTOR = '';
6566

6667
export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
6768
constructor(private _driver: AnimationDriver) {}
6869

69-
build(metadata: AnimationMetadata|AnimationMetadata[], errors: Error[]):
70+
build(metadata: AnimationMetadata|AnimationMetadata[], errors: Error[], warnings: string[]):
7071
Ast<AnimationMetadataType> {
7172
const context = new AnimationAstBuilderContext(errors);
7273
this._resetContextStyleTimingState(context);
73-
return <Ast<AnimationMetadataType>>visitDslNode(
74-
this, normalizeAnimationEntry(metadata), context);
74+
const ast =
75+
<Ast<AnimationMetadataType>>visitDslNode(this, normalizeAnimationEntry(metadata), context);
76+
77+
if (context.unsupportedCSSPropertiesFound.size) {
78+
pushUnrecognizedPropertiesWarning(
79+
warnings,
80+
[...context.unsupportedCSSPropertiesFound.keys()],
81+
);
82+
}
83+
return ast;
7584
}
7685

7786
private _resetContextStyleTimingState(context: AnimationAstBuilderContext) {
@@ -297,7 +306,8 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
297306

298307
tuple.forEach((value, prop) => {
299308
if (!this._driver.validateStyleProperty(prop)) {
300-
context.errors.push(invalidProperty(prop));
309+
tuple.delete(prop);
310+
context.unsupportedCSSPropertiesFound.add(prop);
301311
return;
302312
}
303313

@@ -503,6 +513,7 @@ export class AnimationAstBuilderContext {
503513
public currentTime: number = 0;
504514
public collectedStyles = new Map<string, Map<string, StyleTimeTuple>>();
505515
public options: AnimationOptions|null = null;
516+
public unsupportedCSSPropertiesFound: Set<string> = new Set<string>();
506517
constructor(public errors: Error[]) {}
507518
}
508519

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {buildAnimationAst} from '../dsl/animation_ast_builder';
1212
import {AnimationTrigger, buildTrigger} from '../dsl/animation_trigger';
1313
import {AnimationStyleNormalizer} from '../dsl/style_normalization/animation_style_normalizer';
1414
import {triggerBuildFailed} from '../error_helpers';
15+
import {warnTriggerBuild} from '../warning_helpers';
1516

1617
import {AnimationDriver} from './animation_driver';
1718
import {parseTimelineCommand} from './shared';
@@ -44,11 +45,15 @@ export class AnimationEngine {
4445
let trigger = this._triggerCache[cacheKey];
4546
if (!trigger) {
4647
const errors: Error[] = [];
47-
const ast =
48-
buildAnimationAst(this._driver, metadata as AnimationMetadata, errors) as TriggerAst;
48+
const warnings: string[] = [];
49+
const ast = buildAnimationAst(
50+
this._driver, metadata as AnimationMetadata, errors, warnings) as TriggerAst;
4951
if (errors.length) {
5052
throw triggerBuildFailed(name, errors);
5153
}
54+
if (warnings.length) {
55+
warnTriggerBuild(name, warnings);
56+
}
5257
trigger = buildTrigger(name, ast, this._normalizer);
5358
this._triggerCache[cacheKey] = trigger;
5459
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {ElementInstructionMap} from '../dsl/element_instruction_map';
1515
import {AnimationStyleNormalizer} from '../dsl/style_normalization/animation_style_normalizer';
1616
import {createAnimationFailed, missingOrDestroyedAnimation, missingPlayer, registerFailed} from '../error_helpers';
1717
import {ENTER_CLASSNAME, LEAVE_CLASSNAME} from '../util';
18+
import {warnRegister} from '../warning_helpers';
1819

1920
import {AnimationDriver} from './animation_driver';
2021
import {getOrSetDefaultValue, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared';
@@ -32,10 +33,14 @@ export class TimelineAnimationEngine {
3233

3334
register(id: string, metadata: AnimationMetadata|AnimationMetadata[]) {
3435
const errors: Error[] = [];
35-
const ast = buildAnimationAst(this._driver, metadata, errors);
36+
const warnings: string[] = [];
37+
const ast = buildAnimationAst(this._driver, metadata, errors, warnings);
3638
if (errors.length) {
3739
throw registerFailed(errors);
3840
} else {
41+
if (warnings.length) {
42+
warnRegister(warnings);
43+
}
3944
this._animations.set(id, ast);
4045
}
4146
}

‎packages/animations/browser/src/warning_helpers.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
const NG_DEV_MODE = typeof ngDevMode === 'undefined' || !!ngDevMode;
10+
11+
function createListOfWarnings(warnings: string[]): string {
12+
const LINE_START = '\n - ';
13+
return `${LINE_START}${warnings.filter(Boolean).map(warning => warning).join(LINE_START)}`;
14+
}
15+
16+
export function warnValidation(warnings: string[]): void {
17+
NG_DEV_MODE && console.warn(`animation validation warnings:${createListOfWarnings(warnings)}`);
18+
}
19+
20+
export function warnTriggerBuild(name: string, warnings: string[]): void {
21+
NG_DEV_MODE &&
22+
console.warn(`The animation trigger "${name}" has built with the following warnings:${
23+
createListOfWarnings(warnings)}`);
24+
}
25+
26+
export function warnRegister(warnings: string[]): void {
27+
NG_DEV_MODE &&
28+
console.warn(`Animation built with the following warnings:${createListOfWarnings(warnings)}`);
29+
}
30+
31+
export function triggerParsingWarnings(name: string, warnings: string[]): void {
32+
NG_DEV_MODE &&
33+
console.warn(`Animation parsing for the ${name} trigger presents the following warnings:${
34+
createListOfWarnings(warnings)}`);
35+
}
36+
37+
export function pushUnrecognizedPropertiesWarning(warnings: string[], props: string[]): void {
38+
if (ngDevMode && props.length) {
39+
warnings.push(
40+
`The provided CSS properties are not recognized properties supported for animations: ${
41+
props.join(', ')}`);
42+
}
43+
}

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

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -218,16 +218,28 @@ function createDiv() {
218218
/state\("panfinal", ...\) must define default values for all the following style substitutions: greyColor/);
219219
});
220220

221-
it('should throw an error if an invalid CSS property is used in the animation', () => {
221+
it('should provide a warning if an invalid CSS property is used in the animation', () => {
222222
const steps = [animate(1000, style({abc: '500px'}))];
223223

224-
expect(() => {
225-
validateAndThrowAnimationSequence(steps);
226-
})
227-
.toThrowError(
228-
/The provided animation property "abc" is not a supported CSS property for animations/);
224+
expect(getValidationWarningsForAnimationSequence(steps)).toEqual([
225+
'The provided CSS properties are not recognized properties supported for animations: abc'
226+
]);
229227
});
230228

229+
it('should provide a warning if multiple invalid CSS properties are used in the animation',
230+
() => {
231+
const steps = [
232+
state('state', style({
233+
'123': '100px',
234+
})),
235+
style({abc: '200px'}), animate(1000, style({xyz: '300px'}))
236+
];
237+
238+
expect(getValidationWarningsForAnimationSequence(steps)).toEqual([
239+
'The provided CSS properties are not recognized properties supported for animations: 123, abc, xyz'
240+
]);
241+
});
242+
231243
it('should allow a vendor-prefixed property to be used in an animation sequence without throwing an error',
232244
() => {
233245
const steps = [
@@ -1116,12 +1128,20 @@ function invokeAnimationSequence(
11161128
function validateAndThrowAnimationSequence(steps: AnimationMetadata|AnimationMetadata[]) {
11171129
const driver = new MockAnimationDriver();
11181130
const errors: Error[] = [];
1119-
const ast = buildAnimationAst(driver, steps, errors);
1131+
const ast = buildAnimationAst(driver, steps, errors, []);
11201132
if (errors.length) {
11211133
throw new Error(errors.join('\n'));
11221134
}
11231135
}
11241136

1137+
function getValidationWarningsForAnimationSequence(steps: AnimationMetadata|
1138+
AnimationMetadata[]): string[] {
1139+
const driver = new MockAnimationDriver();
1140+
const warnings: string[] = [];
1141+
buildAnimationAst(driver, steps, [], warnings);
1142+
return warnings;
1143+
}
1144+
11251145
function buildParams(params: {[name: string]: any}): AnimationOptions {
11261146
return <AnimationOptions>{params};
11271147
}

‎packages/animations/browser/test/render/transition_animation_engine_spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,9 +735,11 @@ function registerTrigger(
735735
element: any, engine: TransitionAnimationEngine, metadata: AnimationTriggerMetadata,
736736
id: string = DEFAULT_NAMESPACE_ID) {
737737
const errors: Error[] = [];
738+
const warnings: string[] = [];
738739
const driver = new MockAnimationDriver();
739740
const name = metadata.name;
740-
const ast = buildAnimationAst(driver, metadata as AnimationMetadata, errors) as TriggerAst;
741+
const ast =
742+
buildAnimationAst(driver, metadata as AnimationMetadata, errors, warnings) as TriggerAst;
741743
if (errors.length) {
742744
}
743745
const trigger = buildTrigger(name, ast, new NoopAnimationStyleNormalizer());

‎packages/animations/browser/test/shared.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,21 @@ import {buildAnimationAst} from '../src/dsl/animation_ast_builder';
1313
import {AnimationTrigger, buildTrigger} from '../src/dsl/animation_trigger';
1414
import {NoopAnimationStyleNormalizer} from '../src/dsl/style_normalization/animation_style_normalizer';
1515
import {triggerParsingFailed} from '../src/error_helpers';
16+
import {triggerParsingWarnings} from '../src/warning_helpers';
1617
import {MockAnimationDriver} from '../testing/src/mock_animation_driver';
1718

1819
export function makeTrigger(
1920
name: string, steps: any, skipErrors: boolean = false): AnimationTrigger {
2021
const driver = new MockAnimationDriver();
2122
const errors: Error[] = [];
23+
const warnings: string[] = [];
2224
const triggerData = trigger(name, steps);
23-
const triggerAst = buildAnimationAst(driver, triggerData, errors) as TriggerAst;
25+
const triggerAst = buildAnimationAst(driver, triggerData, errors, warnings) as TriggerAst;
2426
if (!skipErrors && errors.length) {
2527
throw triggerParsingFailed(name, errors);
2628
}
29+
if (warnings.length) {
30+
triggerParsingWarnings(name, warnings);
31+
}
2732
return buildTrigger(name, triggerAst, new NoopAnimationStyleNormalizer());
2833
}

0 commit comments

Comments
 (0)