backbone-forms
backbone-forms copied to clipboard
Memory leak in Form.editors.List.setValue()
On the master, Form.editors.List.prototype.setValue() is
setValue: function(value) {
this.value = value;`
this.render();
},
The render() causes the view's $el to be replaced, without detaching the previous elements. I'm currently overwriting this:
Backbone.Form.editors.List.prototype.setValue = function(value) {
this.value = value;
if (!$.isArray(value)) {
throw new Error("List value must be an array");
}
for (var i = 0; i < value.length; i++) {
this.items[i].setValue(value[i]);
}
};
If you agree with the change, would be nice to see that patched.
What if value.length != this.items.length
?
I'm not saying my Overwrite is bug-free, but the current setvalue() implementation definitely causes problems. Probably for for-loop would need to work with addItem / removeItem ?
Probably, yes. I was just concerned because you said that you overwrote it, so I thought you might have a bug in production somewhere.
Sorry about the misleading wording. I want to use the backbone forms JS unchanged, so I'm overwriting some prototype functions in my own code. My suggestion (haven't tested it):
Backbone.Form.editors.List.prototype.setValue = function(value) {
this.value = value;
if (!$.isArray(value)) {
throw new Error("List value must be an array");
}
for (var i = 0; i < value.length; i++) {
if (this.items.length < i) {
this.items[i].setValue(value[i]);
}
else {
this.addItem(value[i], false);
}
}
while (this.items.length > value.length) {
this.removeItem(this.items[this.items.length - 1]);
if (value.length === 0 && this.items.length === 1) {
// beware that removeItem() will re-add a new item
// if 'this.items' is empty
break;
}
}
};
I've been trying to reproduce this at a higher level - possibly misunderstand what you're trying to achieve. Anyway, I've created a JSFiddle which calls setValue on the field - but nothing seems to change in the editors. Is that what you mean by "not detached", or are you talking about something else (perhaps there's 2 bugs here, or I'm testing something that shouldn't normally happen?)
I've taken your JSFiddle and extended it a bit to this one.
I'd expect the list to re-render on the form.setValue() call, for seem reason that's not happening. If the list get's re-rendered, the .change() handler doesn't get called anymore, because this.$el was replaced.
I see what you mean now.
The other problem is that if you console.log form.getValue() after clicking the button, you have 6 items in your array - that's #372 all over. I think I'll try to look at that pull-request to see if we can solve your issue too.