Angular IVY Animation issue - element is not getting removed

[LanderBeeuwsaert]

🐞 bug report

Affected Package

Angular animations in combination with IVY, and maybe some other stuff mixed in. Unkown because unable to reproduce in stackblitz.

Is this a regression?

Yeah, sort of, I don't have the issue pre-IVY

Description

2 screenshots above. The first one has this as DOM: the class combination-container can only happen once (it's in an *ngIf). But in combination with it seems that somehow, the previous elements are not removed. I've tried my best to reproduce the situation in a stackblitz: https://stackblitz.com/edit/components-issue-tdugpz?file=src%2Fapp%2Fcontainer.ts with the minimal component logic I have. But I don't succeed. In the stackblitz it works but in the application it doesn't. It happens when the elements are changed while you're in the first tab. So it seems to have to with the fact that the fadein animation is running while the element is not on screen. If I remove the fadein animation, the elements are correctly removed.

🔬 Minimal Reproduction

https://stackblitz.com/edit/components-issue-tdugpz?file=src%2Fapp%2Fcontainer.ts => failed reproduction, but shows the hierarchy where I have the issue.

🔥 Exception or Error

None

Angular Version: 9.0.0 cli 9.0.1

[kara]

@LanderBeeuwsaert Is there any way you could reproduce this in a Github repo instead, if Stackblitz example isn't working like your app? It's hard to investigate without a working reproduction.

[LanderBeeuwsaert]

Hi Kara, last time I stripped our application down for a bughunting repo it took a lot of time, which for the moment we don't have, I'm sorry. What I always can do is have a skype or something to go through the code together. Now that I failed in the stackblitz it's the best I can offer for the moment, my apologies.

[codecoster]

@LanderBeeuwsaert does it reproduce the issue if you set ChangeDetectionStrategy.OnPush in that Component? I recently faced an Issue with Change Detection OnPush in Ivy

[codecoster]

@LanderBeeuwsaert @kara Maybe this reproduces the issue: https://stackblitz.com/edit/angular-mda6c4?file=src%2Fapp%2Fdemo%2Fdemo.component.ts If the Component is set to ChangeDetection.OnPush, the setInterval does no longer trigger change detection in Ivy

[LanderBeeuwsaert]

@codecoster Thanks for the catch, I forgot to add the onPush, all components in the hierarchy have onPush indeed. Just added it to the stackblitz but also with that I can't reproduce.

For your example you should add .detectChanges() in your interval to make it working.

[codecoster]

@LanderBeeuwsaert Indeed you need to trigger the change detection manually, but this was not neccessary before ivy, even with OnPush activated

[matsko]

@LanderBeeuwsaert @codecoster any luck with a reproduction? It's been playing with the demos, but I can't know for sure if this is an onPush or something else in Ivy? Is there anything more that can be done that can lead us towards a reproduction?

[LanderBeeuwsaert]

Hi @matsko ,

I've just played around a bit again. We're on 9.0.7 and latest Material as well. I've really isolated it to one instance of animation:

<div *ngIf="model.type === 'pair-static'" matRipple (click)="openStaticSelector()" style="display:flex;flex-direction:column;flex-grow:1;flex-shrink: 0;" [style.opacity]="model.is_1_second ? 0.4 : 1" [@fadeInOut]>

If I remove that one [@fadeInOut], the issue is gone.

I also see that there is code that on the tab animation done Output, it does this:

  async tabChangedAnimationDone() {
    if (this.selectedTabIndex === 1) {
      this.ejJudgeComponent.doWidthRecalculation();
      this.ejJudgeComponent.doWidthRecalculation();
      this.ejJudgeComponent.doWidthRecalculation();
      this.ejJudgeComponent.doWidthRecalculation();
    }
  }

which calls this code:

  doWidthRecalculation() {
    // this.sheetViewComponent.doWidthRecalculation();

    for (let i: number = 0; i < 3; i++) {
      this.sheetViewComponent.containers.forEach((container: Container) => container.doResize());
      this.sheetViewComponent.individualContainers.forEach((container: IndividualContainer) => container.doResize());

      this.sheetViewComponent.containers.forEach((container: Container) => container.doResize());
      this.sheetViewComponent.individualContainers.forEach((container: IndividualContainer) => container.doResize());
    }
  }

Which calls this code:

  doResize(timeout: number = 1) {
    //quickly make the image smaller so the pictureContainer can revert to the size it would like to be
    if (this.pictureContainer && !this.isPublicView) {
      this.pictureWidth = this.pictureContainer.nativeElement.clientWidth / 2;
      this.pictureHeight = this.pictureContainer.nativeElement.clientHeight / 2;
      this.cdr.detectChanges();
    }

    //after the pictureContainer has become smaller, take the size of it, but keep for comfort 10 px margin
    setTimeout(() => {
      if (this.pictureContainer) {
        this.pictureWidth = this.pictureContainer.nativeElement.clientWidth - 10;
        this.pictureHeight = this.pictureContainer.nativeElement.clientHeight - 10;
        this.cdr.detectChanges();
      }
    }, timeout);
  }

It's a total pain, but for the moment, it's the only way we found to resize the images correctly and have the parent divs resize correctly with them.

However so this means that on going to the tab, there are A LOT of calls to this.cdr.detectChanges() while at the same time the animation is running.

[jonnhy12333]

I am also seeing this issue (combination of using a custom RouteReuseStrategy, animation, and ngIf). I've made a repo that demonstrates how an element can be duplicated by toggling its visibility:

https://github.com/jonnhy12333/duplicate-element

The ChangeDetectionStrategy.OnPush didn't help / haven't found any workarounds other than disabling Ivy in the tsconfig.json file...

[matsko]

@jonnhy12333 excellent reproduction. I can see the issue clearly now in the demo. We've hotlisted this issue and will begin to investigate.

[Da13Harris]

This issue seems to be related to #26133

[Da13Harris]

This was occurring with high frequency in our application, which uses a custom RouteReuseStrategy extensively, so we've had to remove most of the animations until this regression bug is resolved. It's unfortunate, because some of the animations are key to managing the user experience within complex forms (like expanding grid rows and fading form controls in and out).

[temaisgod]

I have same problem in:

#38871

I included a example in stackblitz

[temaisgod]

Hi I have investigated on this problem.

Apparently, when an already instantiated view is restored, is when the triggers of the animations lose their state and everything is out of square. This causes it to fail when:

1. (RESTORE A VIEW IN ADHOST COMPONENT with Dinamyc component load)
2. (Restore Routing component with RouteReuseStrategy)

In my case when in the same HTML, i have material animations are joined with animations declared in angular, failed too the material animations. example:

<div [@sideIn]="xxxxx"></div>
<mat-sidenav></..>

Since they have closed my GIT for duplication I leave the example here for the record. I think it is very relevant: https://stackblitz.com/edit/angular-9-bugsanimations?file=src/app/app.component.ts

MORE INFO: #38871 Please, read this GIT to help the angular team solve the bug.

its a big IVY bug, tthis problem is not something new and I don't understand why it hasn't been fixed yet.

I wait impatiently for an answer. Thanks

[temaisgod]

Finally, i migrated alls animations of angular to CSS animations. Solved it but im waiting to angular team fix

[Da13Harris]

I really wish there was some visibility into whether progress can be expected on this bug. It seems to have all the tags to indicate that it's a confirmed regression due to IVY and a high priority (P2).

Last word from @matsko was...

We've hotlisted this issue and will begin to investigate.

That was almost a year ago.

[petebacondarwin]

Hi @Da13Harris - I am sorry that a lot of these animations issues have gone quiet. @matsko left the team last summer and there is currently no one focussing on these issues. So we are not able to give a strong indication on when these will be resolved.

[andreialecu]

Recently enabled RouteReuseStrategy in an app and suddenly noticed it broke our app very subtly.

In our case we had a *ngFor where each item has a :leave animation on it. Items have a Remove button on them.

Once navigating to a separate route and then back to the list of items, once the user tried to delete an entry, it will not be removed from the DOM and will stick there in the HTML until the page is refreshed.

This is because of the [@animateInOut], without it everything is fine.

My assumption is that when navigating back and forth, something about the animation is not reattached properly to the DOM node when the view is reattached.

[andreialecu]

Repro at: https://stackblitz.com/edit/angular-ivy-18qzyz?file=src/app/components/component-a/component-a.component.ts

[JoostK]

This is caused by #28162; I'm currently exploring ways to fix this issue.

[JoostK]

I have a preliminary PR up at #45516 that attempts to avoid this issue. It is non-trivial and I suspect there may be some animation scenarios that it won't cover entirely correctly, but it appears to be working in my testing. The big challenge is to come up with meaningful tests for the new behaviour, as the PR adds significant use-cases into the mix.

Build snapshots based on 14.0.0-next.10 are available here. Instructions on how to use these can be found here. If you're using 14.0.0-next.10 then using the snapshots of @angular/animations should be sufficient. I'd appreciate if any of you can test 1) whether this fixes the issue for your use-case, and 2) whether it has any undesirable side-effects.