Knockout binding value.update not being called with custom binding and defineProperty

I have a Knockout extension, knockout-secure-binding, and we have encountered an issue.

In particular when using Object.defineProperty, as knockout-es5 does, the value binding's update function is not called when a change event is triggered on an input.

My unit tests illustrate the peculiarity. This works:

it("reads an input `value` binding", function () {
    var input = document.createElement("input"),
        evt = new CustomEvent("change"),
        context = { vobs: ko.observable() };
    input.setAttribute("data-sbind", "value: vobs")
    ko.applyBindings(context, input)
    input.value = '273-9164'
    input.dispatchEvent(evt)
    assert.equal(context.vobs(), '273-9164')
})

This (being how knockout-es5 defines properties) does not work:

it("reads an input `value` binding for a defineProperty", function () {
    // see https://github.com/brianmhunt/knockout-secure-binding/issues/23
    var input = document.createElement("input"),
        evt = new CustomEvent("change"),
        obs = ko.observable(),
        context = { };
    Object.defineProperty(context, 'pobs', {
        configurable: true,
        enumerable: true,
        get: obs,
        set: obs
    });
    input.setAttribute("data-sbind", "value: pobs")
    ko.applyBindings(context, input)
    input.value = '273-9164'
    input.dispatchEvent(evt)
    assert.equal(context.pobs, '273-9164')
})

In the latter case, as mentioned, value.update is not being called when input.dispatchEvent is called.

The custom binding is returning its own valueAccessor, so I expect the problem is related to that. It just strikes me as particularly odd that it would work with an object property but not defineProperty.

Knockout rewrites the binding expessions before processing them in order to support "two-way bindings to include a write function that allow the handler to update the value even if it's not an observable." This part makes the Object.defineProperty defined properties working in the bindings.

This is implemented in the ko.expressionRewriting.preProcessBindings method (source)

This method turns the following binding expression:

data-bind="value: pobs, checked: vobs"

To the following:

"'value':function(){return pobs },'checked':function(){return vobs },'_ko_property_writers':function(){return {'value':function(_z){pobs=_z},'checked':function(_z){vobs=_z}} }"

Note the generated _ko_property_writers which contains the code for setting the non observable proeprties.

And here is the source code comment about this magic property:

// For those developers who rely on _ko_property_writers in their custom bindings, we expose _twoWayBindings as an
// undocumented feature that makes it relatively easy to upgrade to KO 3.0. However, this is still not an official
// public API, and we reserve the right to remove it at any time if we create a real public property writers API.

So you just need to reproduce the same logic in your convert_to_accessors function: you need to create a new property on the result object named "_ko_property_writers" which return the appropriate writer functions:

Parser.prototype.convert_to_accessors = function (result) {
    var propertyWriters = {};
    ko.utils.objectForEach(result, function (name, value) {
      if (value instanceof Identifier || value instanceof Expression) {
        result[name] = function expidAccessor() {
          // expression or identifier accessir
          return value.get_value();
        };
        if (ko.expressionRewriting.twoWayBindings[name]) {
          var token = value.token;
          var context = value.parser.context.$data;
          propertyWriters[name] = function(_z) {
              context[token] = _z;
            };
        }
      } else if (typeof(value) != 'function') {
        result[name] = function constAccessor() {
          return value;
        };
      }
    });
    if (Object.keys(propertyWriters).length > 0)
        result["_ko_property_writers"] = function () {
           return propertyWriters;
        }
    return result;
};

Disclaimer: this is not a production ready implementation! It is just shows the idea what needs to be done. Although it makes both of your sample tests it might break other parts of the plugin. You should also take extra care of the correct context handling because using value.parser.context.$data is kinda hacky.

Recommended topics

Hot tags