-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add new bindingHandler type (functions) re. #663 / #1602 #1
Conversation
So in ES6 parlance the following ought to work: class Handler {
constructor(params) {
this.controlsBindingDescendants = true
this.computed_value = this.computed(this.computeMethod)
this.computed_read_only = this.computed({read: this.computeMethod})
this.subscribe(computed_value, this.onComputedChange)
}
dispose() {
/*...*/
}
computeMethod() {
/*...*/
}
onComputedChange(newValue) {
/*...*/
}
}
Handler.allowVirtualElements = true
class HandlerChild extends Handler {
computeMethod() {
/* some child method */
}
}
ko.bindingHandlers.handlerExample = Handler
ko.bindingHandlers.childHandlerExample = HandlerChild |
TODO:
|
…anges - replace the longer (and more ambiguously spelt) `allowVirtualElements` and `controlsDescendentBindings` - better names for the binding handler functions (object or constructor) - expose `constructor` on the handler instance prototype - simplify the `value` accessor for the binding params
Seems like a pretty elegant approach to me. Once you settle on the API a little more I'd like to try this out with TypeScript too. The only trick would be to let TypeScript know "trust me, there are two functions on the prototype called computed & subscribe". I've not tried it before, but declaring the existence of a base class and then inheriting the binding handlers from that should do the trick. |
Thanks @IanYates – I've added I'd be interested in how TypeScript handles this. Not knowing TS at all, if things go wonky I'd be most grateful for insight. |
This opens the door to events being cleaned up on `cleanNode`, re. knockout#910 and related.
Having implemented something very similar on my team's site, I figured I'd weigh in. Any function TextBinding(params) {
this.element = params.element;
this.valueAccessor = params.valueAccessor;
this.allBindingsAccessor = params.allBindingsAccessor;
this.bindingContext = params.bindingContext;
this._updateText = ko.computed(function () {
var value = ko.unwrap(this.valueAccessor());
this.element.textContent = value;
}, this);
}
TextBinding.prototype.dispose = function () {
this._updateText.dispose();
};
TextBinding.init = function (element, valueAccessor, allBindingsAccessor, viewModel, bindingContext) {
var binding = new (this)({
element: element,
valueAccessor: valueAccessor,
allBindingsAccessor: allBindingsAccessor,
viewModel: viewModel,
bindingContext: bindingContext
});
ko.utils.domNodeDisposal.addDisposeCallback(element, function () {
binding.dispose();
});
}
ko.bindingHandlers['text'] = TextBinding; Note the use of There's one catch, though, and this is easy to fix: var initResult = handlerInitFn.call(bindingKeyAndHandler.handler, node, /* ... */);
/* ... */
handlerUpdateFn.call(bindingKeyAndHandler.handler, node, /* ... */); I think it's great to provide a base binding handler class that makes it even easier to consume this pattern, but I think we can just provide it as a optional utility and not bake it into the binding behavior. |
@ThomasMichon – great feedback, thank you! |
I was recently reminded of a couple of issues I had with binding handlers that @brianmhunt helped with, in the end I used the approach detailed in knockout#1891. Brian, you mentioned that this might help with my issues, reading through, honestly I don't quite understand it enough to know whether it does or not - but I'd be interested if it doesn't whether it could include this somehow. The issues were essentially
I've got a solution at the moment which sets |
This is baked into tko now, so I'll close this PR just to clean it up. |
re. knockout#663 / knockout#1602 ; depends on knockout#1360.
The new binding handlers have the following properties:
value
(created bydefineProperty
so one does not have to call an accessor)$context
$data
element
allBindings
.dispose
for the instance, if it exists, is called when the node is removed;this.computed
andthis.subscribe
, equivalent to theko.*
equivalent, but their owner is set to the instance and they are cleaned up when the node is removed;allowVirtual
is truthy on either the constructor or prototype chain;controlsContext
is truthy on the instance or the constructor.