Buggy work with Undercore's debounce

[ogonkov]

I've tired to update model value after user stopped his input, but it seems working very strange.

Here is a little demo http://jsbin.com/rulula/4/

When i wrap onSet callback into _.debounce it's working pretty tricky.

I have add some setInterval for demonstration, because when textarea lost focus, value is updating as expected. But it seems very strange for me, because it should update after 250 ms of debounce.

What i'm doing:

1. Click on textarea;
2. Start some input ('12345' for example);
3. Press 'spacebar';
4. Preview should show '12345';
5. Now select some of your text with keyboard (SHIFT button);
6. Remove selected text.

This action took less than 250ms.

What i expect:
Preview area should show my rest text (ie get it from model).

What happens:
It will keep show previous text. It's looks like something happen with debounced function and it's waiting for one more call. If i blur textarea (click somewhere outside of textarea), it's update model as expected and updates preview.

If i start invoke callback without debounce, everything is OK, so problem somewhere with working stickit & debounce together.

What i'm doing wrong?

[akre54]

I'm not sure I see what the problem is. Can you create a smaller test case without the extra functions?

Debounce ensures that the function is called only after delay milliseconds after the last time it was called. so if I fire off three clicks in a row to a debounced handler with a wait of 250ms, the function will get called 250ms after the last click.

[ogonkov]

Here is light version @akre54 http://jsbin.com/rulula/5/

[akre54]

I'm still not seeing what the issue is. It appears to be working correctly for me.

Can you make one without the setInterval?

[ogonkov]

On your gif you input some new text ('fdaf'), but model is not updating with new value (it still 'asdf'). It's updated only after next input (space), and value is still old 'asdffdaf'. It's feel wrong.

I have add some console output here http://jsbin.com/rulula/6/

Set new model value of "text" to 1
Get current model value of text: undefined

I was expected that it should work like this clear JS version http://jsbin.com/yomas/2/
You just put value, and it would be set for object after 250ms.

setInterval is required to keep focus on textarea. You can see on clear js version that it's works correctly with setInterval. For some reason model didn't trigger any events in my demo, i didn't understand why, i think it should?

[akre54]

Is it an event thing? The matching handler is the one for textarea, which listens for ['propertychange', 'input', 'change'] events which IIRC are not fired immediately on value change (see the MDN article on change). You could add keyup as an event if you need more immediate propagation.

here shows how the textbox losing focus (blur) updates the prop.

[ogonkov]

But how it's work with just only input event on clear js version?

[akre54]

sorry could you rephrase? I'm not sure I understood that

[ogonkov]

Look above, here is a version that uses same handlers, and listen to input version with debounce of callback http://jsbin.com/yomas/2/

I assume that my code for stickit should work in the same way

[ogonkov]

I have found bug in my previous examples, here is a very minimal version without setInterval, please, take a look, it's logs models changes to console.

http://jsbin.com/rulula/7/

[akre54]

What's the issue here?

Also why are you using addBinding? Why not use the bindings hash?

[ogonkov]

Issue is that after last onInput event triggered by textarea, models attribute didn't update.

How to debounce callback from bindings hash? I've got undefined error, when tired to do that.

[akre54]

Right, because the this in your _.debounce(this.update, ...) refers to the global context, not the this context of the view.

You could try

Textarea = Backbone.View.extend({    
  bindings: {
    '.ad-template-value': {
      onSet  : _.debounce(function() {
        this.update();
      }, TYPING_SPEED),
    }
  }
});

Or you could bind your update method in initialize:

Textarea = Backbone.View.extend({    
  initialize: function() {
    this.update = _.debounce(this.update, TYPING_SPEED);
  },
});

[akre54]

(oops, didn't mean to close). Does this make sense?

[ogonkov]

It doesn't solve problem. When i type any text it's still didn't save actual value of input to model.

http://jsbin.com/rulula/9/

[akre54]

Ahhhh ok now we're onto something. It's because onSet needs to return a value to Stickit. If you've debounced the result you'll need to handle the return value yourself (since Underscore doesn't pass back the result to you). I'm not sure how to solve this at the moment, but I'll have a think on it. Anything come to mind to you that we could look into?

[ogonkov]

Actually underscore return result of debounce function
https://github.com/jashkenas/underscore/blob/master/underscore.js#L786

[ogonkov]

It's looks like it's Underscore's implementation of debounce fault here.

Underscore debounce result returned before timeout in fact. Looks like It's not intended to return right value, just to debounce function execution.

We should add it to README, i think, to prevent others from errors.

My suggestion

Well, in fact i didn't think that setting attribute value from View is great idea anyway. It's would be great if we could have ability to trigger some method, but if it's return undefined -- didn't set model with that result.

In my model i have listener needUpdate, that triggered by View with new data.

var BasicModel;

BasicModel = Backbone.Model.extend({
  initialize: function() {
    this.on('needUpdate', this.updateAttribute, this);
    // Other listeners
  },

  /**
   * @param {Object} options       Something to help me get decision
   * @param {String} options.param Attribute for update
   * @param {String} options.value New attribute value
   * @returns Boolean
   */
  shouldBeUpdated: function(options) {
    var decision;
    // I will decide what model should do
    return decision;
  },

  /**
   * @param {Object} options
   * @param {String} options.param Attribute for update
   * @param {String} options.value New attribute value
   */
  updateAttribute: function(options) {
    // Logic to make sure that attributes should be updated
   if (!this.shouldBeUpdated(options)) {
     return;
   }

   this.set(options.param, options.value);
  }
});

Later in some View:

var SomeView;

SomeView = Backbone.View.extend({
  /**
   * @param {Object} options
   */
  updateMethod: function(options) {
    this.model.trigger('needUpdate', {
      param: options.param,
      value: options.value
    });
  }
});

Model decide itself what data should be set. I think it's how it should work with MV* architecture. Set anything from View is not right. It should just notify Model about changes, and it's a Model responsibility to make a decision. Like this:

var FieldView;

FieldView = SomeView.extend({
  bindings: {
    '.selector': {
      observe: 'someParam',
      onSet  : _.debounce(function(value, options) {
        this.updateMethod({ // Update method would be triggered only once
          param: options.observe
          value: value
        });
      }, 250) // Without return value both methods didn't set attribute of model twice
    }
  }
});

In current implementation of onSet callback it's not impossible, i think. Stickit always set value that is returned by callback.

Adding option for debounce

I don't think it's great idea. It's makes stickit too much smart and will make logic more complicated.

[spectras]

I posted a long comment on #251, about the wrong assumption and a possible solution.

I'm just commenting about this:

Well, in fact i didn't think that setting attribute value from View is great idea anyway. It's would be great if we could have ability to trigger some method

If you are doing true MV* pattern, you are right. And you should not use 2-way data binding then, that pattern is essentially incompatible with the MV* pattern. You can still use stickit, but only use it for model→view data binding, not the other way around. I do this in my own projects.

As for triggering some method, well, that's already part of Backbone itself.

events: {
    '.control blur': '_onControlBlur'
}

Model decide itself what data should be set. I think it's how it should work with MV* architecture. Set anything from View is not right. It should just notify Model about changes, and it's a Model responsibility to make a decision.

I have to disagree on that one. In no MV* pattern should the model decide anything. The model carries knowledge. Knowledge of data and knowledge of how to handle. But it does not use any of that by itself.

What should happen is you set up a third object that does the deciding. There are several ways to do this, a common one being introducing an controller (thus the MVC pattern). The view converts DOM events into user intentions. The controller listens for intentions and acts upon them, ordering the model to update some data. The model notifies the view of changes so it can update itself.

[euge]

Thanks for all the ideas here, I came up with a slightly differently solution involving using silent:true

{
  bindings: {
    ":text" : {
      observe: "attrName",
      setOptions: {
        silent: true
      },
      onSet: "_setSomething"
    }
  },

  initialize: function() {
    var model = this.model;
    this._lazyChange = _.debounce(function() {
      model.trigger("change");
    }, 300);
  },

  _setSomething: function(val, options) {
    this._lazyChange();
    return val;
  }
}

With this approach, the new attributes are set in realtime, but the change notification happens 300ms when the user input stops.

[spectras]

That should work, but be a bit error-prone if you have other devs on the project (or yourself, in a few years). Understanding why update events fire 300ms late if you don't know this trick will let people scratching their head for a while. It may even cause race conditions if something else updates the attribute in between _setSomething and the actual event.
Your call. By the way, to mimic all events, you probably want to trigger a change:attrname event as well ^^

[akre54]

Agreed with @spectras. This is definitely hackier than it should be. If there's something Stickit can do to improve I'm all ears.

(Could we maybe allow you to set val in the config object so we wouldn't have to care about the return value? or some other way of avoiding the syncronicity of onSet?)

[spectras]

I guess you could document config.set and make it an official part of the API? It does the job, but last time I looked it was not documented. From this function it looks it is intended for overriding the actual setting of a value into the model.