gmac / backbone.epoxy

Declarative data binding and computed models for Backbone
http://epoxyjs.org
MIT License
615 stars 89 forks source link

get attribute of another model within computed attribute causes memory leak #103

Closed militeev closed 10 years ago

militeev commented 10 years ago

The code below keeps adding event handler for 'change:attribute1' event to model1 as many times as get attribute is called for computed attribute of model2. The last console.log() statement outputs 1000

    var Model1 = Backbone.Epoxy.Model.extend({
        defaults: {
            attribute1: [ { foo: "bar" }, { bar: "foo" } ]
        }   
    });

    var model1 = new Model1();

    var Model2 = Backbone.Epoxy.Model.extend({
        computeds: {
            attribute2: function() {
                return model1.get('attribute1');
            }
        }
    });

    for (var i = 0; i < 1000; i++) {
        var model2 = new Model2();
        console.log(model2.get('attribute2'));
    }

    console.log(model1._events['change:attribute1'].length);
gmac commented 10 years ago

Yep, that looks about right... through poor software design, model1 is now listening to 1000 dependent models. It doesn't know that you're orphaning your model2 instances, so it obediently keeps on listening to each one. Naturally, you'd never really take this approach in production code (right?), you'd assign model1 instance references to model2, and then call clearComputeds on model2 while deprecating it.

militeev commented 10 years ago

Thanks for the response! I guess my thinking here was that get attribute from another model in computed attribute shouldn't create the dependency behind the scene. I.e. I think the dependency for computed attributes shouldn't go cross-model. If you as the author disagree, I'm fine with that. And thanks for the great plugin, it has been very useful!

gmac commented 9 years ago

RE: I guess my thinking here was that get attribute from another model in computed attribute shouldn't create the dependency behind the scene

Ever use Backbone's on() or listenTo() methods? Same deal... those create a relationship between two objects that must be specifically released using the reciprocal off or stopListening call, lest they become memory leaks. It's the same deal here. Automation can't make informed decisions about when you're done with an object, so you still need to take some responsibility for your own cleanup.