theironcook / Backbone.ModelBinder

Simple, flexible and powerful Model-View binding for Backbone.
1.42k stars 159 forks source link

Problem with undelegate #193

Closed qchroman closed 9 years ago

qchroman commented 9 years ago

Hello,

I think I may have found an issue when was playing with converters. I have added a converter to one of the bindings and noticed that it is called 3 times. I have checked my code and I do call bind() 3 times. Per the source code and the documentation the unbind() should be called internally, so I started digging. The unbind() is called indeed, but the problem is that the following line didn't remove event handlers from the HTML element:

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

Changing it to

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

and changing how event handlers are attached to the element's event in bindViewToModel from delegate to on

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

solved the problem.

chuyler1 commented 9 years ago

I'm having the same problem. If you call bind multiple times, regardless of whether you call unbind afterward, an additional handler is added. When you change something on the page, model.set() ends up getting called numerous times.

I tried your fix and it did not work. The model isn't getting updated at all after your change.

theironcook commented 9 years ago

@chuyler1 I never was able to reproduce this issue. My current project used JQuery 1.7 and the latest uses JQuery 2.1.1.

I can upgrade to @qchroman's suggested fix but if it doesn't work for you anyways there isn't much point. Can you please confirm that you updated the source to the Backbone.ModelBinder correctly and that it does not solve your problem. If you are still having issues there might be something else going on...

chuyler1 commented 9 years ago

I downloaded the master branch of this project and created an example. It appears that the "change" event from Backbone.Model will always get correctly called once. But if I turn validation on for every change you can see that the Backbone.Model.set() function is getting called multiple times, once for every time "ModelBinder.bind()" is executed on an element. Load this example, edit the fields, click Bind or Reset and edit the fields again. Repeat and see how the validate count starts increasing faster than the change count.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xml:lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
    <meta content="text/html;charset=UTF-8" http-equiv="Content-Type"/>
    <title>Example 1</title>

    <!-- include source files here... -->
    <script type="text/javascript" src="../lib/underscore.js"></script>
    <script type="text/javascript" src="../lib/jquery.js"></script>
    <script type="text/javascript" src="../lib/backbone.js"></script>
    <script type="text/javascript" src="../Backbone.ModelBinder.js"></script>

    <script type="text/template" id="view-template">
        <div id="welcome"> Welcome,
            <span name="firstName"></span><span name="lastName"></span>
        </div>
        <div>
            Edit your information:
            <input type="text" name="firstName"/>
            <input type="text" name="lastName"/>
        </div>
        <div><a class="bindButton" href="#">Bind</a></div>
        <div><a class="resetButton" href="#">Reset</a></div>
    </script>

    <script>
        $().ready(function () {

            var UserModel = Backbone.Model.extend({
                validateCount: 0,
                validate: function(attrs, options) {
                    this.validateCount++;
                    console.log('Validate Count: '+this.validateCount);
                }
            });

            ViewClass = Backbone.View.extend({

                _modelBinder: undefined,
                options: {modelSetOptions: {validate: true}},
                changeCount: 0,
                model: null,

                events: {
                    'click .bindButton': 'onClickBindButton',
                    'click .resetButton': 'onClickResetButton'
                },

                initialize:function () {
                    this._modelBinder = new Backbone.ModelBinder();
                    this.model = new UserModel();
                    this.model.set({firstName: 'Bob'});
                    view = this;
                    this.model.bind('change', function () {
                        $('#modelData').html(JSON.stringify(view.model.toJSON()));
                        view.changeCount++;
                        console.log('Change Count: '+view.changeCount);
                    });
                },

                onClickBindButton: function() {
                    console.log('binding again...');
                    this._modelBinder.bind(this.model, this.el, null, this.options);
                },

                onClickResetButton: function() {
                    console.log('resetting...');
                    this.model.set({firstName: 'Bob', lastName:''});
                    this._modelBinder.bind(this.model, this.el, null, this.options);
                },

                close: function(){
                    this._modelBinder.unbind();
                },

                render:function () {

                    this.$el.html($('#view-template').html());

                    this._modelBinder.bind(this.model, this.el, null, this.options);

                    return this;
                }
            });

            view = new ViewClass();

            $('#viewContent').append(view.render().$el);
        });

    </script>

</head>
<body>
<br>
Model data: <div id="modelData"></div>

<hr><br>
<div id="viewContent"></div>

</body>
</html>
platinumazure commented 9 years ago

@qchroman @chuyler1 Can you try with the version of code in my pull request and let me know if this is fixed?