Memory leak in AnimationUpdater; no destroy() method

[travislaynewilson]

The AnimationUpdater class collects each gauge instance and hangs on to it, and we don't seem to have a way to clear or remove it. There doesn't appear to be a way to destroy a gauge instance, either.

This is creating a large memory leak in my SPA application where the old Gauge references are hanging on to DOM objects that no longer exist.

We need a way to destroy instances that we no longer need.

[kplindegaard]

You are probably right there you cannot remove anything from the AninmationUpdater. Once it has the reference to a gauge object, it stays there. Feel free to submit a pull request with a fix for that.

But why do you need to delete (and I guess recreate from scratch) DOM elements in the first place? Is it really necessary?

[northkode]

Because for single page applications if you navigate to another page that element sits in memory and eventually your applications slow down. Its always important to clean up elements and event listeners for any sort of plugin someone builds

[zigmok]

Same here, at some point it crashes. It would be nice to have a way to empty all collected gauges.

[kplindegaard]

I'll make a modification to the AnimationUpdater so it will let go of gauges and donuts when they have stabilized and are done rendering themselves. That should be sufficient to avoid the memory leak.

[northkode]

I think adding a destroy function would be better that allows the developer to clean up the component when they want. And you build the functionality to cleanup the component inside that. The animation is not the part that concerns me for memory leaks it's the left over artifacts of Dom items if events and other data isn't purged after use.

On Sat., Jun. 15, 2019, 7:04 a.m. Karl-Petter Lindegaard, < notifications@github.com> wrote:

I'll make a modification to the AnimationUpdater so it will let go of gauges and donuts when they have stabilized and are done rendering themselves. That should be sufficient to avoid the memory leak.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bernii/gauge.js/issues/189?email_source=notifications&email_token=ABVKCMZYGKDEDVPYGP7AZ6DP2TSFZA5CNFSM4FPLDX62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXYXYNA#issuecomment-502365236, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVKCM3HFVPRVD3C24UUFL3P2TSFZANCNFSM4FPLDX6Q .

[kplindegaard]

Fixed the AnimationUpdater, which was what I had time for right now. But as you say, @northkode, it's maybe not the ultimate solution.

[northkode]

Sounds good!

On Sat., Jun. 15, 2019, 7:55 a.m. Karl-Petter Lindegaard, < notifications@github.com> wrote:

Fixed the AnimationUpdater, which was what I had time for right now. But as you say, @northkode https://github.com/northkode, it's maybe not the ultimate solution.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bernii/gauge.js/issues/189?email_source=notifications&email_token=ABVKCM5XKPBRCJXLELVCTLLP2TYDVA5CNFSM4FPLDX62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXYYVQA#issuecomment-502368960, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVKCMYDPF5XBKQT2JJQF6LP2TYDVANCNFSM4FPLDX6Q .