feat(core): add enter and leave animation instructions #62682

thePunderWoman · 2025-07-17T10:18:58Z

This adds the instructions to support enter and leave animations on nodes.

Does this PR introduce a breaking change?

Yes
No

packages/core/src/application/application_tokens.ts

packages/core/src/animation.ts

github-actions · 2025-07-17T11:57:47Z

Deployed adev-preview for 85f73c2 to: https://ng-dev-previews-fw--pr-angular-angular-62682-adev-prev-yvy73xs1.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

JeanMeche · 2025-07-17T12:24:54Z

packages/core/src/animation.ts

+  animate(el: Element, removeFn: Function): void {
+    if (!this.outElements.has(el)) return;
+    const details = this.outElements.get(el)!;
+    const timeout = setTimeout(() => removeFn(), 4000);


4000 feels like a magic number here. Can we use a named a named constant and/or have a comment on why this value. (Would animations longer than 4s just get removed after ther 4s ?)

Certainly. I've added a const and associated comment about why 4 seconds.

Also to quickly answer your question here, if you're using an animation function (rather than classes) for animate.leave, you are required to call the complete function to tell the framework when to remove the element. The feedback on the RFC was to have a timeout for safety, and so I went with the same timeout duration as cross document view transitions. So after a 4 second timeout, the element gets removed. If your animation is done before that, no issues.

What if we can have an injection token that configures it? And the default would still be 4s.

We can keep it like this for now, but we should probably output a dev-mode warning for cases when timeout is triggered (for ex. for cases when the "done" callback is never invoked). That can help to surface this situations and we can see if there are cases when the timeout needs to be adjusted.

A couple additional thoughts:

This can be done in a followup PR

We should consider cases when those warnings are produced for elements inside of @for loops and avoid spamming console with those warnings

packages/platform-browser/src/dom/dom_renderer.ts

packages/core/test/acceptance/animation_spec.ts

AndrewKushnir · 2025-07-17T23:05:11Z

goldens/public-api/core/index.api.md

@@ -667,6 +679,23 @@ export class ElementRef<T = any> {
    nativeElement: T;
 }

+// @public
+export class ElementRegistry {


Do we need this symbol in the public API, was it intentionally exposed?

AndrewKushnir · 2025-07-17T23:07:38Z

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

@@ -96,6 +97,7 @@
      "EffectRefImpl",
      "EffectScheduler",
      "ElementRef",
+      "ElementRegistry",


This looks unexpected, should we try making it tree-shakable (maybe via using lightweight injection token in the non-tree-shakable part and providing an actual implementation once the feature is brought it via special instructions)?

I'm not sure how we would be able to do this since there's no provider function for Animations.

We could use the features prop, like we did for standalone in the past (#58288 removed it)

We could use the features prop, like we did for standalone in the past (#58288 removed it)

+1, left a comment here: #62682 (comment)

packages/core/src/animation.ts

AndrewKushnir · 2025-07-17T23:38:46Z

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

+ *
+ * @codeGenApi
+ */
+export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEnter {


This function is pretty long, so I was thinking that it might benefit from either having more docs, explaining the overall logic and/or refactor the logic and extract it into multiple functions (I suspect that some of the functions can also be reused across instructions within this file).

Yeah there was a lot of currying happening here because of needed variable values and wanting to be able to clean up the event listeners. I've switched to listeners that only fire once now, which allowed this to be broken up. So, this should hopefully work well. Otherwise I'll have to figure out a good cleanup solution.

Update: We really only need to clean up the listeners for animate.enter and only two of them. I've found a nice clean way to deal with that. This should be decently organized now.

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

AndrewKushnir · 2025-07-17T23:52:20Z

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

+): LongestAnimation | undefined {
+  if (!(event.target instanceof Element)) return;
+  const nativeElement = event.target;
+  if (typeof nativeElement.getAnimations === 'function') {


We have this check in a few places and I was wondering if we are checking for the getAnimations function presence on a given element or it's a test whether it's generally supported in the current environment? If the latter - we can just do it once somewhere and reuse the result.

This is primarily because getAnimations does not exist outside of a browser environment. There's two cases where we check it, one is here and the other is when we set up a cancellation on enter animations. I don't know that it makes sense to use it only once in that case. The cancellation function only is used in animateEnter cases.

Got it, thanks. My thinking was to have this check somewhere at the module level, e.g. check if document.body.getAnimations is present and just use this info everywhere. The problem that in this case well be bypassing renderer completely, which may not be great (but likely in practice it'd not be a big problem). Maybe there are some other ways to do it? WDYT?

Another idea is that we can move this check from this utility function into an instruction, so that we just exit early (similar to SSR checks).

AndrewKushnir · 2025-07-17T23:54:18Z

packages/platform-browser/src/dom/dom_renderer.ts

@@ -273,6 +281,7 @@ class DefaultDomRenderer2 implements Renderer2 {
    private readonly ngZone: NgZone,
    private readonly platformIsServer: boolean,
    private readonly tracingService: TracingService<TracingSnapshot> | null,
+    private readonly registry: ElementRegistry | null,


This is likely where the ElementRegistry symbol is "leaking" (becomes non-tree-shakable).

Yeah. The DOM renderer is the key place this registry is needed to delay removal of elements.

thePunderWoman · 2025-07-18T12:56:36Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces support for animate.enter and animate.leave instructions for animations on nodes. The changes include new public APIs like AnimationCallbackEvent, AnimationFunction, and ANIMATIONS_DISABLED, a new ElementRegistry service to manage animations for leaving elements, and the core implementation in the rendering instructions. The changes are well-supported by a comprehensive suite of acceptance tests. I've identified a couple of potential issues in packages/core/src/animation.ts and packages/core/src/render3/instructions/animation.ts. Please see my detailed comments.

packages/core/src/animation.ts

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

This adds the instructions to support enter and leave animations on nodes.

AndrewKushnir · 2025-07-18T18:27:22Z

packages/core/src/errors.ts

@@ -92,7 +92,7 @@ export const enum RuntimeErrorCode {
  ASSERTION_NOT_INSIDE_REACTIVE_CONTEXT = -602,

  // Styling Errors


We'd probably need to update a section title above (that says "// Styling Errors").

AndrewKushnir · 2025-07-18T18:30:58Z

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

+ * @codeGenApi
+ */
+export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEnter {
+  performanceMarkFeature('NgAnimateEnter');


Should we just have 1 metric that we track (e.g. something like performanceMarkFeature('NgLightweightAnimations')) vs having individual metrics for enter and leave?

AndrewKushnir · 2025-07-18T18:32:15Z

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

+    return ɵɵanimateEnter;
+  }
+
+  assertAnimationTypes(value, 'animate.enter');


The dev mode check should be present at this level, otherwise the function itself would be retained (the contents of the function would be tree-shaken, but not the function def itself).

AndrewKushnir · 2025-07-18T18:34:49Z

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

+  const tNode = getCurrentTNode()!;
+  const nativeElement = getNativeByTNode(tNode, lView) as HTMLElement;
+
+  if ((nativeElement as Node).nodeType !== Node.ELEMENT_NODE) {


Curious what'd be an example of such usage (I believe host bindings might be in that category)? If we come up with an example, we can see what'd be the best way to handle it and whether the code should be at compile-time (preferable, if possible) or at runtime.

AndrewKushnir · 2025-07-18T18:40:46Z

packages/core/src/animation.ts

+  animate(el: Element, removeFn: Function): void {
+    if (!this.outElements.has(el)) return;
+    const details = this.outElements.get(el)!;
+    const timeout = setTimeout(() => removeFn(), 4000);


We can keep it like this for now, but we should probably output a dev-mode warning for cases when timeout is triggered (for ex. for cases when the "done" callback is never invoked). That can help to surface this situations and we can see if there are cases when the timeout needs to be adjusted.

A couple additional thoughts:

This can be done in a followup PR

We should consider cases when those warnings are produced for elements inside of @for loops and avoid spamming console with those warnings

AndrewKushnir · 2025-07-18T18:57:43Z

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

+  const elementRegistry = lView[INJECTOR]!.get(ElementRegistry);
+  const animationsDisabled = lView[INJECTOR]!.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);


Suggested change

const elementRegistry = lView[INJECTOR]!.get(ElementRegistry);

const animationsDisabled = lView[INJECTOR]!.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);

const injector = lView[INJECTOR]!;

const elementRegistry = injector.get(ElementRegistry);

const animationsDisabled = injector.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);

AndrewKushnir · 2025-07-18T19:04:24Z

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

+): LongestAnimation | undefined {
+  if (!(event.target instanceof Element)) return;
+  const nativeElement = event.target;
+  if (typeof nativeElement.getAnimations === 'function') {


Got it, thanks. My thinking was to have this check somewhere at the module level, e.g. check if document.body.getAnimations is present and just use this info everywhere. The problem that in this case well be bypassing renderer completely, which may not be great (but likely in practice it'd not be a big problem). Maybe there are some other ways to do it? WDYT?

Another idea is that we can move this check from this utility function into an instruction, so that we just exit early (similar to SSR checks).

AndrewKushnir · 2025-07-18T19:14:28Z

packages/core/src/animation.ts

+ */
+@Injectable({providedIn: 'root'})
+export class ElementRegistry {
+  private outElements = new Map<Element, AnimationDetails>();


Should it be a WeakMap instead?

AndrewKushnir · 2025-07-18T19:25:01Z

packages/platform-browser/src/dom/dom_renderer.ts

+    if (this.registry && this.registry.has(oldChild as Element)) {
+      this.registry.animate(oldChild, (timeout: ReturnType<typeof setTimeout>) => {
+        clearTimeout(timeout);
+        this.registry?.remove(oldChild);
+        oldChild.remove();
+      });
+      return;
+    }
+    // child was removed
    oldChild.remove();


We should try to minimize the amount of non-tree-shakable code here (in favor of having more code inside of the registry itself):

Suggested change

if (this.registry && this.registry.has(oldChild as Element)) {

this.registry.animate(oldChild, (timeout: ReturnType<typeof setTimeout>) => {

clearTimeout(timeout);

this.registry?.remove(oldChild);

oldChild.remove();

});

return;

}

// child was removed

oldChild.remove();

if (!this.registry?.animateRemoval(oldChild, () => oldChild.remove())) {

// Either animation registry is not present (i.e. a component doesn't use animations)

// or that element was not in the registry. If that's the case - remove the element here.

// Otherwise, it'd be removed by the animation logic at the end of the animation.

oldChild.remove();

}

AndrewKushnir · 2025-07-18T19:36:42Z

packages/platform-browser/src/dom/dom_renderer.ts

+    @Inject(ElementRegistry)
+    private readonly registry: ElementRegistry | null,


Based on the current implementation, the ElementRegistry would always be present (since it's provided in root).

I like the idea that @JeanMeche proposed: we can use "features" subsystem to bring in ElementRegistry in a tree-shakable way. Another benefit would be that the ElementRegistry would not need to be exposed via public API, since renderers would not need to have it as an argument. We'll be able to retrieve it from the component type (see type: RendererType2 in the createRenderer method below). The "feature" would basically be a function that given an injector, returns an instance of the ElementRegistry (e.g. calls injector.get(ElementRegistry, ...)).

thePunderWoman added area: core Issues related to the framework runtime target: minor This PR is targeted for the next minor release labels Jul 17, 2025

ngbot bot added this to the Backlog milestone Jul 17, 2025

angular-robot bot added the detected: feature PR contains a feature commit label Jul 17, 2025

thePunderWoman force-pushed the animate-instructions branch 2 times, most recently from 843db01 to 46066a1 Compare July 17, 2025 11:04

pullapprove bot requested review from crisbeto, kirjs and mmalerba July 17, 2025 11:04

thePunderWoman force-pushed the animate-instructions branch from 46066a1 to ba72fa8 Compare July 17, 2025 11:26

JeanMeche reviewed Jul 17, 2025

View reviewed changes

packages/core/src/application/application_tokens.ts Show resolved Hide resolved

thePunderWoman force-pushed the animate-instructions branch from ba72fa8 to 594389f Compare July 17, 2025 11:38

JeanMeche added the adev: preview label Jul 17, 2025

JeanMeche reviewed Jul 17, 2025

View reviewed changes

packages/core/src/animation.ts Outdated Show resolved Hide resolved

thePunderWoman force-pushed the animate-instructions branch 2 times, most recently from 3c38d4a to 056dcb3 Compare July 17, 2025 11:53

JeanMeche reviewed Jul 17, 2025

View reviewed changes

packages/platform-browser/src/dom/dom_renderer.ts Outdated Show resolved Hide resolved

michael-small reviewed Jul 17, 2025

View reviewed changes

packages/core/test/acceptance/animation_spec.ts Outdated Show resolved Hide resolved

thePunderWoman force-pushed the animate-instructions branch 2 times, most recently from 72ef660 to 4d2fc63 Compare July 17, 2025 14:34

thePunderWoman added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 17, 2025

AndrewKushnir reviewed Jul 17, 2025

View reviewed changes

thePunderWoman force-pushed the animate-instructions branch 2 times, most recently from 277e2bf to e6569a6 Compare July 18, 2025 12:25

gemini-code-assist bot reviewed Jul 18, 2025

View reviewed changes

packages/core/src/animation.ts Outdated Show resolved Hide resolved

packages/core/src/animation.ts Outdated Show resolved Hide resolved

packages/core/src/render3/instructions/animation.ts Outdated Show resolved Hide resolved

feat(core): add enter and leave animation instructions

85f73c2

This adds the instructions to support enter and leave animations on nodes.

thePunderWoman force-pushed the animate-instructions branch from e6569a6 to 85f73c2 Compare July 18, 2025 13:42

AndrewKushnir reviewed Jul 18, 2025

View reviewed changes

		@@ -92,7 +92,7 @@ export const enum RuntimeErrorCode {
		ASSERTION_NOT_INSIDE_REACTIVE_CONTEXT = -602,

		// Styling Errors

		const elementRegistry = lView[INJECTOR]!.get(ElementRegistry);
		const animationsDisabled = lView[INJECTOR]!.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);

-  const elementRegistry = lView[INJECTOR]!.get(ElementRegistry);
-  const animationsDisabled = lView[INJECTOR]!.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);
+  const injector = lView[INJECTOR]!;
+  const elementRegistry = injector.get(ElementRegistry);
+  const animationsDisabled = injector.get(ANIMATIONS_DISABLED, DEFAULT_ANIMATIONS_DISABLED);

		@Inject(ElementRegistry)
		private readonly registry: ElementRegistry \| null,

feat(core): add enter and leave animation instructions #62682

Are you sure you want to change the base?

feat(core): add enter and leave animation instructions #62682

Conversation

thePunderWoman commented Jul 17, 2025

Does this PR introduce a breaking change?

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeanMeche Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thePunderWoman Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeanMeche Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thePunderWoman Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thePunderWoman commented Jul 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 17, 2025 •

edited

Loading

JeanMeche Jul 17, 2025 •

edited

Loading

thePunderWoman Jul 17, 2025 •

edited

Loading

JeanMeche Jul 18, 2025 •

edited

Loading

AndrewKushnir Jul 18, 2025 •

edited

Loading

thePunderWoman Jul 18, 2025 •

edited

Loading