Improving the 2nd way in knockout-winjs’s two way data binding

In the original version of the knockout-winjs sample I wrote a couple of days ago (see this post)there is a basic implementation of writing back to an observable when a control changes value. The part that does this can be found in the init method of the generic binding handler and it looks like this:

// Add event handler that will kick off changes to the observable values
// For most controls this is the "change" event
if (eventName) {
    ko.utils.registerEventHandler(element, eventName, function changed(e) {
        // Iterate over the observable properties
        for (var property in value) {
            // Check to see if they exist
            if (value.hasOwnProperty(property)) {
                // Determine if that value is a writableObservable property
                if (ko.isWriteableObservable(value[property])) {
                    // Kickoff updates 
                    value[property](control[property]);
                }
            }
        }
    });
}

You might notice that if a control has a change event (the name is control dependent and not all controls need one but in this case that name is set in the eventName variable) then we register an event handler with the element and update ALL writable observables bound to a property of the control.

This is of course not how we want to update our observables. I would prefer to update just the one that needs updating. So I introduced another field in the definition for each control’s binding handler (I named it the changedProperty). Also I won’t bind to the element’s event but to the control’s event directly. This has one issue however if you also want to be able to bind to this event explicitly.

To accomplish this I changed the above code to the following:

// Add event handler that will kick off changes to the observable values
// For most controls this is the "change" event
if (eventName) {
    // If the change event is already bound we wrap the current handler with our update routine.
    var currentEventHandler = null;
    if (control[eventName]) {
        currentEventHandler = control[eventName];
    }

    control[eventName] = (eventInfo) => {
        if (value.hasOwnProperty(changedProperty)) {
            // Determine if that value is a writableObservable property
            if (ko.isWriteableObservable(value[changedProperty])) {
                // Kickoff updates 
                value[changedProperty](control[changedProperty]);
            }
        }

        if (currentEventHandler) {
            currentEventHandler(eventInfo);
        }
    };
}

I also found out that if we want to bind two events on a single control the current implementation of binding event has a bug. It read as follows:

// After the control is created we can bind the event handlers.
for (var property in value) {
    if (value.hasOwnProperty(property) && (property.toString().substr(0, 2) === "on")) {
        control[property] = (eventInfo) => {
            value[property].bind(viewModel, viewModel, eventInfo)();
        };
    }
}

It turns out however that the ‘property’ variable is changed by the time the actual event is called. So we can’t really use it in the event. I fixed it in the following fashion:

// After the control is created we can bind the event handlers.
for (var property in value) {
    if (value.hasOwnProperty(property) && (property.toString().substr(0, 2) === "on")) {
        control[property] = (eventInfo) => {
            // Must use eventInfo.type here because 'property' will
            // be changed by the time the actual event is fired.
            value["on" + eventInfo.type].bind(viewModel, viewModel, eventInfo)();
        };
    }
}

To watch all this in action download this expanded example: ToDoApp4

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.