Skip to content

Commit

Permalink
refactor(animations): drop convertToMap & copyStyles and use nati…
Browse files Browse the repository at this point in the history
…ve functions. (#52441)

`copyStyles` was side-effectful and also returned a value : not great.

PR Close #52441
  • Loading branch information
JeanMeche authored and atscott committed Dec 14, 2023
1 parent d28ce0a commit d7e7409
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 58 deletions.
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"runtime": 2906,
"main": 221187,
"polyfills": 33782,
"node_modules_angular_animations_fesm2022_browser_mjs": 63981,
"node_modules_angular_animations_fesm2022_browser_mjs": 63949,
"default-node_modules_angular_animations_fesm2022_animations_mjs": 4264,
"src_app_open-close_component_ts": 1321
}
Expand Down
4 changes: 2 additions & 2 deletions packages/animations/browser/src/dsl/animation_ast_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {AnimateTimings, AnimationAnimateChildMetadata, AnimationAnimateMetadata,
import {invalidDefinition, invalidKeyframes, invalidOffset, invalidParallelAnimation, invalidProperty, invalidStagger, invalidState, invalidStyleValue, invalidTrigger, keyframeOffsetsOutOfOrder, keyframesMissingOffsets} from '../error_helpers';
import {AnimationDriver} from '../render/animation_driver';
import {getOrSetDefaultValue} from '../render/shared';
import {convertToMap, extractStyleParams, NG_ANIMATING_SELECTOR, NG_TRIGGER_SELECTOR, normalizeAnimationEntry, resolveTiming, SUBSTITUTION_EXPR_START, validateStyleParams, visitDslNode} from '../util';
import {extractStyleParams, NG_ANIMATING_SELECTOR, NG_TRIGGER_SELECTOR, normalizeAnimationEntry, resolveTiming, SUBSTITUTION_EXPR_START, validateStyleParams, visitDslNode} from '../util';
import {pushUnrecognizedPropertiesWarning} from '../warning_helpers';

import {AnimateAst, AnimateChildAst, AnimateRefAst, Ast, DynamicTimingAst, GroupAst, KeyframesAst, QueryAst, ReferenceAst, SequenceAst, StaggerAst, StateAst, StyleAst, TimingAst, TransitionAst, TriggerAst} from './animation_ast';
Expand Down Expand Up @@ -262,7 +262,7 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
context.errors.push(invalidStyleValue(styleTuple));
}
} else {
styles.push(convertToMap(styleTuple));
styles.push(new Map(Object.entries(styleTuple)));
}
}

Expand Down
19 changes: 10 additions & 9 deletions packages/animations/browser/src/dsl/animation_timeline_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {AnimateChildOptions, AnimateTimings, AnimationMetadataType, AnimationOpt

import {invalidQuery} from '../error_helpers';
import {AnimationDriver} from '../render/animation_driver';
import {copyStyles, interpolateParams, resolveTiming, resolveTimingValue, visitDslNode} from '../util';
import {interpolateParams, resolveTiming, resolveTimingValue, visitDslNode} from '../util';

import {AnimateAst, AnimateChildAst, AnimateRefAst, Ast, AstVisitor, DynamicTimingAst, GroupAst, KeyframesAst, QueryAst, ReferenceAst, SequenceAst, StaggerAst, StateAst, StyleAst, TimingAst, TransitionAst, TriggerAst} from './animation_ast';
import {AnimationTimelineInstruction, createTimelineInstruction} from './animation_timeline_instruction';
Expand Down Expand Up @@ -89,8 +89,7 @@ const LEAVE_TOKEN_REGEX = new RegExp(LEAVE_TOKEN, 'g');
* from all previous animation steps. Therefore when a keyframe is created it would also be missing
* from all previous keyframes up until where it is first used. For the timeline keyframe generation
* to properly fill in the style it will place the previous value (the value from the parent
* timeline) or a default value of `*` into the backFill map. The `copyStyles` method in util.ts
* handles propagating that backfill map to the styles object.
* timeline) or a default value of `*` into the backFill map.
*
* When a sub-timeline is created it will have its own backFill property. This is done so that
* styles present within the sub-timeline do not accidentally seep into the previous/future timeline
Expand Down Expand Up @@ -802,7 +801,7 @@ export class TimelineBuilder {

let finalKeyframes: Array<ɵStyleDataMap> = [];
this._keyframes.forEach((keyframe, time) => {
const finalKeyframe = copyStyles(keyframe, new Map(), this._backFill);
const finalKeyframe = new Map([...this._backFill, ...keyframe]);
finalKeyframe.forEach((value, prop) => {
if (value === PRE_STYLE) {
preStyleProps.add(prop);
Expand Down Expand Up @@ -858,11 +857,11 @@ class SubTimelineBuilder extends TimelineBuilder {
const startingGap = delay / totalTime;

// the original starting keyframe now starts once the delay is done
const newFirstKeyframe = copyStyles(keyframes[0]);
const newFirstKeyframe = new Map(keyframes[0]);
newFirstKeyframe.set('offset', 0);
newKeyframes.push(newFirstKeyframe);

const oldFirstKeyframe = copyStyles(keyframes[0]);
const oldFirstKeyframe = new Map(keyframes[0]);
oldFirstKeyframe.set('offset', roundOffset(startingGap));
newKeyframes.push(oldFirstKeyframe);

Expand All @@ -884,7 +883,7 @@ class SubTimelineBuilder extends TimelineBuilder {
// offsets between 1 ... n -1 are all warped by the keyframe stretch
const limit = keyframes.length - 1;
for (let i = 1; i <= limit; i++) {
let kf = copyStyles(keyframes[i]);
let kf = new Map(keyframes[i]);
const oldOffset = kf.get('offset') as number;
const timeAtKeyframe = delay + oldOffset * duration;
kf.set('offset', roundOffset(timeAtKeyframe / totalTime));
Expand Down Expand Up @@ -915,12 +914,14 @@ function flattenStyles(input: Array<(ɵStyleDataMap | string)>, allStyles: ɵSty
let allProperties: string[]|IterableIterator<string>;
input.forEach(token => {
if (token === '*') {
allProperties = allProperties || allStyles.keys();
allProperties ??= allStyles.keys();
for (let prop of allProperties) {
styles.set(prop, AUTO_STYLE);
}
} else {
copyStyles(token as ɵStyleDataMap, styles);
for (let [prop, val] of token as ɵStyleDataMap) {
styles.set(prop, val);
}
}
});
return styles;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/
import {AnimationPlayer, ɵStyleDataMap} from '@angular/animations';

import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, camelCaseToDashCase, computeStyle, copyStyles, normalizeKeyframes} from '../../util';
import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, camelCaseToDashCase, computeStyle, normalizeKeyframes} from '../../util';
import {AnimationDriver} from '../animation_driver';
import {containsElement, getParentElement, invokeQuery, validateStyleProperty, validateWebAnimatableStyleProperty} from '../shared';
import {packageNonAnimatableStyles} from '../special_cased_styles';
Expand Down Expand Up @@ -73,7 +73,7 @@ export class WebAnimationsDriver implements AnimationDriver {
});
}

let _keyframes = normalizeKeyframes(keyframes).map(styles => copyStyles(styles));
let _keyframes = normalizeKeyframes(keyframes).map(styles => new Map(styles));
_keyframes = balancePreviousStylesIntoKeyframes(element, _keyframes, previousStyles);
const specialStyles = packageNonAnimatableStyles(element, _keyframes);
return new WebAnimationsPlayer(element, _keyframes, playerOptions, specialStyles);
Expand Down
35 changes: 3 additions & 32 deletions packages/animations/browser/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {Ast as AnimationAst, AstVisitor as AnimationAstVisitor} from './dsl/anim
import {AnimationDslVisitor} from './dsl/animation_dsl_visitor';
import {invalidNodeType, invalidParamValue, invalidStyleParams, invalidTimingValue, negativeDelayValue, negativeStepValue} from './error_helpers';

export const ONE_SECOND = 1000;
const ONE_SECOND = 1000;

export const SUBSTITUTION_EXPR_START = '{{';
export const SUBSTITUTION_EXPR_END = '}}';
Expand Down Expand Up @@ -94,15 +94,6 @@ function parseTimeExpression(
return {duration, delay, easing};
}

export function convertToMap(obj: ɵStyleData): ɵStyleDataMap {
const styleMap: ɵStyleDataMap = new Map();
Object.keys(obj).forEach(prop => {
const val = obj[prop];
styleMap.set(prop, val);
});
return styleMap;
}

export function normalizeKeyframes(keyframes: Array<ɵStyleData>|
Array<ɵStyleDataMap>): Array<ɵStyleDataMap> {
if (!keyframes.length) {
Expand All @@ -111,31 +102,11 @@ export function normalizeKeyframes(keyframes: Array<ɵStyleData>|
if (keyframes[0] instanceof Map) {
return keyframes as Array<ɵStyleDataMap>;
}
return keyframes.map(kf => convertToMap(kf as ɵStyleData));
return keyframes.map(kf => new Map(Object.entries(kf)));
}

export function normalizeStyles(styles: ɵStyleDataMap|Array<ɵStyleDataMap>): ɵStyleDataMap {
const normalizedStyles: ɵStyleDataMap = new Map();
if (Array.isArray(styles)) {
styles.forEach(data => copyStyles(data, normalizedStyles));
} else {
copyStyles(styles, normalizedStyles);
}
return normalizedStyles;
}

export function copyStyles(
styles: ɵStyleDataMap, destination: ɵStyleDataMap = new Map(),
backfill?: ɵStyleDataMap): ɵStyleDataMap {
if (backfill) {
for (let [prop, val] of backfill) {
destination.set(prop, val);
}
}
for (let [prop, val] of styles) {
destination.set(prop, val);
}
return destination;
return Array.isArray(styles) ? new Map(...styles) : new Map(styles);
}

export function setStyles(element: any, styles: ɵStyleDataMap, formerStyles?: ɵStyleDataMap) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,9 @@
{
"name": "applyNodes"
},
{
"name": "applyParamDefaults"
},
{
"name": "applyProjectionRecursive"
},
Expand Down Expand Up @@ -737,15 +740,9 @@
{
"name": "convertToBitFlags"
},
{
"name": "convertToMap"
},
{
"name": "copyAnimationEvent"
},
{
"name": "copyStyles"
},
{
"name": "createElementNode"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,9 @@
{
"name": "applyNodes"
},
{
"name": "applyParamDefaults"
},
{
"name": "applyProjectionRecursive"
},
Expand Down Expand Up @@ -797,15 +800,9 @@
{
"name": "convertToBitFlags"
},
{
"name": "convertToMap"
},
{
"name": "copyAnimationEvent"
},
{
"name": "copyStyles"
},
{
"name": "createElementNode"
},
Expand Down

0 comments on commit d7e7409

Please sign in to comment.