Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Velocity animator appears to be leaking memory #19

Open
estaylorco opened this issue Apr 1, 2017 · 6 comments
Open

The Velocity animator appears to be leaking memory #19

estaylorco opened this issue Apr 1, 2017 · 6 comments

Comments

@estaylorco
Copy link

I'm submitting a bug report

  • Library Version:
    1.1.0

Please tell us about your environment:

  • Operating System:
    OSX 10.12.4

  • Node Version:
    7.3.0

  • NPM Version:
    3.10.10
  • JSPM OR Webpack AND Version
    JSPM 0.16.52
  • Browser:
    all

  • Language:
    ESNext

Current behavior:
Simply using the Velocity animator in the following manner causes my transient view to remain in memory when navigating away (where this problem does not occur when I remove calls to the animator):

this.animator.animate(document.getElementById('recoveryErrorBox'), 'callout.shake');

I can actually recreate the problem at will by simply commenting any calls to the animator, and then uncommenting. But apart from the memory leak, everything works correctly.

Also, I should point out that I'm not using the declarative approach to animations at all. All animation is handled in the viewModel.

I believe the problem to be that there is no Velocity-oriented destroy method to call on the nodes involved in animation, and simply letting detach() do its thing is failing (the DOM nodes involved in animation remain).

DIAGNOSTIC STEPS TAKEN

  1. Rewound all the way to the point of simply importing and injecting the VelocityAnimator to see if the mere presence of it in the viewModel was the culprit. Not the case.

  2. I was calling animations in a task added to the taskQueue as a microTask, which led me to believe that I had a closure issue. I unwrapped the animation calls so that they were not being called from a microTask. It appears that calling from a microTask is not a factor in this memory leak.

  3. Calling this.animator.animate(...) from anywhere within the viewModel--whether from within a microTask or not--was, in fact, the culprit.

Expected/desired behavior:

  • What is the expected behavior?
    The VelocityAnimator should not be stranding DOM nodes so that they cannot be removed by Aurelia.

If the VelocityAnimator does require specific steps in the detach() handler, some guidance would be helpful.

@estaylorco
Copy link
Author

I think we can close this issue. I just removed aurelia-animator-velocity from my project, downloaded VelocityJS, and referenced it through a script tag in index.html...in the usual way. I then replaced every instance of this.animator.animate(...) with Velocity(...), according to Velocity's documentation, and no change.

Everything works perfectly, but Velocity is leaking. I think it's safe to say that it has nothing to do with Aurelia. I will pursue the issue through Velocity's support.

@estaylorco estaylorco reopened this Apr 3, 2017
@estaylorco
Copy link
Author

estaylorco commented Apr 3, 2017

In light of the response I got from the maintainers of the Velocity project, I decided to reopen this issue.

Please see issue #769, which I opened, in their Issues, along with the response.

Unless the Aurelia team has some advice or superior experience with the Velocity animator to offer up, I would say that the Velocity animator is broken.

@estaylorco
Copy link
Author

Aurelia Team and Community,

My apologies.

I just revised by most recent comment from "...has some advice or better experience to offer up..." to "...has some advice or superior experience with the Velocity animator to offer up..." I fear my previous wording might have given readers and newcomers a negative impression of the "animation experience" in Aurelia in general.

To the contrary, the animation story in Aurelia is fantastic! In light of the Velocity bug, I switched to the aurelia-animator-css. All is working perfectly and there are no memory leaks! Because the Aurelia team has abstracted animation, switching from VelocityAnimator to CssAnimator was absolutely trivial. It took about 10 minutes, the kind of efficiency we would expect from a well-written abstraction.

I will keep an eye on the issue I posted with the Velocity team and come back here if/once there is resolution.

@EisenbergEffect
Copy link
Contributor

Thanks @estaylorco 😄
We're glad to have great community members like you here with us.

@estaylorco
Copy link
Author

Thank you. Kind words.

@Alexander-Taran
Copy link

Alexander-Taran commented May 24, 2022

@estaylorco would you close this, please?

or @zewa666

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants