define event handlers in Ractive.extend()

[Rich-Harris]

See #357.

Basically, many components defined with Ractive.extend() have this sort of thing:

MyView = Ractive.extend({
  init: function () {
    someMyViewSpecificInitCode();

    this.on('foo', function (event) {
      // some code happens
    }
  }
});

This is fine except that in practice it can end up with duplicated code and other evils. And if you subclass MyView, and that subclass has an init() method, the foo handler won't be bound unless you call this._super(), except that maybe that's not what you want because MyView.prototype.init actually contains a whole load of other MyView specific init code that the subclass doesn't need, and frankly it can all get a bit messy.

If instead we had...

MyView = Ractive.extend({
  init: function () {
    someMyViewSpecificInitCode();
  },
  on: {
    foo: function (event) {
      // some code happens
    }
  }
});

then we'd have neater control of things. In fact, Ractive.extend() is a red herring - this may as well be an initialisation option as well as an extend option.

Unresolved questions

1.

In the following example...

One = Ractive.extend({
  on: { foo: doFoo }
});

Two = One.extend({
  on: { bar: doBar }
});

...should instances of Two have both foo and bar handlers bound? Intuitively it seems they ought to, especially since transitions and partials and so on accumulate in this manner.

2.

Should event handlers overwrite handlers of the same event further up the chain, or should they too accumulate? I.e....

One = Ractive.extend({
  on: { foo: doFooOneWay }
});

Two = One.extend({
  on: { foo: doFooAnotherWay }
});

...should Two instances call both doFooOneWay and doFooAnotherWay, or just doFooAnotherWay? The second option would, I suspect, be less surprising, and it would make it possible to 'null out' event handlers that a subclass didn't need. But at the same time it's contrary to the way event handlers ordinarily work (i.e. you can add as many as you like to the same event). I lean towards the first but would be glad to hear opinions.

[martypdx]

In regards to the discussion from #357, I like the idea of having methods directly invoked by name. Maybe this could happen when magic: true.

[binarykitchen]

Good ideas. Yes, it is quite magical but I'd welcome these features too.

For the first unresolved question I think class Two should overwrite class One's event handler. And in the second question, if I want to call both methods, then I recommend to use this._super() like:

One = Ractive.extend({
    on: {
        foo: doFooOneWay
    }
});

Two = One.extend({
    on: {
        foo: function() {
            this._super(); // calls foo() of parent class
            this.doFooAnotherWay();
        }
    }
});

And your other question regarding firing events without method names:

this.on('showOptionsMenu', this.showOptionsMenu);

vs

this.on('showOptionsMenu'); // also calls this.showOptionsMenu();

is a nice idea. Why not? Just make sure this is well-documented.

[Rich-Harris]

Thanks @martypdx and @binarykitchen for the input. The _super thing makes sense.

Despite having been the one who initially suggested it, I found that something felt not-quite-right about automatically calling methods that have the same names as events, beyond the potential for surprise, and I eventually pinned it down: when you create a method, you're defining an interface for the external world, and so that method should have a signature that is designed accordingly. But event handlers don't have that signature - the first argument is always an internally-generated event object, which no third party code would have.

So if we had something like this...

<div class='gallery'>
  {{#images:id}}
    <img src='{{thumbnail}}' on-click='show:{{id}}'>
  {{/images}}
</div>

Gallery = Ractive.extend({
  show: function (event, id) {
    // some code happens
  }
});

..and we wanted to programmatically show a particular image (e.g. as soon as the page has loaded), then we'd need to write some contorted code:

gallery.show(null, 'trollface');

// meanwhile, the show() method can't use the `event` object

So gallery.show() would be a poor interface.

Arguably, this is just persnickitiness (I realise that many event handlers, e.g. the showOptionsMenu one, wouldn't be affected in the same way). But it feels like a big red flag - like it's hinting at some deeper truth about interface design, that we should keep show() the method and show the event at one degree of separation from each other. This...

Gallery = Ractive.extend({
  show: function (id) {
    // some code happens
  },
  on: {
    show: function (event, id) {
      this.show(id);
    }
  }
});

...is clearly longer, but it feels more hygienic and less brittle. Or maybe that's just me?

[binarykitchen]

Hmmm, good thoughts. No, this is not a persnicketiness, but a red flag indeed. I never thought of this. The event object must be passed whenever an event happened.

After a second thought I agree with @codler that it's too magical. I think I wouldn't automatically call methods that have the same names as events. Too risky. And that discussion should be in a new ticket (again)!

Let's stick to the main topic here, inheriting event handlers from the parent :)

[codler]

An idea is to introduce "eventHook" that would intercept all events, I imagine it could look something like this

x.extend({
  /* Mixing parameters with ractive.fire() and ractive.on() */
  eventHook: function(eventObj, eventName, args..) {
    /* One scenario */
    if (/* check if eventName exist in current context and it is a function */) {
      this[eventName].apply(this, [eventObj].concat(args))
    }
    /* Other scenario - Just manipulating eventObj and let it pass to "on function" */
    eventObj.count = this.triggerCount++;
    return true;

    /* Other scenario - replace event */
    if (eventName == 'something') {
      this.fire.apply(this, ['otherEvent'].concat(args));
    }
  }
})

---

A common use case for me with the properties are to pass the metods to the template.

x.extend({
 template: '{{somefunction("hello")}}',
 init: function() {
   this.set('somefunction', this.somefunction);

   /* reuse same function inside */
   var a = this.somefunction("yo");
 },
  somefunction: function(a) {}
})

I could have set directly to data then the function could have been used in template, but it would be more ugly when calling inside the Componant, It would have looked like this.data.somefunction, which looks strange to me.

---

For the question 1 I would say yes, it should inherit events.
For the question 2 I would choose _.super() also. I first though of stopPropagation :P, but _.super() is better

[atmin]

For question 2, I'd choose:

• call doFooAnotherWay
• if return value !== false then call doFooOneWay

So it's like how event handlers work and you can break the event chain, too.

[copongcopong]

For question 2,

since it is using .extend, I'do go with overwriting the initial event method. Having _.super() is nice, but adds complexity (what if i'm on the nth extend?).

If it didn't use .extend, chaining the calls ala DOM events, (with preventPropagation, etc) would be a good pattern.

[quangv]

For question 1. I think it should have both events.

For question 2. At first I thought it should be overwritten (and we can call _super like @binarykitchen first suggest, but then I read @atmin comment. If doFooAnotherWay doesn't return false, doFooOneWay fires, could be powerful indeed.

[Rich-Harris]

For those interested, I wrote down some thoughts on this issue over on #405. The tl;dr is that I'm no longer convinced this is a desirable change - it introduces too much ambiguity and the implementation gets rather complex.

[binarykitchen]

If we follow the basic OOP principles, we'll be fine I think. nth extends are possible in most programming languages and frameworks. So why not here too? It is up to the programmer if (s)he adds complexity or not.

[martypdx]

@binarykitchen Options are a bit different then just normal extensions. For registries, it's geared toward combinatorial extension by key. That is, combining supplied keys for a given registry with those higher in the extension stack given precedence. For example:

var Widget1 = Ractive.extend({
    partials: {
        key1: 'abc', 
        key2: 'xyz'
    }
}) 

var Widget2 = Widget1.extend({
    partials: function(partials, data) {
        this._super(partials)
        if (data.whatever) {
            partials.key2 = '123';
        }
        partials.key3 = '456';
    }
}) 

var r = new Widget2({
    partials: {
        key3: 'foo', 
        key4: 'bar'
    },
    data: { whatever: true }
})
// r.partials are key1='abc', key2='123', key3='foo', key4='bar'

The end result is a hashmap of partials, with ultimately only one value for a given key for any ractive instance.

For handlers, "one per key" is probably not what is intended:

var Widget1 = Ractive.extend({
    on: {
        thing1: fn1, 
        thing2: fn2
    }
})  

var r = new Widget2({
    on: {
        thing2: fn3, 
        thing3: fn4
    }
})
// for thing2 does r.on call:
//       1) fn2 and fn3?
//       2) only fn3 and maybe fn2 if fn3 calls this._super?
//       3) something else?

[binarykitchen]

Good examples. I agree with the first one, hashmap of partials. Nicely presented.

Regarding the second example, thing2 after the r.on-call should probably be fn3 and when called with this._super fn2 (here we can say we follow OOP).

[MartinKolarik]

@binarykitchen that's exactly one of those problematic things. Since registered event handlers are stored as a list rather than a hashmap (because you can have more handlers for the same event), we are basically merging two arrays. In JS that means something like result = array1.concat(array2). Which gives us both fn2 and fn3.

[binarykitchen]

Oh I see. Getting to know better the internals of RactiveJS ... okay, let me think about it.

[binarykitchen]

Probably it's a design flaw and we have to introduce breaking changes by following basic OOP rules. Meaning if you want to keep multiple handlers for the same event and extend the class, you must call this._super() and add your logic like the above Widget1.extend(). If not, then it's overwritten.

[binarykitchen]

I wonder why @Rich-Harris designed it that way? For simplicity's sake?

[MartinKolarik]

Well, pretty much all the pub/sub systems I know are designed that way. You really want to be able to attach multiple listeners for the same event (in the same Ractive instance). The problem comes with the inheritance as pub/sub systems usually don't allow anything like that. From my POV, if this was about to be implemented, than keeping both fn2 and fn3 is the right thing to do. But than again, I think it's better not to implement this at all.

[rstacruz]

I realize this issue is closed, but I just came to a new idea here that's worth mentioning. The original proposed syntax is below:

Ractive.extend({
  observe: {
    "name": function () { ... }
    "email": function () { ... }
  },
  on: {
    "refresh": function () { ... }
  }
})

It feels natural, but indeed we run into the edge case of not making it clear how to handle multiple handlers per event.

For comparison, Backbone.js has an events object which defines which functions respond to which events.

Backbone.View.extend({
  events: {
    "click .refresh": "updateNewsfeed"
  },
  updateNewsfeed: function () { ... }
});

Ember allows you to make methods that you tag with function () { ... }.on('event').

Ember.View.extend({
  updateNewsfeed: function () {
  }.on('refresh')
});

The common pattern here is that they make you define methods, and you just define which methods are associated per event/property. Taking inspiration from this, we can make a better proposal on how to do this in Ractive:

Dashboard = Ractive.extend({
  events: {
    "refresh": "updateNewsfeed",
    "dismiss": ["showExitNotification", "closeNewsfeed"]
  },
  updateNewsfeed: function () { ... },
  showExitNotification: function () { ... },
  closeNewsfeed: function () { ... }
})

This has the bonus benefit of making it clear how to handle subclassing: if you define more events on a subclass of Dashboard, it will just add those handlers to the registry. Furthermore, if you'd like to override event behavior of a parent class, you can just reimplement the method call it uses. (I believe _super() support will land in the upcoming version soon, so there's that nicety as well.)

[martypdx]

@rstacruz 👍 I proposed that very thing here and I still think it's a decent idea and good to know there's precedent in other libraries.

I do think that we are missing the 90% sweet spot of just accepting functions. So I think:

new Ractive({
    on: {
        refresh: function(){ ... }
    }
})

should do exactly what you expect it would do. And string methods, and arrays, are just a more advanced case:

new Ractive({
    on: {
        refresh: ['refresh', function(){ ... }]
    },
    refresh: function(){ ... }
})

IMO, I would stick with on and observe instead of events, but whatever

[rstacruz]

👍 on all those points. Backbone can also accept functions in those
registries. And perhaps even make .on({}) accept that format too for POLS.

On Friday, September 26, 2014, Marty Nelson [email protected]
wrote:

@rstacruz https://github.com/rstacruz [image: 👍] I proposed that
very thing here
#927 (comment)
and I still think it's a decent idea and good to know there's precedent in
other libraries.

I do think that we are missing the 90% sweet spot of just accepting
functions. So I think:

new Ractive({
on: {
refresh: function(){ ... }
}})

should do exactly what you expect it would do. And string methods, and
arrays, are just a more advanced case:

new Ractive({
on: {
refresh: ['refresh', function(){ ... }]
},
refresh: function(){ ... }})

IMO, I would stick with on and observe instead of events, but whatever

—
Reply to this email directly or view it on GitHub
#360 (comment).

[dagnelies]

Despite it's closed, I'd like to vote +1 for this feature.

I'd also be very happy with the "light version" of it, I.e. without the super and all that inheritance fluff.

I think the "light version" where it simply adds declarations is the most practical and covers the vast majority of use cases. The inheritance within event handlers, to me, looks like YAGNI.

For the initial questions:

1. Yes, simply make them both available
2. Let's just override/redeclare it.

My 2 cents. ;) (...and I'd be happy about it)

[martypdx]

I agree, will try and do a PR this release (or someone else can take a stab 😸 )

[guilhermeaiolfi]

+1 from me. Using methods hooks to define events are not a very nice API.

Another idea would be to use the extended class to define its events using a "on" class method. It would be consistent with how you do it in instances:

var MyView = Ractive.extend({});
MyView.on("eventName", handler);

The only problem is that classes would be mutable after declaration. So the on hash is better IMO.