Add binding ability

[jsguy]

Hi,

I've added the ability to create custom bindings, this is crucial for extendability, for example with these small additions, you can do 2-way bindings on one line:

https://github.com/jsguy/mithril.bindings

Also, see example code:

https://github.com/jsguy/mithril.bindings/blob/master/example.htm

Note: This does not change the default behaviour of Mithril, it only adds the ability to customise how bindings are handled.

As soon as these changes are accepted, I'll add the bindings project to the wiki - I really think this is a useful addition - let me know what you think!

[lhorie]

Hi @jsguy

I have a few comments (ok, maybe a lot of comments):

• the PR is breaking tests in the Travis CI build
• isFunc is only getting set to true if cachedAttr is a function. Does that mean the binding is not meant to activate on first render? If so, why not?
• on line 240, why do you iterate over m.bindings if you already know that the desired key is attrName? Can't you access m.bindings[attrName] directly without the for/if?
• why does binder take tag as a parameter, if it already takes node?
• node is exposed to the binding handler during the build function, so it cannot be used reliably for reading values from the DOM (e.g. current CSS, offset, etc)
• it's not possible to manage the lifecycle of DOM elements created by a binding (i.e. if my binding creates elements, and the element w/ this binding gets removed, how does the binding know to remove the elements it created?)
• m.bindings should never be undefined (i.e. it should be possible to write m.bindings["myBinding"] = foo without having to do object sniffing/initialization)
• it's difficult to figure out when a binding will break the core API, e.g. how does one know that {oninput: doStuff, value: someProp} behaves as expected, but that m("div", {onchange: doStuff, value: someProp}) ignores onchange?

And some comments about mithril.bindings.js:

• The text binding seems redundant, why not just use m("h1", foo()) or m("h1", m.trust(foo()))?
• Is it possible to have a value binding that binds on change in one place and on input on another? The only way I can see is to hack the names (e.g. valueInput, valueChange)
• It feels like many bindings can be implemented in a much less magical way with helper functions, e.g.

function hides(flag) {
  return {style: {display: flag ? "none" : "block"}}
} 
m("div", hides(ctrl.shouldHide()), "foo")

Or, to make it look closer to mithril.bindings.js:

var bindings = {
  hides: function(flag) {
    if (!this.style) this.style = {}
    this.style.display = flag ? "none" : "block"
  }
}
var b = function(attrs) {
  for (var key in attrs) {
    if (bindings[key]) bindings[key].call(attrs, attrs[key])
  }
}
m("div", b({hides: ctrl.shouldHide(), class: "foo"}), "bar")

Doing stuff at the virtual DOM level also gives you the ability to leverage config which takes care of lifecycle management, presence in document, etc.

I'm kinda inclined to not add the b helper into core because of the issue of overwriting attributes. If it's a app-space function, then the developer can debug their own helpers. If the helper is in core, then it becomes a library problem, which might require a lot of code to handle all possible edge cases.

The PR is very similar to b in spirit, but it creates a lot of subtle and complex interactions. Would using something like b instead of this PR work for you?

[jsguy]

Hi Leo, thanks for answering so quickly and thoroughly - I've answered your queries below:

I have a few comments (ok, maybe a lot of comments):

the PR is breaking tests in the Travis CI build

Sorry about that, build doesn't fail locally for some reason, I'll look into it and submit a new PR.

isFunc is only getting set to true if cachedAttr is a function. Does that mean the binding is not meant to activate on first render? If so, why not?

It is actually being run the first time as the attrName is not in cachedAttrs - in some cases the cached attr may not be a function, so that is why it checks that way.

on line 240, why do you iterate over m.bindings if you already know that the desired key is attrName? Can't you access m.bindings[attrName] directly without the for/if?

Yes, that is better, I'll update the code.

why does binder take tag as a parameter, if it already takes node?
node is exposed to the binding handler during the build function, so it cannot be used reliably for reading values from the DOM (e.g. current CSS, offset, etc)

We may well want to know the tag in your binding, and we already have it available, so we might as well expose it, rather than having to read the DOM.

it's not possible to manage the lifecycle of DOM elements created by a binding (i.e. if my binding creates elements, and the element w/ this binding gets removed, how does the binding know to remove the elements it created?)

Ideally bindings should not create elements - they should just bind events. The exception here is the text binding, which cleans up the element if it has to, see my comment below on why it works the way it does.

m.bindings should never be undefined (i.e. it should be possible to write m.bindings["myBinding"] = foo without having to do object sniffing/initialization)

I think it is better if the user always uses m.addBinding, that way it is easier to extend the binding functionality later on, so it abstracts the underlying functionality away from the user. I'm all for defining m.bindings if it will save a check in the main loop though.

it's difficult to figure out when a binding will break the core API, e.g. how does one know that {oninput: doStuff, value: someProp} behaves as expected, but that m("div", {onchange: doStuff, value: someProp}) ignores onchange?

That is true, and you have the same issue right now - ie: onchange bindings are silently added to elements that don't support it. It would be nice to be able to test for that - in the case of a value binding, at least with custom bindings the end user has the capabilities to make the binding check for that, should they need to do so. Ie: it would be a good thing to give the user more control over the bindings.

And some comments about mithril.bindings.js:

The text binding seems redundant, why not just use m("h1", foo()) or m("h1", m.trust(foo()))?

The text binding ensures there is a single text node - this makes it work correctly in older browsers - I actually borrowed the code from Knockout, as they work with IE7 and up.

Is it possible to have a value binding that binds on change in one place and on input on another? The only way I can see is to hack the names (e.g. valueInput, valueChange)

I'm yet to implement different event bindings, I haven't decided on the best strategy yet, but that would be one way - I'll let you know if I come up with something good.

It feels like many bindings can be implemented in a much less magical way with helper functions, e.g.
function hides(flag) {
return {style: {display: flag ? "none" : "block"}}
}
m("div", hides(ctrl.shouldHide()), "foo")

This will make the display "block" always, if it is visible - that's bad - you might have "inline", "inline-block", etc...
You really need to know if the element is hidden - the method I've added will work with the element - and external libraries - if we were not concerned with that, we could probably get away with a virtual DOM version like so:

function hide(flag) {
    return flag? { style: {display: "none"}}: {};
} 
m("div", hide(ctrl.shouldHide()), "foo")

This is also in the global namespace, and "hide" could easily be overwritten, so I think it is better to add this to a namespace, which is what m.addBinding does...

Or, to make it look closer to mithril.bindings.js:

var bindings = {
hides: function(flag) {
if (!this.style) this.style = {}
this.style.display = flag ? "none" : "block"
}
}
var b = function(attrs) {
for (var key in attrs) {
if (bindings[key]) bindings[key].call(attrs, attrs[key])
}
}
m("div", b({hides: ctrl.shouldHide(), class: "foo"}), "bar")
Doing stuff at the virtual DOM level also gives you the ability to leverage config which takes care of lifecycle management, presence in document, etc.

I see where you are going with this, however you won't be able to do bi-directional binding with this, as you're passing in the value, not the function. Also, I'm rather against using a globally defined "b", and adding further clutter to the view by wrapping it in the "b(" call.

I'm kinda inclined to not add the b helper into core because of the issue of overwriting attributes.

Understood and agreed - polluting the global namespace with short variable names is bad.

If it's a app-space function, then the developer can debug their own helpers. If the helper is in core, then it becomes a library problem, which might require a lot of code to handle all possible edge cases.

Yeah the bindings do not belong in the core - they will be separate - I don't think debugging will be affected - all we need is the ability to write external bindings.

The PR is very similar to b in spirit, but it creates a lot of subtle and complex interactions. Would using something like b instead of this PR work for you?

Probably not. I agree, the spirit is the same but if we're to take this to the next level, there can be no compromise on succinctness. Compared to Angular/Knockout/Ember/etc, Mithril is too "chatty" to be considered elegant - I really like the core, and what you've done with it however I really think that externalising the DOM binding parts would not only be useful for what I'm trying to do, but also a good design philosophy.

[jsguy]

Looks like I branched off a broken build - I'll create a new one from a fixed one - after all, I only want 7 lines added :)

[lhorie]

Ideally bindings should not create elements

I think it's very conceivable that someone would try to use the mechanism for, say, a tooltip, since the API looks generic and it is very similar to the Angular directive API.

You really need to know if the element is hidden - the method I've added will work with the element - and external libraries - if we were not concerned with that, we could probably get away with a virtual DOM version

Doing it at the virtual element level lets you do both because you can tap into config from there if you do need to access the real DOM element. And with config, you get lifecycle management for free, and correct call timing (so you can measure things like offesetHeight correctly etc).

What I'm trying to get at is that I generally favor editing at the virtual dom level than the real DOM level because poking the real DOM can expose you to problems that have already been solved by config.

Re: syntax: I was just polluting the global scope in the snippets above for the sake of making the code simpler and the idea easier to grasp. There are lots of ways that the syntax can be improved, for example:

m.bindings = {
    value: function(prop) {
        if (typeof prop == "function") {
            this.value = prop()
            this.onchange = m.withAttr("value", prop)
        }
        else this.value = prop
    }
}
var mx = function(selector, attrs, children) {
    for (var attrName in attrs) {
        if (m.bindings[attrName]) customAttrs[name].call(attrs, attrs[attrName]) 
    }
    return m(selector, attrs, children)
}

mx("input", {value: someProp, onchange: doMoreThings})

I'm actually putting together a utilities library where a more robust iteration of this specific example could fit well. One could alternatively monkeypatch m to have custom bindings work invisibly. Monkeypatching m.render would also do the same thing. So syntax/succinctness is not really an issue, imho.

I don't think debugging will be affected - all we need is the ability to write external bindings

It does affect debugging. For example, imagine someone types m("input", {value: something, onchange: save}) when they meant m("input", {value: something(), onchange: save}). Given that the issue is that the developer could not spot the typo due to perception blindness, debugging this would typically involve stepping via a debugger, which takes you several stack levels deep into Mithril core code before arriving at the offending code (and console.log is somewhat useless here because the function is called recursively). With mx, you just look up the 4 liner helper, and you can arrive at the source of the problem fairly quickly. This also affects the size and clarity of stack traces when errors get thrown.

Anyways, tl;dr: wrt whether this feature should live in core: I'd prefer to have this outside of core for a few reasons:

• core is designed to be easy to learn by being light on framework-specific features, and I feel this makes core more opinionated and magic
• I'm trying to stay under 5kb gzipped because code size is an important selling point, and I'd rather not put things in core that can be implemented via libraries or helpers
• having this feature live in application-space instead of in core actually provides an important level of control over multi-binding interactions that I cannot provide at a framework level unless I expose a metaprogramming API (I'm writing an article on this aspect)
• virtual dom based approach is more robust
• template syntax can be achieved without changing core

[jsguy]

I don't think people will try to use the bindings for things they shouldn't - after all, there is no exposed interface - just the bindings object, not exactly ideal.
I don't really want to monkeypatch the "m" object - that will be brittle, (if the core changes, the monkeypatch has a good chance of breaking), and so not really worth my time.
I had considered going down the route of creating a completely separate binding module, (mx),
but the core seemed a much more logical place to do this and a better way to actually contribute to the project, but doing as you suggested with "mx" will of course work, but is not ideal - it would make it more verbose by wrapping it in mx-calls everywhere... I guess I can hide the verboseness in sugartags or something.

I'll have a play around and see how it goes.

[jsguy]

Whilst it is not as integrated as I would have liked, it actually turned out pretty good:

https://github.com/jsguy/mithril.bindings

I'm interested to see your article on multi binding - binding is one of, if not the most, important features of these types of libraries - you can make the core as optimal as you like, but if the user binding API is cumbersome, or the templating solution is too verbose, people won't want to use it.