Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning about using debounce together with onSet #251

Closed
wants to merge 1 commit into from
Closed

Warning about using debounce together with onSet #251

wants to merge 1 commit into from

Conversation

ogonkov
Copy link

@ogonkov ogonkov commented Aug 29, 2014

Someone (like me) could decide that debounce could be applied here.

Someone (like me) could decide that debounce could be applied here.
@akre54
Copy link
Contributor

akre54 commented Aug 29, 2014

How about something more descriptive about the problem and a solution?

Did you find a good solution? Seems to me this should work:

View = Backbone.View.extend({
  bindings: {
    '.foo': {
      onSet: function() { this.doSomething(); }
    }
  },
  initialize: function() {
    this.doSomething = _.debounce(this.doSomething, 500);
  },
  render: function() {
    this.stickit();
  },
  doSomething: function() {
    this.counter++;
  }
});

@ogonkov
Copy link
Author

ogonkov commented Sep 16, 2014

In your example model property would be set to undefined, isn't it?

I've write some suggestions in original thread #248.

@akre54
Copy link
Contributor

akre54 commented Sep 17, 2014

This isn't a full example, it's just to give you a sense of how it should look. Can you put up a small jsfiddle (smaller than the last one please) that shows your issue and we'll try to debug through it?

@spectras
Copy link

This cannot work, because this should not work in the first place, it is a misuse of onSet.
The onSet method must return the actual value that will be applied onto the model. So any form of asynchronicity in onSet is ruled out.

What happens is _.debounce setups a timer, and returns immediately. It cannot return the result of the debounced function, because the function has not executed yet (that's the point of debouncing). Therefore it just returns nothing, so your onSet returns undefined and the model attribute gets set to undefined.
Working as intended.

Now, there are two ways to approach the problem.

  • Have stickit change the semantics of onSet so it mirrors update. That would make it responsible for setting model attributes, and its return value would be ignored. Probably more consistent with the way onGetupdate works, but would be a breaking change for all users of stickit.
  • Use the set callback. It looks it is not documented yet, but from the code I see if it is defined, it overrides the actual setting of the value returned by onSet. Debouncing it will work.

Sample using the set callback:

bindings: {
    '.selector' {
        observe: 'attrib',
        set: 'debouncedSetAttrib'
    }
},
initialize: function () {
    this.debouncedSetAttrib = _.debounce(_.bind(this.setAttrib, this), 500);
},
setAttrib: function (attr, val, options, config) {
    this.model.set(attr, val, options);
}

To sum it up in one point: you debounce functions that do things, not functions that return things.
Hope it helps.

@ogonkov
Copy link
Author

ogonkov commented Jul 31, 2019

I was so stupid

@ogonkov ogonkov closed this Jul 31, 2019
@ogonkov ogonkov deleted the patch-1 branch July 31, 2019 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants