Search code examples
knockout.jsknockout-3.0

Knockout observable not triggering when using throttle


I have the following:

self.periodicallySave = ko.computed(function () {
    if (self.optionA() || self.optionB() || self.optionC()) {
        self.saveOptions();
    }
}).extend({ throttle: 1000 });

I'm using this computed to periodically save text fields as they update. Those text fields are textareas and look like this:

    <textarea rows="2" data-bind="textInput: optionA"></textarea>

With the exception of optionA which is a select.

            <select data-bind="value: optionA">
                <option>Foo</option>
                <option>Bar</option>
            </select>

For some reason, only OptionA triggers my computed observable; changing the others has no effect. Am I misunderstanding how computed observables work? My expectation is that if any of those observables change, the function will be triggered.

Update:

If I use a + instead of a ||, it works as expected. Odd that OR seems to stop the observable chain; especially since optionA is a select that can only ever contain one of two values. Removing optionA also yields expected results.


Solution

  • I think this has to do with the way knockout sets up the change tracking dependencies for the computed function. Your 3 option observables are all within an IF statement and therefore if the first option starts as true, the second two conditions get short circuited and are not evaluated. Since they aren't evaluated knockout never binds to them.

    A computed observable used this way is a little bit dangerous in my opinion because it often has unexpected side effects. You can easily "fix" this behavior by unwrapping the observables into variables first before checking their values:

    self.periodicallySave = ko.computed(function () {
        var a = self.optionA();
        var b = self.optionB();
        var c = self.optionC();
    
        if (self.optionA() || self.optionB() || self.optionC()) {
            self.saveOptions();
        }
      }).extend({ throttle: 1000 });
    

    but I think it might be a little cleaner (although less concise) to set up subscriptions to each of the observables instead.