backbone-support icon indicating copy to clipboard operation
backbone-support copied to clipboard

reorder remove() and _leaveChildren() in leave method under composite_view.js

Open Kufert opened this issue 8 years ago • 2 comments

I'm having some memory leaks caused by components added to the screen via various jQuery plugins. After few hours of debugging, I found that those plugins have a destroy API I can all from the view before detaching it to prevent the memory leak. I tried to call this from leave() on the view, but that did not work.

It seems like the issue is now with the order of the calls remove() and _leaveChildren() in the code:

  leave: function() {
    this.trigger('leave');
    this.unbind();
    this.stopListening();
    this.remove();
    this._leaveChildren();
    this._removeFromParent();
  },

because remove() removes the view, and only then the view leave() method is called by running _leaveChildren(), the item is not really destroyed as it is no longer on the scree, resulting in memory leak.

My Q - can I change the order? what is the reason behind the current order? If you think it's OK, I'll create PR with that change.

Thanks!

Kufert avatar Mar 02 '17 20:03 Kufert

To confirm I understand the problem, I tried to call this from leave() on the view, but that did not work. means code similar to the following?

var NodeView = CompositeView.extend({
  render: function() {
    this.$el.plugin();
  },

  leave: function() {
    this.$el.plugin("destroy");
    CompositeView.prototype.leave.call(this);
  }
});

var parent = new NodeView();
var child = new NodeView();
parent.appendChild(child);

  // `.plugin("destroy")` in custom `leave()` does not work when called by `child`
parent.leave();

If so, we can change the order. From git history the current order is how the methods were added.

ecbypi avatar Mar 06 '17 18:03 ecbypi

@ecbypi yes that's the case. The issue is that jQuery plugins, such as typeahead, bootstrap-timepicker and many others, are not destroyed properly.

Reason is that when adding anything to the DOM using jQuery, jQuery saves a reference to that item event listeners. That data is stored under $.cache (what's $.cache?). The problem is that even when I try to use destroy on the leave method, the event listeners are not removed as their bound element is not in the DOM anymore, which it turn create a memory leak.

When calling _leaveChildren() before remove(), the leak is prevented.

  leave: function() {
    this.trigger('leave');
    this.unbind();
    this.stopListening();
    this._leaveChildren();
    this.remove();
    this._removeFromParent();
  },

make it all makes sense :)

Kufert avatar Mar 08 '17 15:03 Kufert