-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
cache causes Detached Node memory leaks #115
Comments
I've done a clunky work-around on my project that may help someone who finds this issue: var onImagesLoaded = function($el, callback){
var ld = imagesLoaded($el);
ld.off('always').on('always', function(){
callback.apply(view, arguments, ld);
_.each(ld.images, function(loadingImage, index){
loadingImage.img = {src:loadingImage.img.src};
});
ld.images = [];
ld.elements = null;
ld._events = null;
});
return ld;
}; |
Thanks for digging into this! This is not good! 🐐 I've pushed an update that removes reference to any elements in Can I ask you to try out and verify? |
I've ended up moving away from your plugin and just manually checking the images that I needed to wait for loading on. I don't have an easy way of checking. Thanks for the reply and for open sourcing your project. |
Closing as fixed. |
After a day trying to figure out why I had so many detached elements in my page, I tracked it down to ImagesLoaded, specifically the cache. Because the cache keeps dom node references, it causes potentially hundreds of detached nodes on a dynamic page.
I'm looking at the cache and it contains dom elements that I no longer have on the page.
If I comment out the cache and do another profile, there are no detached dom nodes from the same set of actions.
This is somewhat related to #103 but that would force the user to manually clear the cache every time the page changes. Can we do it without retaining a reference to the DOM element?
The text was updated successfully, but these errors were encountered: