Can't seem to cleanup detached DOM elements

Question

I'm using jquery-ui Tabs and I'm having a problem that occurs when a tab has been removed. The tab appears to be removed, along with its content div but when you take a look at the heap in Chrome DevTools Profiles (after a tab has been removed) you'll see that the tab li and div elements are still present, but detached. Over time, the repeated addition/removal of tabs causes these elements to accumulate. For example, if you add a tab 10 times, there will be 10 detached div elements and 10 detached li elements showing up in the heap snapshot:

I have the following views:

TabLabel = Marionette.ItemView.extend({
    template: "#tab-label",
    tagName: "li",
    events: {
        "click .ui-icon-close": "closeTab"
    },  
    closeTab: function(e) {
        this.$el.contents().remove();
        this.model.collection.remove(this.model);
        $("#main-container").tabs("refresh");
        this.close();
    }   
}); 

TabContainer = Marionette.ItemView.extend({
    template: "#tab-container",
    tagName: "div",
    onBeforeRender: function() {
        this.$el.attr("id", "div-" + this.id);
    },  
    onClose: function() {
        // This removes the region that contains the container
        App.layout.removeRegion(this.containerRegion);
    }   
});

TabLabels = Marionette.CollectionView.extend({
    tagName: "ul"
});

TabContainers = Marionette.CollectionView.extend({
    tagName: "div"
});

The views are instantiated like so:

tabs = new TabsCollection(); // Create a new collection instance

var tabLabelView = new TabLabels({
    itemView: TabLabel,
    collection: tabs
}); 

var tabContainerView = new TabContainers({
    itemView: TabContainer,
    collection: tabs
}); 

As you can see, the views both refer to the same collection; each model in the collection can be said to represent a single tab (it just so happens that the model contains the necessary information to satisfy jquery-ui tabs). The views are shown in a region via Marionette.Layout... pretty standard. Adding a tab is accomplished by clicking a link in the tab container; all this does is adds another model to the collection and then calls tabs("refresh") on the main container, which makes the new tab appear. Removing a tab is accomplished by clicking an "X" icon in the upper right-hand corner of the tab.

I've spent a lot of time trying to track down this leak and I can't figure out if it's a problem in my code (the way I'm closing views/models/etc. perhaps?) or if it's a problem in the jquery-ui tabs plugin.

Thoughts? Thanks in advance!

Update #1 As requested, here is a jsfiddle demonstrating the problem -- just close the tabs and you'll see that detached elements are left behind.

Also, a screenshot:

Update #2 This appears to be a leak in the jquery-ui tabs widget. The same behavior occurs in the widget demonstration on the jquery-ui website. I added a few tabs and then closed them out and sure enough, they persisted:

I've tested this with the latest (at the time of this writing) version of jQuery UI (1.10.3) and the previous version (1.10.2).

Hazelhazelnut

Can you post a jsfiddle demonstrating this? – Lermontov

I wonder if the issue is that event handler which isn't getting removed – Thickleaf

@JaniHartikainen I thought that could be the problem, but I've tried calling multiple permutations of unbind() in the close function and the results are the same. – Hazelhazelnut

Have you tried forcing the garbage collector between snapshots to ensure you're not actually counting unreferenced items? – Blindheim

One approach that could work for you would be to drop UI-Tabs altogether. Marionette makes designing widgets like this from scratch fairly easy, although you will spend more time in the CSS department. Take a look at this proof-of-concept fiddle: jsfiddle.net/ccamarat/v6bM2 – Blindheim

@ChrisCamaratta From what I've read, garbage collection is run whenever you take a heap snapshot, before the results are outputted. How would I manually force garbage collection anyway? I'm not sure how to go about that. As for your second suggestion, I've thought about that, but I feel like I'm going to run into this type of problem again in the future and I'd really like to get to the bottom of it. Thanks for taking the time to reply -- all of you. – Hazelhazelnut

Did you try using .children() instead of .contents()? – Smuts

a) You can force Garbage Collection on the Timeline tab, which is not very practical if you're profiling events, so you're probably right that GC occurs on capture. b) jQuery UI has a lot going on; it's entirely possible the leak is in the library. Why not try a Marionette-only solution and see if the memory leak disappears? If nothing else it would certainly isolate the problem. – Blindheim

@Smuts Yes, and it left detached text nodes. According to the API documentation, this behavior is by design: Note also that like most jQuery methods, .children() does not return text nodes; to get all children including text and comment nodes, use .contents(). – Hazelhazelnut

Answer 1

So I eventually fixed this problem (quite some time ago) by doing two things:

1. Ditching jQueryUI in favor of Bootstrap

2. Changing the closeTab function (see the original jsFiddle) to the following:

   closeTab: function (e) {
       e.stopPropagation();
       e.preventDefault();
       this.model.collection.remove(this.model);
       this.close();
   }
   

In that snippet, stopPropagation and preventDefault are really what stopped this element from remaining detached (and accumulating on subsequent adds/removes) in the DOM. To summarize: this problem was created by not calling stopPropagation/preventDefault on the event object after it was triggered. I've updated the jsFiddle so that it correctly removes the tab elements and for posterity, here's a screenshot of the result:

As you can see, none of the tab elements are remaining detached after clicking the "X" to close them. The only detached elements are the ones that come from jsFiddle's interface.

Hazelhazelnut

Answer 2

Is there a reason why you use this.$el.contents().remove() instead of this.$el.empty()?

Using this.$el.empty() in that jsFiddle of yours seemed to remedy the detached NodeList.

A few notes memory profiling:

• watch http://www.youtube.com/watch?v=L3ugr9BJqIs
• Use the 3 snapshots method, 2 is not enough. What does that mean?
  • Start fresh, incognito mode and refreshed
  • Take snapshot (forced GC will happen every time you take snapshot)
  • Do something, take snapshot (gc)
  • Do something similar, take snapshot (gc)
  • Compare snapshot 2 with snapshot 1, find deltas with +
  • Choose Summary and Objects allocated between Snapshots 1 and 2 for snapshot 3
  • Look for stuff that you found comparing 2 and 1 that shouldn´t be there. These are the leeks

I have found cases where jQuery seems to leek because they save the current jQuery object in .prevObject when doing some operations like calling .add(). Maybe that call to .contents() do some funky magic.

Sellers

I've tested this, using .empty() instead of .contents().remove() and the results are the same unfortunately. – Hazelhazelnut