perf(animations): Remove generic objects in favor of Maps #44482
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,11 @@ | |
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import {AnimateTimings, AnimationAnimateChildMetadata, AnimationAnimateMetadata, AnimationAnimateRefMetadata, AnimationGroupMetadata, AnimationKeyframesSequenceMetadata, AnimationMetadata, AnimationMetadataType, AnimationOptions, AnimationQueryMetadata, AnimationQueryOptions, AnimationReferenceMetadata, AnimationSequenceMetadata, AnimationStaggerMetadata, AnimationStateMetadata, AnimationStyleMetadata, AnimationTransitionMetadata, AnimationTriggerMetadata, AUTO_STYLE, style, ɵStyleDataMap} from '@angular/animations'; | ||
|
|
||
| import {AnimationDriver} from '../render/animation_driver'; | ||
| import {getOrSetDefaultValue} from '../render/shared'; | ||
| import {convertToMap, copyObj, extractStyleParams, iteratorToArray, NG_ANIMATING_SELECTOR, NG_TRIGGER_SELECTOR, normalizeAnimationEntry, resolveTiming, SUBSTITUTION_EXPR_START, validateStyleParams, visitDslNode} from '../util'; | ||
|
|
||
| import {AnimateAst, AnimateChildAst, AnimateRefAst, Ast, DynamicTimingAst, GroupAst, KeyframesAst, QueryAst, ReferenceAst, SequenceAst, StaggerAst, StateAst, StyleAst, TimingAst, TransitionAst, TriggerAst} from './animation_ast'; | ||
| import {AnimationDslVisitor} from './animation_dsl_visitor'; | ||
|
|
@@ -75,8 +75,8 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor { | |
|
|
||
| private _resetContextStyleTimingState(context: AnimationAstBuilderContext) { | ||
| context.currentQuerySelector = ROOT_SELECTOR; | ||
| context.collectedStyles = new Map<string, Map<string, StyleTimeTuple>>(); | ||
| context.collectedStyles.set(ROOT_SELECTOR, new Map()); | ||
| context.currentTime = 0; | ||
| } | ||
|
|
||
|
|
@@ -129,11 +129,10 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor { | |
| if (styleAst.containsDynamicStyles) { | ||
| const missingSubs = new Set<string>(); | ||
| const params = astParams || {}; | ||
| styleAst.styles.forEach(style => { | ||
| if (style instanceof Map) { | ||
| style.forEach(value => { | ||
| extractStyleParams(value).forEach(sub => { | ||
| if (!params.hasOwnProperty(sub)) { | ||
| missingSubs.add(sub); | ||
| } | ||
|
|
@@ -206,9 +205,9 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor { | |
| AnimateAst { | ||
| const timingAst = constructTimingAst(metadata.timings, context.errors); | ||
| context.currentAnimateTimings = timingAst; | ||
| let styleAst: StyleAst|KeyframesAst; | ||
| let styleMetadata: AnimationStyleMetadata|AnimationKeyframesSequenceMetadata = | ||
| metadata.styles ? metadata.styles : style({}); | ||
| if (styleMetadata.type == AnimationMetadataType.Keyframes) { | ||
| styleAst = this.visitKeyframes(styleMetadata as AnimationKeyframesSequenceMetadata, context); | ||
| } else { | ||
|
|
@@ -245,37 +244,32 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor { | |
|
|
||
| private _makeStyleAst(metadata: AnimationStyleMetadata, context: AnimationAstBuilderContext): | ||
| StyleAst { | ||
| const styles: Array<(ɵStyleDataMap | string)> = []; | ||
| const metadataStyles = Array.isArray(metadata.styles) ? metadata.styles : [metadata.styles]; | ||
|
|
||
| for (let styleTuple of metadataStyles) { | ||
| if (typeof styleTuple === 'string') { | ||
| if (styleTuple === AUTO_STYLE) { | ||
| styles.push(styleTuple); | ||
| } else { | ||
| context.errors.push(`The provided style string value ${styleTuple} is not allowed.`); | ||
| } | ||
| } else { | ||
| styles.push(convertToMap(styleTuple)); | ||
| } | ||
| } | ||
|
|
||
| let containsDynamicStyles = false; | ||
| let collectedEasing: string|null = null; | ||
| styles.forEach(styleData => { | ||
| if (styleData instanceof Map) { | ||
| if (styleData.has('easing')) { | ||
| collectedEasing = styleData.get('easing') as string; | ||
| styleData.delete('easing'); | ||
| } | ||
| if (!containsDynamicStyles) { | ||
| for (let value of styleData.values()) { | ||
| if (value!.toString().indexOf(SUBSTITUTION_EXPR_START) >= 0) { | ||
| containsDynamicStyles = true; | ||
| break; | ||
| } | ||
|
|
@@ -303,17 +297,19 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor { | |
| } | ||
|
|
||
| ast.styles.forEach(tuple => { | ||
| if (typeof tuple === 'string') return; | ||
|
|
||
| tuple.forEach((value, prop) => { | ||
| if (!this._driver.validateStyleProperty(prop)) { | ||
| context.errors.push(`The provided animation property "${ | ||
| prop}" is not a supported CSS property for animations`); | ||
|
||
| return; | ||
| } | ||
|
|
||
| // This is guaranteed to have a defined Map at this querySelector location making it | ||
| // safe to add the assertion here. It is set as a default empty map in prior methods. | ||
| const collectedStyles = context.collectedStyles.get(context.currentQuerySelector!)!; | ||
jessicajaniuk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const collectedEntry = collectedStyles.get(prop); | ||
| let updateCollectedStyle = true; | ||
| if (collectedEntry) { | ||
| if (startTime != endTime && startTime >= collectedEntry.startTime && | ||
|
|
@@ -333,11 +329,11 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor { | |
| } | ||
|
|
||
| if (updateCollectedStyle) { | ||
| collectedStyles.set(prop, {startTime, endTime}); | ||
| } | ||
|
|
||
| if (context.options) { | ||
| validateStyleParams(value, context.options, context.errors); | ||
| } | ||
| }); | ||
| }); | ||
|
|
@@ -445,7 +441,7 @@ export class AnimationAstBuilderVisitor implements AnimationDslVisitor { | |
| const [selector, includeSelf] = normalizeSelector(metadata.selector); | ||
| context.currentQuerySelector = | ||
| parentSelector.length ? (parentSelector + ' ' + selector) : selector; | ||
| getOrSetDefaultValue(context.collectedStyles, context.currentQuerySelector, new Map()); | ||
|
|
||
| const animation = visitDslNode(this, normalizeAnimationEntry(metadata.animation), context); | ||
| context.currentQuery = null; | ||
|
|
@@ -513,36 +509,34 @@ export class AnimationAstBuilderContext { | |
| public currentQuerySelector: string|null = null; | ||
| public currentAnimateTimings: TimingAst|null = null; | ||
| public currentTime: number = 0; | ||
| public collectedStyles = new Map<string, Map<string, StyleTimeTuple>>(); | ||
| public options: AnimationOptions|null = null; | ||
| constructor(public errors: string[]) {} | ||
| } | ||
|
|
||
| type OffsetStyles = string|ɵStyleDataMap; | ||
|
|
||
| function consumeOffset(styles: OffsetStyles|Array<OffsetStyles>): number|null { | ||
| if (typeof styles == 'string') return null; | ||
|
|
||
| let offset: number|null = null; | ||
|
|
||
| if (Array.isArray(styles)) { | ||
jessicajaniuk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| styles.forEach(styleTuple => { | ||
| if (styleTuple instanceof Map && styleTuple.has('offset')) { | ||
| const obj = styleTuple as ɵStyleDataMap; | ||
| offset = parseFloat(obj.get('offset') as string); | ||
| obj.delete('offset'); | ||
| } | ||
| }); | ||
| } else if (styles instanceof Map && styles.has('offset')) { | ||
| const obj = styles; | ||
| offset = parseFloat(obj.get('offset') as string); | ||
| obj.delete('offset'); | ||
| } | ||
| return offset; | ||
| } | ||
|
|
||
| function constructTimingAst(value: string|number|AnimateTimings, errors: string[]) { | ||
| let timings: AnimateTimings|null = null; | ||
| if (value.hasOwnProperty('duration')) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I like the
StyleDataas a symbol name a bit more. TheMappart leaks the implementation detail into the name (for ex. we'd need to change the name if we later decide to use an array).This is a very minor thing, not a blocker for this PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually agree, but we still use the old symbol. We'd need a new name. I'll think on it for a potential refactor after this PR.