Opened 9 years ago

Closed 3 years ago

#10941 closed enhancement (patchwelcome)

Add NodeList.destroy

Reported by: Eugene Lazutkin Owned by: Kris Zyp
Priority: high Milestone: 1.13
Component: Events Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description

This ticket is created from Les' remark in #10827:

This is something I don't understand about Dojo. If I destroy a widget or remove a DOM element, all event event handlers bound to that widget or DOM element should be unbound. I shouldn't have to worry about it.

See the jQuery doc for the remove() method. They unbound event handlers, but dojo.orphan() is not doing that.

http://api.jquery.com/remove/

Change History (6)

comment:1 Changed 9 years ago by James Burke

Milestone: tbdfuture

comment:2 Changed 9 years ago by Eugene Lazutkin

One possible solution is to add destroy() (as in dojo.destroy) to our NodeList --- NodeList.orphan() does nothing to prevent leaks. In the same vein we lack NodeList.removeAttr() and some other useful helpers.

Obviously we should augment dojo.destroy() code too, if it leaks.

comment:3 Changed 9 years ago by Terence Kent

/bump

I'm just starting out with dojo and ran into this right away. An easy way to illustrate the leak is to create some large number of dom nodes inside of some container (say 1000), and attach some anonymous function as an event handler to each dom node using dojo.connect.

Then, remove all the DOM nodes in the container using dojo.empty. Rinse and repeat.

You won't see a leak with a good browser (e.g. FireFox? 2, Chrome, or Safari) but bring in everybody's favorite IE (v. 6) and it's clearly apparent.

The issue can also easily be solved by tracking all the handles returned from dojo.connect calls and then using dojo.disconnect to remove the event listeners before calling dojo.empty. When the events are disconnected prior to an dojo.empty() call, no leak occurs. Would be great if dojo behaved like jQuery in this instance and managed this clean up for us measly developers.

comment:4 Changed 8 years ago by Chris Mitchell

Owner: anonymous deleted

comment:5 Changed 7 years ago by bill

Component: CoreEvents
Owner: set to Kris Zyp

Not sure if this is still an issue or not, or who it should be assigned to... #10827 is resolved, as is #13065, but it looks like this ticket is really about adding a NodeList.destroy() method.

comment:6 Changed 3 years ago by dylan

Milestone: future1.12
Resolution: patchwelcome
Status: newclosed
Summary: Removing nodes should remove event handlers to prevent memory leaks.Add NodeList.destroy
Type: defectenhancement

Renaming given that the main issue of the original ticket was closed elsewhere. Marking this enhancement as patchwelcome.

Note: See TracTickets for help on using tickets.