Widget disposal about aura HOT 16 CLOSED

Our current approach handles module level unloading (if you take a look behind the scenes at how we use RequireJS 2.0) better, but you're absolutely right in that we don't have as complex an event unbinding approach as other frameworks do just yet. @dustinboston perhaps we should target this for the next release?

from aura.

Yes lets push it to the next milestone.

from aura.

Would be awesome to add event unbinding to the stop module action. It is also worth considering to add check if module is already started for given DOM namespace. At the moment it just reloads module, rerenders it and duplicates events. Could work as reload in that situation, but then event unbinding is must.

from aura.

@gziolo that's a fair point. So, basically, are you saying before we try loading a module into a DOM namespace we check to see if it's already been loaded in that context? I imagine that if someone or something has already loaded a widget into a namespace it was on purpose and perhaps they'd purposefully be trying to overwrite what was previously loaded. @dustinboston what do you think?.

This may be worth exploring if its a) configurable as an option and b) can be done without a significant performance penalty for the check.

from aura.

I've started taking a look at ways to approach the widget descendants event handler unbinding issues this evening.

• Explored writing our own event cache system to facilitate easier unbinding on widget unload
• Reviewed how Marionette currently achieves this https://github.com/derickbailey/backbone.marionette/blob/master/lib/backbone.marionette.js#L22
• Reviewed hacking on the jQuery internal cache to better handle this http://jsfiddle.net/nkMQv/1/ (dislike because I want s to work with Zepto too)
• Explored enforcing internal namespaces on top of custom bound handlers so we can again, more easily remove them later on.

I have to say that having inspected the jQuery and Zepto sources for how they're handling unbinding event handlers for children at the moment, I'm uncertain why a more sophisticated event management system would be necessary here unless you want to expose the events cache for other uses.

@derickbailey we're currently using the RequireJS 2.0 undef() method as a part of our widget clean-up process. To help with disposal of event handlers bound to elements within a widget, I've ensured we're using $.fn.remove() and added an unbindAll method in case this is required. Beyond this (having read https://github.com/derickbailey/backbone.marionette/wiki/When-to-use-the-eventbinder), do you think we need to implement an event cache/management system like Marionette has to tackle this properly?

from aura.

I've ensured we're using $.fn.remove() and added an unbindAll method in case this is required.

@addyosmani I think what we've got is sufficient.

from aura.

Strictly speaking, Marionette's EventBinder is never necessary. But it adds a lot of value through convenience, and makes it a no-brainer to handle event binding clean-up.

If you're looking for an automated way to clean up event handlers when the event is triggered from a Backbone object, then an EventBinder is something you should think about using. Without it, you have to manually unbind all of your event handlers for your objects and components. But the EventBinder doesn't handle DOM events. I've never seen a need to use any add-ons or additional abstractions, other than the Backbone.View remove method, to handle DOM event clean up.

FWIW: I've thought about extracting a Backbone.EventBinder plugin directly, so that projects that aren't using Marionette could take advantage of this object. If you're interested in using the one that I built, I could easily do this. Or you could just copy & paste the code from Marionette. It's all MIT license, so nothing to worry about if that's what you want to do. :)

For anyone that's not familiar with the event binder or why it helps, here are the blog posts and wiki pages related to it:

• http://lostechies.com/derickbailey/2011/09/15/zombies-run-managing-page-transitions-in-backbone-apps/
• http://lostechies.com/derickbailey/2012/03/19/backbone-js-and-javascript-garbage-collection/
• https://github.com/derickbailey/backbone.marionette/wiki/When-to-use-the-EventBinder

Hope that helps. :)

from aura.

@derickbailey Thanks a lot for your input on EventBinder. It was very useful to hear some more about its capabilities and where one would ideally use it :). I think what we'll end up doing is play around with the Marionette EventBinder implementation a little more and see if it offers a better cleanup for our current setup than what we already have.

Remembering what I read in your Marionette View code, it just requires an instance of the EventBinder to be created once in the context where you wish to use it and the rest of the event management should work fine after that, right? :)

from aura.

correct. the wiki page i linked to shows an example of doing that in your own object.

from aura.

the new version of backbone might help:
https://github.com/documentcloud/backbone/pull/1461/files

from aura.

Niiice! Thanks for the heads up. I think we should consider demonstrating
dispose() in our backbone extension to the project.

from aura.

Hi everyone. Just an FYI in case anyone is still interested in using my EventBinder to help clean up events in Aura - I finally got around to extracting that in to it's own plugin for Backbone. You can find it here: https://github.com/marionettejs/backbone.eventbinder

I've also been paying attention to the new Backbone dispose method, and I think that it's going to help move things forward a lot.

from aura.

Thanks for the very helpful update @derickbailey. Thats awesome!. I think we should:

• Review what Derick has in EventBinder and consider whether we should be adding the plugin to Aura
• Definitely update our Backbone extension to use the dispose method.

from aura.

+1 on dispose.

from aura.

@sbellity could you comment on the current status of widget disposal in master?

from aura.

Thanks for the discussion! Closing due to age of ticket and applicability to the refactor. Please feel free to reopen if you would like to address this via a pull request or specific solution with the current version in master.

from aura.