Skip to content

Commit 6c61d20

Browse files
dario-piotrowiczthePunderWoman
dario-piotrowicz
authored and
thePunderWoman
committed
fix(animations): allow animations with unsupported CSS properties (#45185)
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 #45185
1 parent e914da1 commit 6c61d20

File tree

8 files changed

+116
-20
lines changed

8 files changed

+116
-20
lines changed

‎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, ɵStyleData}
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 {getOrSetAsInMap} from '../render/shared';
1313
import {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) {
@@ -303,7 +312,8 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
303312

304313
Object.keys(tuple).forEach(prop => {
305314
if (!this._driver.validateStyleProperty(prop)) {
306-
context.errors.push(invalidProperty(prop));
315+
delete tuple[prop];
316+
context.unsupportedCSSPropertiesFound.add(prop);
307317
return;
308318
}
309319

@@ -507,6 +517,7 @@ export class AnimationAstBuilderContext {
507517
public currentTime: number = 0;
508518
public collectedStyles: {[selectorName: string]: {[propName: string]: StyleTimeTuple}} = {};
509519
public options: AnimationOptions|null = null;
520+
public unsupportedCSSPropertiesFound: Set<string> = new Set<string>();
510521
constructor(public errors: Error[]) {}
511522
}
512523

‎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 {getOrSetAsInMap, 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[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 = [
@@ -1070,12 +1082,20 @@ function invokeAnimationSequence(
10701082
function validateAndThrowAnimationSequence(steps: AnimationMetadata|AnimationMetadata[]) {
10711083
const driver = new MockAnimationDriver();
10721084
const errors: Error[] = [];
1073-
const ast = buildAnimationAst(driver, steps, errors);
1085+
const ast = buildAnimationAst(driver, steps, errors, []);
10741086
if (errors.length) {
10751087
throw new Error(errors.join('\n'));
10761088
}
10771089
}
10781090

1091+
function getValidationWarningsForAnimationSequence(steps: AnimationMetadata|
1092+
AnimationMetadata[]): string[] {
1093+
const driver = new MockAnimationDriver();
1094+
const warnings: string[] = [];
1095+
buildAnimationAst(driver, steps, [], warnings);
1096+
return warnings;
1097+
}
1098+
10791099
function buildParams(params: {[name: string]: any}): AnimationOptions {
10801100
return <AnimationOptions>{params};
10811101
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,9 +732,11 @@ function registerTrigger(
732732
element: any, engine: TransitionAnimationEngine, metadata: AnimationTriggerMetadata,
733733
id: string = DEFAULT_NAMESPACE_ID) {
734734
const errors: Error[] = [];
735+
const warnings: string[] = [];
735736
const driver = new MockAnimationDriver();
736737
const name = metadata.name;
737-
const ast = buildAnimationAst(driver, metadata as AnimationMetadata, errors) as TriggerAst;
738+
const ast =
739+
buildAnimationAst(driver, metadata as AnimationMetadata, errors, warnings) as TriggerAst;
738740
if (errors.length) {
739741
}
740742
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)