Method calls

[Rich-Harris]

This pull request is in the same spirit as #1141 - it's a work-in-progress (e.g. no tests/error handling!), put here in hopes of stimulating discussion and catching any howlers before it gets merged in. Like {{yield}}, this is a feature that I'd sort of envisaged as part of the next major release (0.6.0 - not semver major, but whatever), but there's no harm in getting it out there sooner so we can sweep up any bugs.

For detailed background, see #568 (from this comment onwards). The idea is to replace (eventually - this PR doesn't break any existing code) the current event syntax. Instead of...

<button on-click='select:{{object}}'>select object</button>

ractive.on( 'select', function ( event, object ) {
  this.set( 'selected', object );
});

...we can do this instead:

<button on-click='set("selected",object)'>select object</button>

It could be any method belonging to the current instance, even teardown():

<button on-click='teardown()'>self-destruct!</button>

If you need to access the event object, you can:

<svg class='googly-eyes' on-mousemove='lookAt(event.original.clientX, event.original.clientY)'>
  <!-- google eyes template goes here -->
</svg>

I didn't fully realise how powerful this pattern is at first. Here's an 80% complete TodoMVC implementation (we're using the key event plugin for on-enter):

<input on-enter='push("todos", { description: newItem, done: false })' value='{{newItem}}'>

{{#each todos :i}}
  <div>
    <button on-click='splice("todos", i, 1)'>x</button>
    <p class='{{ done ? "done" : "pending" }}'>{{description}}</p>
  </div>
{{/each}}

You can still fire 'proxy events' to achieve the same effect as the old syntax...

<button on-click='fire("select",event,object)'>select object</button>

...but the need to fire events like this will be much less frequent. (In fact, I did wonder about rewriting the existing event handler code so that select:{{object}} is automatically treated as fire("select",event,object) at parse time, to make the runtime code a bit leaner. But there are a couple of edge cases where rewriting gets tricky. Never mind, if we decide to deprecate the old syntax, we'll be able to delete that extra code soon enough.)

So...

As I say this is a WIP but I hope to merge it in soon. If anyone has any strong objections, speak now or forever hold your peace! The only major outstanding question is whether the old syntax should be deprecated over the next few versions - my vote is yes (it's less intuitive, more verbose in most cases, has more edge cases, and is a maintenance burden).

If you want to try it out in the meantime, here are some steps (should probably come up with a way to host feature builds online...):

# clone this repo, switch to the method-calls branch
git clone -b method-calls https://github.com/ractivejs/ractive

# install dev dependencies
npm install

# install gobble-cli (build tool)
npm install -g gobble-cli

# run the build
gobble
open http://localhost:4567/sandbox

Then, copy the sample folder in sandbox, and poke around with the index.html file therein.

[martypdx]

Adding those array modification methods to ractive sured paid off. 😄

[martypdx]

How sweet is this:

<input type='checkbox' checked="{{all}}" on-change="set('todos.*.done', all)">

UPDATE: Oh, don't even need the e.context!

[evs-chris]

I like it! My only concern would be discoverability of the event object. Is it always just event even if there happens to be a key in the current context with the same name?

[martypdx]

Should event be the original DOM event, not the current ractive event object? There's already access to context and index directly. Avoids having to type event.original Keypath might be useful, could be added it to the DOM event?

[Rich-Harris]

Adding those array modification methods to ractive sured paid off

Yep!

My only concern would be discoverability of the event object

That's a fair point. I think (can't check right now) that you can work around it by using this.event to get a data property with that name.

Should event be the original DOM event, not the current ractive event object?

I did wonder about that... it would mean that there was no way to access custom event properties (e.g. a pan event with dx and dy values). Not sure.

[martypdx]

Should event be the original DOM event, not the current ractive event object?
I did wonder about that... it would mean that there was no way to access custom event properties (e.g. a pan event with dx and dy values). Not sure.

Nah, it's a bad idea. I'll come up with some better less worse bad ideas. Like:

<input on-enter='push("todos", { description: newItem, done: false }).then(set(newItem, ""))' value='{{newItem}}'>
// or maybe just
<input on-enter='push("todos", { description: newItem, done: false }); set(newItem, "")' value='{{newItem}}'>

Couple of issues thus far:

on-mousemove='' // empty handlers fall over
on-mouseup="{{#condition}}set('foo', bar){{/}}" //blocks don't work

Also, IMO we should keep the existing syntax as well. It's a different set of use cases around publishing component events.

[MartinKolarik]

I think the old syntax should be deprecated. It'd be confusing for new users to have 2 ways of doing almost the same thing and it's still possible to use fire for that.

[martypdx]

@MartinKolarik for the case when you do want to pub an event, IMO it sucks to go from: on-click='foo' to on-click='fire("foo", event)'. Method calls are great, so is the more direct abstraction of pub/sub.

[Rich-Harris]

What about a third option - retain on-click='foo', but deprecate on-click='foo:{{x}},{{y}},{{z}}? The ability to fire events with arguments has proven to be problematic for inter-component communication, because of this:

<widget on-foo='bar:"these arguments will be ignored"'/>

<!-- widget template -->
<button on-click='foo:1,2,3'>click me</button>

Maybe allowing on-click='foo' as a shortcut for on-click='fire("foo",event)' while disallowing arguments would be a step in the right direction? Though of course this is all related to the work on component event bubbling. I'll admit I haven't fully thought it through, just throwing it out there.

For the record my inclination is the same as @MartinKolarik's :-)

[martypdx]

That makes sense as you could then allow params on the component defined by the parent:

{{#items:i}}
// these actually make sense!
<widget on-foo='bar:"valid args", {{i}}'/>
{{/}}
<!-- widget template -->
// yes :)
<button on-click='foo:'>click me</button> 

// no :(
<button on-click='foo:"parse error?"'>click me</button> 

on-click='foo' as a shortcut for on-click='fire("foo",event)'

Yep

[thomsbg]

As a newcomer to Ractive, the concept of proxy events and all their benefits is a somewhat unique feature and major draw to the library. Complicating that syntax with fire("event", ...) would be a step in the wrong direction IMO.

As verbose as multiple eventName:{{args}},{{in}},{{braces}} may be, it at least feels consistent with interpolating properties elsewhere in the template.

[Rich-Harris]

@thomsbg Definitely agree that having a way to fire events in a way that's 'consumer-agnostic' is important - my hunch though is that the vast majority of uses for events are better met by calling methods directly. Personally, my app code is littered with things like...

ractive.on( 'select', function ( event, selected ) {
  this.set( 'selected', selected );
});

...where there's really no benefit to treating it as an event. I actually find it very rare that events make more sense (outside the context of components, where it's necessary to listen to child component events), but I'd be interested in any examples you have where events are necessary and on-click='foo' (without any arguments) doesn't suffice.

In any case, both syntaxes will be supported from 0.5.6, and if we do deprecate it it'll be after we've thoroughly discussed the pros and cons.

In the meantime, I'll merge this PR - am excited about this one, it's going to save us all a ton of code.

[MartinKolarik]

In any case, both syntaxes will be supported from 0.5.6, and if we do deprecate it it'll be after we've thoroughly discussed the pros and cons.

In the meantime, I'll merge this PR - am excited about this one, it's going to save us all a ton of code.

👍

[martypdx]

am excited about this one, it's going to save us all a ton of code

👍

I actually find it very rare that events make more sense (outside the context of components, where it's necessary to listen to child component events), but I'd be interested in any examples you have where events are necessary and on-click='foo' (without any arguments) doesn't suffice.

With bubbling events you might very well want the component event to have args

[unity]

Adding my contribution to the discussion.
I share @thomsbg concerns:

We're building a rather large dashboard, full-ractive (multiple rewrites, each making more and more use of it) and we rarely have single line event handler methods as opposed to you @richharris - most are doing a few operations to massage data - different coding styles I guess.

I'm also worried about crossing a conceptual line here: as much as I am at peace with adding logic to the views with expressions, because it simplifies code that much, I'm troubled by adding state-mutating code directly there. Reminds me of this ugly inline JavaScript we had when DHTML became a trend.

I'm also worried about discoverability: on larger apps, having two places for each component where data can be modified seems like a recipe for headaches 2 month down the road

Again that's my 2 cents. Have to validate those assumptions ;)

[Rich-Harris]

Now that this issue has been merged, it's probably better to continue the discussion about deprecating the old syntax on a separate thread, otherwise this one risks getting buried. I've just opened #1172 for that purpose.