Skip to content

Commit af63b80

Browse files
cexbrayatcrisbeto
cexbrayat
authored and
crisbeto
committed
refactor(core): improve animation instruction robustness and maintainability (#63163)
This commit extracts helper functions to reduce code duplication across animation instructions and adds early return when animations are disabled. PR Close #63163
1 parent 9e828d5 commit af63b80

File tree

1 file changed

+71
-55
lines changed

1 file changed

+71
-55
lines changed

‎packages/core/src/render3/instructions/animation.ts‎

Lines changed: 71 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@ import {
1515
AnimationRemoveFunction,
1616
ANIMATIONS_DISABLED,
1717
} from '../../animation/interfaces';
18-
import {getClassListFromValue} from '../../animation/element_removal_registry';
18+
import {
19+
AnimationRemovalRegistry,
20+
getClassListFromValue,
21+
} from '../../animation/element_removal_registry';
1922
import {getLView, getCurrentTNode, getTView, getAnimationElementRemovalRegistry} from '../state';
20-
import {RENDERER, INJECTOR, CONTEXT, FLAGS, LViewFlags} from '../interfaces/view';
23+
import {RENDERER, INJECTOR, CONTEXT, FLAGS, LViewFlags, LView, TView} from '../interfaces/view';
2124
import {RuntimeError, RuntimeErrorCode} from '../../errors';
2225
import {getNativeByTNode, storeCleanupWithContext} from '../util/view_utils';
2326
import {performanceMarkFeature} from '../../util/performance';
@@ -35,6 +38,44 @@ const areAnimationSupported =
3538
// tslint:disable-next-line:no-toplevel-property-access
3639
typeof document?.documentElement?.getAnimations === 'function';
3740

41+
/**
42+
* Helper function to check if animations are disabled via injection token
43+
*/
44+
function areAnimationsDisabled(lView: LView): boolean {
45+
const injector = lView[INJECTOR]!;
46+
return injector.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);
47+
}
48+
49+
/**
50+
* Helper function to setup element registry cleanup when LView is destroyed
51+
*/
52+
function setupElementRegistryCleanup(
53+
elementRegistry: AnimationRemovalRegistry,
54+
lView: LView,
55+
tView: TView,
56+
nativeElement: Element,
57+
): void {
58+
if (lView[FLAGS] & LViewFlags.FirstLViewPass) {
59+
storeCleanupWithContext(tView, lView, nativeElement, (elToClean: Element) => {
60+
elementRegistry.elements!.remove(elToClean);
61+
});
62+
}
63+
}
64+
65+
/**
66+
* Helper function to cleanup enterClassMap data safely
67+
*/
68+
function cleanupEnterClassData(element: HTMLElement): void {
69+
const elementData = enterClassMap.get(element);
70+
if (elementData) {
71+
for (const fn of elementData.cleanupFns) {
72+
fn();
73+
}
74+
enterClassMap.delete(element);
75+
}
76+
longestAnimations.delete(element);
77+
}
78+
3879
const noOpAnimationComplete = () => {};
3980

4081
// Tracks the list of classes added to a DOM node from `animate.enter` calls to ensure
@@ -50,7 +91,7 @@ const leavingNodes = new WeakMap<TNode, HTMLElement[]>();
5091
/**
5192
* Instruction to handle the `animate.enter` behavior for class bindings.
5293
*
53-
* @param value The value bound to `animate.enter`, which is a string or a string array.
94+
* @param value The value bound to `animate.enter`, which is a string or a function.
5495
* @returns This function returns itself so that it may be chained.
5596
*
5697
* @codeGenApi
@@ -61,21 +102,17 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn
61102
if ((typeof ngServerMode !== 'undefined' && ngServerMode) || !areAnimationSupported) {
62103
return ɵɵanimateEnter;
63104
}
64-
65105
ngDevMode && assertAnimationTypes(value, 'animate.enter');
66106

67107
const lView = getLView();
108+
if (areAnimationsDisabled(lView)) {
109+
return ɵɵanimateEnter;
110+
}
111+
68112
const tNode = getCurrentTNode()!;
69113
const nativeElement = getNativeByTNode(tNode, lView) as HTMLElement;
70-
71114
const renderer = lView[RENDERER];
72-
const injector = lView[INJECTOR]!;
73-
const animationsDisabled = injector.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);
74-
const ngZone = injector.get(NgZone);
75-
76-
if (animationsDisabled) {
77-
return ɵɵanimateEnter;
78-
}
115+
const ngZone = lView[INJECTOR]!.get(NgZone);
79116

80117
// Retrieve the actual class list from the value. This will resolve any resolver functions from
81118
// bindings.
@@ -97,7 +134,7 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn
97134

98135
// When the longest animation ends, we can remove all the classes
99136
const handleInAnimationEnd = (event: AnimationEvent | TransitionEvent) => {
100-
animationEnd(event, nativeElement, renderer, cleanupFns);
137+
animationEnd(event, nativeElement, renderer);
101138
};
102139

103140
// We only need to add these event listeners if there are actual classes to apply
@@ -119,7 +156,7 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn
119156
trackEnterClasses(nativeElement, activeClasses, cleanupFns);
120157

121158
for (const klass of activeClasses) {
122-
renderer.addClass(nativeElement as HTMLElement, klass);
159+
renderer.addClass(nativeElement, klass);
123160
}
124161
}
125162

@@ -165,14 +202,12 @@ export function ɵɵanimateEnterListener(value: AnimationFunction): typeof ɵɵa
165202
ngDevMode && assertAnimationTypes(value, 'animate.enter');
166203

167204
const lView = getLView();
168-
const tNode = getCurrentTNode()!;
169-
const nativeElement = getNativeByTNode(tNode, lView) as HTMLElement;
170-
const animationsDisabled = lView[INJECTOR]!.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);
171-
172-
if (animationsDisabled) {
205+
if (areAnimationsDisabled(lView)) {
173206
return ɵɵanimateEnterListener;
174207
}
175208

209+
const tNode = getCurrentTNode()!;
210+
const nativeElement = getNativeByTNode(tNode, lView) as HTMLElement;
176211
value.call(lView[CONTEXT], {target: nativeElement, animationComplete: noOpAnimationComplete});
177212

178213
return ɵɵanimateEnterListener;
@@ -183,7 +218,7 @@ export function ɵɵanimateEnterListener(value: AnimationFunction): typeof ɵɵa
183218
* It registers an animation with the ElementRegistry to be run when the element
184219
* is scheduled for removal from the DOM.
185220
*
186-
* @param value The value bound to `animate.leave`, which can be a string or string array.
221+
* @param value The value bound to `animate.leave`, which can be a string or a function.
187222
* @returns This function returns itself so that it may be chained.
188223
*
189224
* @codeGenApi
@@ -198,24 +233,24 @@ export function ɵɵanimateLeave(value: string | Function): typeof ɵɵanimateLe
198233
ngDevMode && assertAnimationTypes(value, 'animate.leave');
199234

200235
const lView = getLView();
236+
const animationsDisabled = areAnimationsDisabled(lView);
237+
if (animationsDisabled) {
238+
return ɵɵanimateLeave;
239+
}
240+
201241
const tView = getTView();
202242
const tNode = getCurrentTNode()!;
203243
const nativeElement = getNativeByTNode(tNode, lView) as Element;
204244

205245
// This instruction is called in the update pass.
206246
const renderer = lView[RENDERER];
207-
const injector = lView[INJECTOR]!;
208-
209-
// Assume ElementRegistry and ANIMATIONS_DISABLED are injectable services.
210247
const elementRegistry = getAnimationElementRemovalRegistry();
211248
ngDevMode &&
212249
assertDefined(
213250
elementRegistry.elements,
214251
'Expected `ElementRegistry` to be present in animations subsystem',
215252
);
216-
217-
const animationsDisabled = injector.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);
218-
const ngZone = injector.get(NgZone);
253+
const ngZone = lView[INJECTOR]!.get(NgZone);
219254

220255
// This function gets stashed in the registry to be used once the element removal process
221256
// begins. We pass in the values and resolvers so as to evaluate the resolved classes
@@ -240,12 +275,7 @@ export function ɵɵanimateLeave(value: string | Function): typeof ɵɵanimateLe
240275
};
241276

242277
// Ensure cleanup if the LView is destroyed before the animation runs.
243-
if (lView[FLAGS] & LViewFlags.FirstLViewPass) {
244-
storeCleanupWithContext(tView, lView, nativeElement, (elToClean: Element) => {
245-
elementRegistry.elements!.remove(elToClean);
246-
});
247-
}
248-
278+
setupElementRegistryCleanup(elementRegistry, lView, tView, nativeElement);
249279
elementRegistry.elements!.add(nativeElement, value, animate);
250280

251281
return ɵɵanimateLeave; // For chaining
@@ -271,6 +301,10 @@ export function ɵɵanimateLeaveListener(value: AnimationFunction): typeof ɵɵa
271301

272302
ngDevMode && assertAnimationTypes(value, 'animate.leave');
273303

304+
// Even when animations are disabled, we still need to register the element for removal
305+
// to ensure proper cleanup and allow developers to handle element removal in tests
306+
// So we don't have an early return here.
307+
274308
const lView = getLView();
275309
const tNode = getCurrentTNode()!;
276310
const tView = getTView();
@@ -280,19 +314,16 @@ export function ɵɵanimateLeaveListener(value: AnimationFunction): typeof ɵɵa
280314
return ɵɵanimateLeaveListener;
281315
}
282316

283-
// Assume ElementRegistry and ANIMATIONS_DISABLED are injectable services.
284-
const injector = lView[INJECTOR]!;
285317
const elementRegistry = getAnimationElementRemovalRegistry();
286318
ngDevMode &&
287319
assertDefined(
288320
elementRegistry.elements,
289321
'Expected `ElementRegistry` to be present in animations subsystem',
290322
);
291-
292-
const animationsDisabled = injector.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);
323+
const animationsDisabled = areAnimationsDisabled(lView);
293324

294325
const animate: AnimationEventFunction = (
295-
el: Element,
326+
_el: Element,
296327
value: AnimationFunction,
297328
): AnimationRemoveFunction => {
298329
return (removeFn: VoidFunction): void => {
@@ -311,11 +342,7 @@ export function ɵɵanimateLeaveListener(value: AnimationFunction): typeof ɵɵa
311342
};
312343

313344
// Ensure cleanup if the LView is destroyed before the animation runs.
314-
if (lView[FLAGS] & LViewFlags.FirstLViewPass) {
315-
storeCleanupWithContext(tView, lView, nativeElement, (elToClean: Element) => {
316-
elementRegistry.elements!.remove(elToClean);
317-
});
318-
}
345+
setupElementRegistryCleanup(elementRegistry, lView, tView, nativeElement);
319346
elementRegistry.elements!.addCallback(nativeElement, value, animate);
320347

321348
return ɵɵanimateLeaveListener; // For chaining
@@ -360,13 +387,7 @@ function cancelAnimationsIfRunning(element: HTMLElement, renderer: Renderer): vo
360387
}
361388
}
362389
// We need to prevent any enter animation listeners from firing if they exist.
363-
if (elementData) {
364-
for (const fn of elementData.cleanupFns) {
365-
fn();
366-
}
367-
}
368-
longestAnimations.delete(element);
369-
enterClassMap.delete(element);
390+
cleanupEnterClassData(element);
370391
}
371392

372393
function setupAnimationCancel(event: Event, renderer: Renderer) {
@@ -407,7 +428,6 @@ function animationEnd(
407428
event: AnimationEvent | TransitionEvent,
408429
nativeElement: HTMLElement,
409430
renderer: Renderer,
410-
cleanupFns: Function[],
411431
) {
412432
const elementData = enterClassMap.get(nativeElement);
413433
if (!elementData) return;
@@ -420,11 +440,7 @@ function animationEnd(
420440
for (const klass of elementData.classList) {
421441
renderer.removeClass(nativeElement, klass);
422442
}
423-
enterClassMap.delete(nativeElement);
424-
longestAnimations.delete(nativeElement);
425-
for (const fn of cleanupFns) {
426-
fn();
427-
}
443+
cleanupEnterClassData(nativeElement);
428444
}
429445
}
430446

0 commit comments

Comments
 (0)