Re-binding is not clearing old bindings

[mikebridge]

When I'm re-rendering the same view I'm noticing that it is not unbinding the previous binding. As I understand it from the documentation it should clear the old bindings.

I've reproduced this here by just calling modelbinder three times in a row. You can see when you change the lastname field that it calls the converter four times:

http://jsfiddle.net/mikebridge/qbv6f4at/2/

[theironcook]

Hi @mikebridge You have to manually call the unbind() function. I've updated your fiddle: http://jsfiddle.net/qbv6f4at/3/

Your fiddle looks good - the close() function calls unbind() which is correct.

Normally in my SPA, views are only ever rendered a single time. So, the bind() function is only called once and then unbind() is called when the view closes.

[platinumazure]

Weirdly enough, the current source does show a call to unbind() in each of the bind() methods (in both ModelBinder and CollectionBinder). Wonder why they're not unbinding successfully?

[platinumazure]

I'm stepping through the JSFiddle and it appears to be a bug in jQuery 1.11.0 (unless we're invoking jQuery incorrectly somehow). Lines 4411-4414 (per the JSFiddle's version of jQuery 1.11.0, which is not minified) contain this if statement:

if ( ( mappedTypes || origType === handleObj.origType ) &&
    ( !handler || handler.guid === handleObj.guid ) &&
    ( !tmp || tmp.test( handleObj.namespace ) ) &&
    ( !selector || selector === handleObj.selector || selector === "**" && handleObj.selector ) ) {
    // Remove the handler
}

The first three conditions are true (origType matches, guid matches, no tmp). The fourth condition fails because we do have a selector (jQuery sets this to "**" when we pass in no selector for the change event) but the handleObj.selector (in other words the original selector when binding the event) is "", which is falsy. So the handler is not removed and it leaks.

As far as I can tell, there is no real bug in Backbone.ModelBinder because jQuery transforms "" to "**" for undelegating, but does not do the same for delegating events. We may be able to work around this by specifying "**" ourselves when calling jQuery delegate, but I don't know if that will force us to upgrade the minimum required version of jQuery.

[amakhrov]

Another possible workaround is to handle bindings to the "" selector differently in ModelBinder. Instead of:

$(this._rootEl).delegate(selector, event, this._onElChanged);

Do this:

if (selector) {
    $(this._rootEl).delegate(selector, event, this._onElChanged);
} else {
    $(this._rootEl).on(event, this._onElChanged);
}

and do the same for unbinding.

Not sure it's actually a bug in jquery. The docs doesn't say it explicitly how it should work with an empty selector. More looks like undefined behavior. I don't think we should rely on it anyway.

[amakhrov]

Since we require jquery 1.7 we should use .on/.off instead of .delegate/.undelegate. And this will also solve the issue, as '' is mapped to '**' in undelegate but not in off

[amakhrov]

@platinumazure your snippet is from the .off documentation. This method is working well. The issue is with .undelegate - it maps '' to \ before passing to off

[platinumazure]

@amakhrov Yes I know, hence deleting the comment. Also, we're using .on/.off in master (fixed in my PR #205). @theironcook just needs to cut a release and then we can say it's fixed in 1.0.7/1.1.0.

[theironcook]

Arrggg, sorry but I forgot the call to bind called unbind internally. @amakhrov and @platinumazure have some pretty nice suggestions here.

@platinumazure - I've planned some time next week to finally catch up and finally cut a new release.

[amakhrov]

@platinumazure good point, I didn't realize we had similar bug reports and that it's been fixed already.

[mikebridge]

Thanks! I gave this a quick test by copying-and-pasting the current github version into my initial jsfiddle and it seems to fix that issue:

http://jsfiddle.net/mikebridge/qbv6f4at/4/

[theironcook]

closing this issue then