$destroy event is triggered on leaving view before animation/transition ends

[ghoullier]

When animations are associated to a view using ngAnimate, $destroy event is triggered on leaving view before animation/transition ends.

This event should be triggered after animation/transition end?

Plunker

[email protected]
[email protected]
[email protected]

[Roman-Didenko]

+1

[philBrown]

This is because this part in cleanupLastView() doesn't wait for the renderer.leave promise

if (currentScope) {
    currentScope.$destroy();
    currentScope = null;
}

[nateabele]

Okay, so, I guess it should?

[philBrown]

@nateabele yes, because until the animation completes, the leaving state's scope is still relevant.

For example, I have an Highcharts chart in my view (using highcharts-ng). When I transition to another state via an animation, the chart is destroyed (and removed from the DOM) before the animation completes.

[nateabele]

Well, I'll patch it as soon as I can, unless you think you can do it faster.

[manro]

I think this code change would help:

Current cleanupLastView function:

function cleanupLastView() {
  if (previousEl) {
    previousEl.remove();
    previousEl = null;
  }

  if (currentScope) {
    currentScope.$destroy();
    currentScope = null;
  }

  if (currentEl) {
    renderer.leave(currentEl, function() {
      previousEl = null;
    });

    previousEl = currentEl;
    currentEl = null;
  }
}

cleanupLastView function with deferred animate-out element scope destroy:

function cleanupLastView() {
  if (currentEl) {
      (function(previousEl, currentScope) {
          renderer.leave(currentEl, function() {
              if (previousEl) {
                  previousEl.remove();
                  previousEl = null;
              }
              if (currentScope) {
                  currentScope.$destroy();
                  currentScope = null;
               }

               previousEl = null;
               currentScope = null;
          });
      })(previousEl, currentScope);
  } else {
      if (previousEl) {
          previousEl.remove();
          previousEl = null;
      }
      if (currentScope) {
          currentScope.$destroy();
          currentScope = null;
      }
  }
  previousEl = currentEl;
  currentEl = null;
}

[nateabele]

Great. PR + unit test?

[SmuliS]

@nateabele Ping. Now there is a PR open with tests. This is a very welcome PR if you have state change animations!

[nateabele]

@SmuliS Merged.

[ghoullier]

👍

[petermolyneux]

This fix worked for some of our situations, but made it worse for others.

The problem for us is when we have a Kendo grid on a page with a base page. When leaving the page we get an error something like "Can't set property template of null". Before the fix we got the error only if the grid was in a template for a component with transclude. Now it works fine in that situation, but we get the same error when our routing is set up to use base pages.

I think the problem with the fix is that we now don't control the order things are removed/destroyed in.

I.e. If a scope has a child scope and the child contains animations, the parent will try to broadcast destroy before the child is ready.

I hacked the code to get it to work. Basically what I do is wait for all elements to report there leave-events and then do a cleanup for all of them at once.

The code might help you understand the issue and find a good solution:

function cleanupLastView() {
    var _currentEl;
    var i = 0;

    if (currentScope) {
        currentScope._willBeDestroyed = true;
    }

    function cleanOld(prevEl, currScope) {
        if (prevEl) {
            prevEl.remove();
        }

        if (currScope) {
            try {
                currScope.$destroy();
            } catch (e) {
                currScope.$destroy();
            }
        }
    }

    if (currentEl) {
        awaitingCleanup.push({ prevEl: previousEl, currScope: currentScope, currEl: currentEl, readyForRemoval: false });
        _currentEl = currentEl;
        renderer.leave(_currentEl, function () {
            var allReady = true;
            for (i = 0; i < awaitingCleanup.length; i++) {
                if (awaitingCleanup[i].currEl === _currentEl) {
                    awaitingCleanup[i].readyForRemoval = true;
                }
                allReady = allReady && awaitingCleanup[i].readyForRemoval;
            }
            if (allReady) {
                while (awaitingCleanup.length > 0) {
                    cleanOld(awaitingCleanup[awaitingCleanup.length - 1].prevEl, awaitingCleanup[cleanUpOrder.length - 1].currScope);
                    awaitingCleanup.pop();
                }
            }
            previousEl = null;
        });

        previousEl = currentEl;
    } else {
        cleanOld(previousEl, currentScope);
        previousEl = null;
    }

    currentEl = null;
    currentScope = null;
}

Hope this help,

Peter Molyneux

[christopherthielen]

@petermolyneux I'd like to understand your use case better.

Please explain your setup.

• ui-views
• animations
• how do you utilize $destroy

If we expose the animation promise, will you be able to clean your resources using that promise, instead of $on($destroy)?

Any chance you could set up a representative plunker, so we can accomodate your use case?

[christopherthielen]

We will be reverting this change in favor of exposing the animation promise #2562

[dmitry-dedukhin]

#2614
Another terrible issue caused by this change.