Possible memory leak?

[francescozaniol]

Hi, unless I'm doing something wrong, this extension seems to be causing a memory leak in my SPA vue app. To be more precise it seems that when an element with "v-click-outside" gets destroyed (by a route change) not all references to the doms are properly removed and they stay as "detached" elements in the memory. See the two imgs attached:

In this memory snapshot I've got 4645 detached elements, and all of them have v-click-outside in common. This causes memory to keep going up on every route change.
In the following screenshot I've removed the v-click-outside directive and I end up with just one detached elements, which seems to be keeping memory stable:

Am I missing something?
Thanks!

[ndelvalle]

@francescozaniol thanks for reporting the issue, I will take a look 👍

[theprobugmaker]

Is this affecting only SPA?

[ndelvalle]

@zefexdeveloper yes this issue affects SPA mostly if you refresh the page the problem goes away.
@francescozaniol I am investigating the issue, the event listeners are properly removed but still, there are some Detached HTMLDiv Element left. I am updating the directive implementation in this branch, I also added an example project to be able to reproduce the problem if you want to play around.
I tested some other event listeners related directives and they have the same problem, even tho the listener is properly removed.

[francescozaniol]

Thanks @ndelvalle , I'll have a look as soon as I can

[francescozaniol]

Hi @ndelvalle , I had a look at your example branch; actually in that example the detached nodes remain in memory because you're using console.log of "el" (file example/src/views/Home.vue); if you remove the console.logs (or you can just keep the strings) the memory snapshot remains clear from detached nodes. Can you confirm it too?
If this is the case then you successfully fixed the issue, or, it might be an issue related to my app only.
Thanks again!

[francescozaniol]

Hi again @ndelvalle , good news: I made a build from the branch you created and placed it directly inside node_modules of my app, no more detached nodes and memory is drastically lower. I can confirm the issue is gone. Thanks, well done

[ndelvalle]

Wow, thanks @francescozaniol for helping me debug the problem 🙏🏻! I will update the tests according to the changes I made and push a new version tonight.

[ndelvalle]

v-click-outside 2.1.5 published 🎉

[ndelvalle]

I should also do not expose the el to middlewares and the handler function anymore to totally avoid this issue 🤔

[francescozaniol]

Hey @ndelvalle , sorry to bother you again with this but I'm still experiencing the issue. I've updated at 2.1.5 and I've had a look at a diff and I noticed you added a setTimeout, which seems to be the culprit. Was there a particular reason for that? removing it fixes the memory leak in my app. Let me know.

[ndelvalle]

Strange, @francescozaniol this was introduced at #42

I wonder if this still happens so we can remove it, or maybe we should tackle differently. I am also preparing a new version removing the el argument in the callbacks to avoid having unexpected Detached HTMLDiv Element as you suggested.

[ndelvalle]

@francescozaniol are you sure you are not saving the el reference somewhere? I am having the same results with and without the set timeout. I like having the timeout, to avoid executing the event listener handler when navigating to the route that is using the directive without clicking anything. There are some other ways we can achieve this, but I am not sure if the time out is the problem here.

[francescozaniol]

@ndelvalle I'll do some more testing asap and I'll let you know

[francescozaniol]

@ndelvalle after quite some testing, I've found out that the issue was indeed linked with the timeout, but only with elements populated by async data. In this scenario there were cases when the "unbind" function was run before the timeout function (for reasons I cannot still quite understand ..), therefore the event was added to a detached node, causing the memory leak. Check my PR, that commit fixes the issue (at least in my app)

[francescozaniol]

Installed v3.0.0 in my SPA, confirmed memory leak is gone. Thanks again @ndelvalle