Knockout custom binding

I've written a custom binding for KnockoutJS which renders ` elements with children.

When run, this binding adds and DOM elements.

In order to make the binding two-way I obviously need to 'record' the observables for each of those newly-added DOM elements. I've done this using

ko.utils.domData.set(option, "data", thisOption);

and then in order to retrieve the observable (or plain object) I call

ko.utils.domData.get(node, "data");

This works fine. However, in Knockout's native bindings I can call ko.dataFor(element) and retrieve the observable, yet this doesn't work in my custom binding.

Can anyone tell me if I've gone about this the right/wrong way? And if wrong, how do I 'record' data that ko.dataFor can retrieve?

Furthermore, my custom binding accepts a parameter which is the property that the selected element's observable is assigned to (in exactly the same way that Knockout's native options binding uses the 'value' parameter). So in that same change-handler I retrieve this property using the same methodology as above - i.e. using ko.utils.domData...

Is this the correct approach for assigning a selected value to another observable in a change handler?

``
ko.bindingHandlers.groupedOptions = {
"init": function (element, valueAccessor, allBindings) {
ko.utils.registerEventHandler(element, "change", function () {
var value = valueAccessor(),
property = ko.utils.domData.get(element, "property");
ko.utils.arrayForEach(element.getElementsByTagName("option"), function(node) {
if (node.selected) {
var data = ko.utils.domData.get(node, "data");
if (typeof(property) === "function") {
property(data);
} else if (typeof(property) === "string") {
var vm = ko.dataFor(element);
if (vm !== null) {
vm[property] = data;
}
}

From a once over:

-
You use the following type of coding a ton:

var groupsLabel = "Label";
...
if (typeof (groups["label"]) === "string" && groups["label"].length) {
  groupsLabel = groups["label"];
}

You could consider using a helper function for that

function getStringValue( s , defaultValue )
{
  return (typeof s === "string" && s.length) ? s : defaultValue;
}

Then you can

var groupsLabel,
     optionsCollProp,     
     optionsTextProp,     
     optionsValProp,    
     optionsValue,                                             

 groupsLabel = getStringValue( groups.label, "Label" );

 if (typeof (groups["options"]) === "object") {
   var config = groups["options"];
   optionsCollProp = getStringValue( config.coll , "Options" );
   optionsTextProp = getStringValue( config.text , "Text" );
   optionsValProp  = getStringValue( config.value, "Value" );        
 }

Note that I also changed options["something"] to options.something which is the preferred style in JavaScript.

• You use var options twice, the meaning of options is different in the 2 usages, I would use config instead the first time as in my counter proposal above this point.

• You do not use var value = valueAccessor() as far as I can tell

• You do not use optionsValue = null; as far as I can tell

• Some variables are perhaps too Spartan vm -> viewModel ?

• .nodeType != 3

As for your actual question, I see nothing wrong with your approach.

StackExchange Code Review Q#41281, answer score: 3