ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

Discussion: should we deprecate the old proxy event syntax? #1172

Closed Rich-Harris closed 8 years ago

Rich-Harris commented 10 years ago

This follows on from the discussion on #1146 - see in particular feedback from @thomsbg (https://github.com/ractivejs/ractive/pull/1146#issuecomment-53526330) and @unity (https://github.com/ractivejs/ractive/pull/1146#issuecomment-54030188).

As of 0.5.6, we have two ways of dealing with events:

Direct method call (new style)

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

Triggering a proxy event, and handling it separately (old style)

<button on-click='select:{{foo}}'>select foo</button>
ractive.on( 'select', function ( event, selected ) {
  this.set( 'selected', selected );
});

These two are functionally identical, but semantically very different - the new syntax is far more concise and convenient, but introduces a coupling (between the event and the action) that wasn't there previously. For example, if you were using a Flux-style architecture, you wouldn't want to set() data directly.

Option 1: deprecate

If we did deprecate the old syntax entirely, you can still maintain that decoupling:

<!-- this... -->
<button on-click='fire("select",event,foo)'>select foo</button>

<!-- is identical to this: -->
<button on-click='select:{{foo}}'>select foo</button>

You can also call custom methods so that you're not blindly setting data, but calling the same API that you'd call programmatically:

var TodoList = Ractive.extend({
  add: function ( description ) {
    // any validation etc goes here...
    this.push( 'todos', { done: false, description: description });
  }
});

Pros:

Cons:

Pros:

Cons:

One idea we briefly tossed around was keeping on-click='foo' as a short version of on-click='fire("foo",event)', but disallowing additional arguments. This alleviates the problem of situations like

<widget on-foo='bar:{{these}},{{are}},{{ignored}}'>...</widget>

In that example, the bar handler receives the original foo event's arguments - the additional arguments are ignored. We don't have a particularly good way around this. Disallowing arguments masks this problem rather than solving it, but maybe there's some value in it.

Pros/Cons:

If you think about it, the curly braces are fairly redundant. For data-binding, they serve as delimiters - it says...

{{ <!-- references to the viewmodel are allowed in here --> }}

...but with method calls, the directive itself serves that purpose:

<button on-click='doSomething( /* references are allowed in here */ )'>click me</button>

We could apply the same logic universally and just do away with the curlies...

<button on-click='select:foo'>select foo</button>

...though for the sake of avoiding total confusion, it'd probably be better to go with something different, such as...

<button on-click='select[foo]'>select foo</button>

<!-- If there we no arguments, syntax would be as currently is: -->
<button on-click='select'>select</button>

...which I personally find rather appealing. It gives us the opportunity to reverse an earlier mistake (whereby anything that couldn't be parsed was simply treated as a string - so select:xyz meant the same as select:"xyz" - bad API decisions are often made in the name of being 'forgiving' and 'flexible'. Mea culpa), and works equally well for decorators and transitions.

Pros:

Cons:


So: would love to know what everyone thinks. Options 1 and 4 are my personal favourites, but your needs may well differ from mine. One thing I will say - I don't think we should worry about deprecating things. Ractive is still relatively young, and it's better that we take opportunities to get things right rather than trying to prevent the pain of updating existing templates. Deprecations can be well signposted with appropriate console warnings etc.

Thanks everyone.

lburgess commented 10 years ago

I much prefer the reduced code within Option 4 and the fact that it allows one to choose the most fitting style depending on personal taste and/or the problem at hand.

It is probably worth noting that NOT using {{ }} is inconsistent with the way in which one would reference a data item anywhere else, including element attributes - which the on-event arguably is. NOT using {{}} may also introduce some minor inconvenience if one wishes to pass an absolute value in as one or more of the arguments:

<button on-click="select:'string1',123">Click Me!</button>

... should the number 123 be used to resolve a keypath or is it an absolute number?

That said, if we go down this route, I personally prefer the format which uses a colon:

<button on-click='select:arg1,arg2'>Click Me!</button>

And in the case of passing in absolute values, would be easy to parse

<button on-click="select:'string1',123">Click Me!</button>

and one can either fallback to Javascript's dynamic casting or manual cast the argument value within one's handler:

<button on-click="select:'string1','123'">Click Me!</button>

ractive.on('select', function(e, string, number){
number = Number(number);
})

I don't see this as a show stopper, but thought I should raise the point.

kabootit commented 10 years ago

The "array syntax" looks out of place in this context given its usage in associative arrays. 1 over 4 gets my vote.

thomsbg commented 10 years ago

Having sat on this for awhile, I think I would choose Option 3. Maintaining multiple syntaxes inside event attributes (as in Option 2 or 4) seems difficult to maintain and document. Option 1 feels a little too verbose for my common case. So that leaves Option 3, which preserves the 80% use case for proxy events, and doesn't involve passing around any magic event parameters.

unity commented 10 years ago

I have the same opinion. I like the idea of #4 but Array notation does not convey the right meaning I feel.

May I suggest something that would look like a method call but keep the method call syntax and still go through event proxies ?

Here's the coding style I employ today:

new Ractive({
  init: function(){
    var self=this;
    _.each(this.events, function(proxy, event){
      self.on(event,_.bind(proxy,self));
    });
  },
  events:{
    select: function(event, main, secondary){...}
  }
})

Here's how it could work then by baking this in (a-la backbone):

<button on-click='select(foo, 123)'>select foo with 123</button>
<button on-click='select(foo, "bar")'>select foo with "bar"</button>
<button on-click='select'>select without arguments</button>
new Ractive({
  ...
  events:{
    select: function(event, main, secondary){...}
  }
})

It seems to me like an intuitive syntax and has the benefit of a clean fallback, but maybe I'm missing some finesses in that.

Only difference is that method calls are "enriched" with an event arguments and sandboxed to the events hash. If that's acceptable then it seems like a nice tradeoff.

Maybe I'm out of scope and the decision has been made, but I like the fact that you need to explicitly declare event handlers in a separate hash.

I feel it categorises concerns and safeguards you from calling any method. For bigger components with a lot of methods, I find it a good thing: the opposite encourages unclean practices mixing what I consider "internal" methods vs events.

Just my 2 cents

kabootit commented 10 years ago

https://github.com/ractivejs/ractive/issues/1172#issuecomment-54161876 +1

MartinKolarik commented 10 years ago

... should the number 123 be used to resolve a keypath or is it an absolute number?

123 is a number and should be treated as such (and is!), but you can still access 123 in the current context using this[123] or this.123.

marcello3d commented 10 years ago

on-click='set("selected",foo)' is a bit confusing for me. My assumption is it would call a method set in the data object (similar to {{#if filter(foo)}}).

lburgess commented 10 years ago

@MartinKolarik

123 is a number and should be treated as such (and is!), but you can still access 123 in the current context using this[123] or this.123.

Indeed it is - not quite sure where I was going with that one!

dodyg commented 10 years ago

The nice thing about the old syntax is I can use dash '-' for event name, which is not a valid JavaScript function argument.

<button on-click='do-something-complicated' />
maxgalbu commented 10 years ago

I'd go with option 1, but without the mandatory event parameter (if possible):

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

I agree with https://github.com/ractivejs/ractive/issues/1172#issuecomment-54097607

It is probably worth noting that NOT using {{ }} is inconsistent with the way in which one would reference a data item anywhere else, including element attributes - which the on-event arguably is.

...but i wouldn't maintain two syntaxes for the same thing, and this:

<button on-click='fire("select", {{ foo }})'>select foo</button>

could be too verbose :)

evs-chris commented 10 years ago

After looking at all the code I have that is using events, my preference would be option 3.

My only reason for passing args is to gather context for the event, which I didn't know at the time was already available in the event context. Anything more elaborate than immediate context and indices, like ancestor references, would probably be ok to bump to using with the fire method.

I would say no to option 1 because proxying is just too darned convenient.

As far as - in event names, that would be supported regardless as on-click="fire(event, 'do-something-complicated'), but it's still not as convenient as on-click="do-something-complicated" that would still work with option 3.

codler commented 10 years ago

I have a few question :P Can "method calls" do everything "proxy event" can?

How do these "proxy event" look in "method calls"?

<!-- Restricted references -->
<button on-click='select:{{.foo}}'>select foo</button>
<!-- Ancestor references -->
<button on-click='select:{{../foo}}'>select foo</button>
<!-- Root references -->
<button on-click='select:{{~/foo}}'>select foo</button>

Hm this can be confusing if you have like below :P

var ractive = new Ractive({
    data: { 
        name: function() { return 'b'; }
    }
});
ractive.name = function(a) { alert(a) }
<p on-click="name(name())">a</p>
martypdx commented 10 years ago

Right now, I'm for Option 2 mostly because I think more time and usage would bring more insight.

Going back over my current project this week and updating to the new features (method calls, yield, event bubbling) and reading through the comments, I offer the following observations:

Proxy events with args are a necessary feature (and both the fire( and [] options seem like a step backwards). They're most useful at the component level:

{{#items}}
<widget item='{{.}}'/>
{{/}}

ractive.on('widget.select', function(item){...})

Relying on context can force weird template constructs, be less clear in the template or forces the subscriber to know intimate data details of the component.

// data something like { foo: 'bar', title: 'title' }

// with args:
<li on-click='select:{{foo}}'>{{title}}</li>

//without, either template gets hacked
{{#with foo}}
<li on-click='select:{{.}}'>{{../title}}</li>
{{/}}
//or subscriber has to "know" about data
r.on('widget.select', function(e){
    var foo = e.context.foo //you need knowledge of component data model
})

I did find that not having the mustache refs made the data stand out less in the template. Maybe just my editor, but cognitively the args in the method don't stick out like the other data usages.

I think another part of the discussion is about developer preference and style as to whether to use the eventing on style structure, or put methods on the ractive instance. If we added the direction option of setting on handlers:

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

Then it's a wash and works equally well. Maybe just an option for how you want to handle events.

The most interesting thing about methods, IMO, is that they really are about custom two-way bindings. I already often try and use radio and checkboxes:

//so instead of
{{#items}}
<li on-click='set("selected", {{.}})'>{{name}}</li>
{{/}}

//I do
{{#items}}
<li><label><input type='radio' name='{{selected}}' value='{{.}}'>{{name}}</label></li>
{{/}}

Yes, it's verbose, but it's appealing semantically and because I'm delegating more to the browser and ractive. I think this is the type of problem ractive should help make simpler.

Tying events to data is what data binding is all about. <input value='{{foo}}'/> mean tie the change event (and possible some others) to the setting of the data item foo based on the value in the input.

Reactively speaking, <button on-click='set("selected",foo)'>select foo</button> means modify the selected data when a click occurs.

I bring this up because, while there's grey area, pub/sub events and custom bindings are really separate things. Maybe the binding case needs to be it's own attribute construct:

//some possibilities to give you the idea:
<button click-value='set("selected",foo)'>select foo</button>

<button click-set='{{selected}}:{{foo}}'>select foo</button>

<button custom-animate='{{item}}:{{foo}}'>select foo</button>

Just some thoughts, I do think we're on to something. But as to what and how, I think waiting and usage would be best course for now...

arxpoetica commented 10 years ago

Is there a 5th option (serious question) where events are bound only per a javascript call, for example, the good ol' JavaScript way? (Separation concerns of JS from HTML.)

martypdx commented 10 years ago

After another week of thought and ractive use, I had a lightbulb moment and I get where @Rich-Harris is coming with Option 1. However, I wanted to propose Option 1B which moves to a single syntax, but uses the existing style instead of the newer "method" syntax

Option 1B

<button on-click='fire: select, {{foo}}'>select foo</button>
<button on-click='set: {{hover}}, false'>select foo</button>

This provides a single, cognitive way to think about events: on-event='controller_action: argument' AND retains the similarity in syntax to decorators and transitions:

on-event='action: arguments'
decorator='name: arguments'
intro='name: arguments'

IMO, declarative directives are easier to comprehend in the context of templating.

With the current method call syntax, there's no ref for the action and no subscribe/unsubscribe ability. This can be added, but I think when you start looking at dynamic directives with the method syntax, it doesn't work quite as well syntactically:

<button on-click='{{#active}}set{{/}}("foo", foo)'>select foo</button>
<button on-click='{{action}}("foo", foo)'>set foo</button>
<button on-click='set("bar", foo + (active ? 3 : 12) )'>copy</button>

versus

<button on-click='{{#active}}set{{/}}: foo, {{foo}}'>select foo</button>
<button on-click='{{action}}: foo, {{foo}}'>select foo</button>
<button on-click='set: bar, {{foo + (active ? 3 : 12)}}'>copy</button>

I'd also go with Rich's suggestion to tighten up the API, but go in the direction of requiring references to be mustached. Meaning in on-event='fire: selected, {{foo}}' selected is a primitive, not a data reference, though optional quotes should be allowed: on-event='fire: "selected", {{foo}}'

Pros:

Cons:

martypdx commented 10 years ago

events are bound only per a javascript call, for example, the good ol' JavaScript way

@arxpoetica not following you here, can you elaborate?

arxpoetica commented 10 years ago

@martypdx inline on-{event} handlers (ala Angular and Ractive) go contrary to philosophy/doctrine of "separation of concern." I think I read somewhere (can't remember now) that @Rich-Harris actually likes to throw styles, scripts, and html into single files (at least in components). This goes contrary to the generally accepted idea that JS goes in JS files, CSS goes in CSS (SCSS/LESS/Stylus) files, and HTML goes in HTML files. It's generally not fun trying to track down event handlers in HTML when looking at JavaScript. Capiche?

So, opinions aside, does Ractive allow adding handlers purely via JavaScript?

Rich-Harris commented 10 years ago

does Ractive allow adding handlers purely via JavaScript?

Of course! It's just DOM, after all. So if you had a template like

<button on-click='foo'>click me</button>

and you didn't want the on-click bit, then instead of doing

ractive.on( 'foo', fooHandler );

you could do

// any CSS selector works with ractive.find()
ractive.find( 'button' ).addEventListener( 'click', clickHandler );

As an aside, just because something's generally accepted doesn't mean it's right :-) I wrote this on the subject a while back, I suspect that's what you're thinking of. Probably best to discuss elsewhere though, this thread is a holy-war-free zone!

kabootit commented 10 years ago

I think this side subject is definitely worth noting in this discussion because it is central to what makes Ractive (and Reactive) approach special. Put another way, ran across this from https://news.ycombinator.com/item?id=8330456

"...separation of concerns is a good thing, but separating HTML and Javascript is not separation of concerns; it's just an arbitrary separation of technologies.

The mindset of React is that you build independent components, where each one contains everything to render information for the user. Whether you use 1, 2 or 3 technos (HTML+CSS+Javascript) for this component is not an issue and there's no reason to separate them."

So while assigning events directly in html markup felt completely wrong to me at first, the implications for code organization is starting to make sense to me.

arxpoetica commented 10 years ago

Sorry if my comments seemed off topic—can easily discuss elsewhere. And thanks for the answers. :)

Iverson commented 10 years ago

On my modest opinion the most common useful case is direct method call, proxy-event needs less frequently, so nothing scary use on-click='fire("select",event,foo) when it necessary. For me more important these few moments: 1) Be able to preventDefault behavior by return false; from direct method call. 2) Optional passing event argument to method calling. 3) Not using verbose {{}} for passing method arguments. When passing more than 2 arguments brackets make string unreadable. I like Angular style where whole attribute string is expression with scope context (Ractive instances context in our case).

halfnelson commented 10 years ago

Not sure how late I am but I heartily go for 1B! I love a language/tool that is consistent and has a core set of concepts that can build into something more complicated. I don't need to reach for the manual to work out how to do the more complicated thing, since I know that it is built of the core concepts and can have a good guess.

If I was new to ractivejs and only had just grasped mustaches and the main ractive class layout, then when trying to write a on click handler I would immediately assume the syntax would be: <a on-click='myfunc("static", {{dynamic}})'> and that my function would be executed with this set to the ractive.

Now if the on-click attribute does not just accept regular javascript (not sure if it does) then it would be wise to make it not look like valid javascript and go with the 1B option of <methodname>:<static arg>,{{<dynamic arg>}} with optional quotes for static arg (or even mandatory)

if you wanted on-click (which looks very much like onclick in dhtml) to rename the event and bubble (proxy) and didn't want to type on-click="fire: <event>", then maybe a different attribute could be the shortcut proxy-click="select"

This keeps it all consistent, {{}} for data access within the mustache template, on-click acts like onclick in javascript except that this is set to the ractive instance not the dom node (which is fair enough) proxy-click is a new concept and is ractive specific and can do what it wants, it is also optional (syntax sugar for the long hand on-click="fire:<blah>"

dagnelies commented 10 years ago

Well, since everyone gives it's opinion, I'll give it my grain of salt to. I vote for option 1, having several syntaxes lying around just makes it more confusing. About option "1B", I must say it's not my taste, I prefer "normal" method call syntax.


I'll also take the opportunity to add another suggestion for a future iteration. I think there are a lot of great things, but some are not as intuitive as they could be (IMHO).

...for the events, as a fresh user, what I found confusing was not only how to call ...hmmm "handlers", but also the way to declare those. Especially when declaring them within components:

new Ractive({
    onrender: function() {
        this.on( 'activate', function ( event ) {...

Actually, to go even further, I confess I kind of missed the whole reason why there are two kind of event mechanisms at all. To be blunt, "normal" events seem easier and more intuitive to use for average users (like me):

<button onclick="show(event, '{{id}}')">...

And show being declared like usually. However, there is one detail that could be greatly helpful: to extend the standart event by adding ractive instance and keypath to it. Like event.ractive and event.keypath.

Your original example would transform from:

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

Into:

<button onclick='event.ractive.set("selected",{{foo}})'>select foo</button>

...and everything is back to the basics, still intuitive and non-intrusive. Sometimes, a bit of simplicity makes wonders.

thomsbg commented 10 years ago

I vote for Option 1B. I like the consistency between events, decorators, and transitions. While Option 3 makes my common use case the most concise, I'd be willing to trade that conciseness for a single events codebase.

I strongly dislike the method call syntax introduced in 0.5 (on-click="foo('bar')"). This syntax makes it appear as though ractive allows arbitrary javascript there, which just leads to more confusion. I also dislike the need for magic event keywords in the binding.

dagnelies commented 10 years ago

On one hand, "normal" onclick events are:

On the other hand, on-click events:

Both have their use. However, the ideal case seems to me the combination of both.

I don't know if it's a technical limitation or if the complexity is simply too great, but finding a way to fully bind values directly inside arbitrary 'onclick' declarations sounds like the holy grail. Instead of calling the handlers directly, it could be wrapped during templating, allowing to pass the variables and extending the event in one sweep.

...and I'd also prefer @Rich-Harris to stick to the current syntax (which I like) rather than spending more time on changing the syntax once more.

martypdx commented 10 years ago

I don't know if it's a technical limitation or if the complexity is simply too great, but finding a way to fully bind values directly inside arbitrary 'onclick' declarations sounds like the holy grail. Instead of calling the handlers directly, it could be wrapped during templating, allowing to pass the variables and extending the event in one sweep.

Not really possible generically, but nothing stopping you from doing window.ractive = new Ractive({...}) or registering your components (or their methods) in their onconstruct in some identifiable manner. Then doing:

<button onclick='ractive.show(foo)'>show foo</button>

hack on my friend...

alt text
fskreuz commented 10 years ago

Skimming through the discussion, I'll just pitch in my thoughts.

I usually just do ala option 3, where I just assign the handler. The data I usually embed as data-*, get them from event.context or just directly via get() in the handler, whichever applicable. This frees up the template from all that clutter while turning over the control more to the JS.

<a href="#" on-click="hazClick" data-x="{{x}}">...</a>
hazClick : function(event){
  var x = event.context.x;
  var x = this.get(event.keypath).x;
  var x = event.node.getAttribute('data-x'); // if using data-*
}
marcello3d commented 10 years ago

What if we just supported onclick and supported "smart" {{mustache}} syntax inside:

<button onclick="alert('hi')">
<button onclick="alert('{{value}}')">
<button onclick="{{this}}.set('foo','bar')">
<button onclick="{{this}}.myfunc({{object}})">
<button onclick="{{this}}.invoke('event-name', {{object}})">

Would that be clear?

Alternatively I could imagine a pseudo-global ractive or __ractive variable being made available to events:

<button onclick="alert('hi')">
<button onclick="alert(ractive.get('value'))">
<button onclick="ractive.set('foo','bar')">
<button onclick="ractive.myfunc(ractive.get('object'))">
<button onclick="ractive.invoke('event-name', ractive.get('object'))">
martypdx commented 10 years ago

What if we just supported onclick and supported "smart" {{mustache}} syntax inside:

You can do that today with the references (see http://jsfiddle.net/s9d7kgx2/). I suppose we could render something different than what's in the template and identify what to call.

<button onclick="this.set('foo','bar')">

// becomes something like
<button onclick="ractives['r-2'].set('foo','bar')">

Using the native onevent attributes is oddly attractive and repulsive at the same time.

There are also potential CSP issues:

Event attributes can be blocked by using Content Security Policy which if used, blocks all inline scripts unless the 'unsafe-inline' keyword is used.

arxpoetica commented 9 years ago

Using the native onevent attributes is oddly attractive and repulsive at the same time.

I second the repulsive part.

image

dagnelies commented 9 years ago

Well, I second the attractive, but I'm ok with both, it's not such a big deal. I'm also happy with the new syntax as it produces less code.

evs-chris commented 9 years ago

So, I have come across a (fairly) good reason to at least drop the curlies from all of the event/decorator/transition/whatever directives. While working on implementing the handling of directives in conditional attributes, I found that conditional attributes are handled as string templates from which attributes are extracted. That's fine until you try to have mustaches in your mustaches for something like

<button {{#clickable}}on-click="foo:{{tryToBindMeIDareYou}}"{{/}}>Click</button>

The corresponding method-call syntax works just fine. The other directives have the same problem. They work the first time they are rendered but don't get anything bound because all they see are the rendered version of their mustaches. I started to have conditionals pre-processed during parse so they can keep their references around for the directives, but that would keep directives out of partials and other sections too, though that may not be a terrible thing. (<button {{#someVar}}{{>attrs}}{{/someVar}}>)

Since this.event is now available to method-call events, how about have fire automatically provide the event if it's not already provided so that the extra parameter can be skipped? Then option 1 becomes slightly less aggravating as on-click="fire('foo', param1)" over on-click="foo:{{param1}}". It's only 5 extra characters, or 4 if you drop the space after the comma. Sticking with an argument delimiter of some sort would make multiple handlers an option using the fairly standard space-separated (or comma if you're into that sort of thing #1396 :-) multi-value attribute style of html: on-click="foo(bar) fire('foo', bar)"

Adopting a curly-free syntax for other directives could also avoid some confusion as in #1419.

In summary, my stance is:

Bonus:

martypdx commented 9 years ago

While working on implementing the handling of directives in conditional attributes, I found that conditional attributes are handled as string templates from which attributes are extracted. That's fine until you try to have mustaches in your mustaches for something

That's a side-effect of the current implementation. I think it needs to change to be fragment based like everything else. Then refs work just fine, yes?

evs-chris commented 9 years ago

That's a side-effect of the current implementation

Yes, but the fragment-based implementation looks to be pretty hugely gnarly if you allow anything other than explicit attribute settings inside an attribute conditional. Even with explicit attribute settings, each text chunk would have to be checked for directives and spliced out with the correct value part(s) before doing the current string-based method of pulling out attributes.

Letting all mustaches be replaced in the string and not having to worry about them for directives is waaay simpler... unless I'm missing something, which is almost certain.

Having gotten a little further in my implementation, the proxy-style events do work, but they get entirely rebound each time one of their mustache parameters changes. The method-style events bind properly to begin with.

martypdx commented 9 years ago

Yes, but the fragment-based implementation looks to be pretty hugely gnarly if you allow anything other than explicit attribute settings inside an attribute conditional.

But the parser already handles attributes, and what else can you put in an element besides attributes?

In general, I would say pre-parsing is better than late parsing. Doesn't the string approach push more the parsing into the runtime resolution?

The one exception would be a straight reference, not a section: <p {{foo}}>hello</p>. In that case it is by necessity dynamic. Though arguable, like dynamic partials, it goes back to the parser to resolve. EDIT: And you could argue that that case should be <p {{>foo}}>hello</p> and that only blocks and partials are allowed.

Maybe we can pick this up if you push a PR?

RangerMauve commented 9 years ago

It'd be nice if it was more clear in the docs that : has a special meaning in event names. I was bashing my head at why trigger:open was only showing up as trigger.

jarofghosts commented 9 years ago

The new proxy event syntax is disappointingly similar to the native "on" attributes of yesteryear that I also find "repulsive" (re: the thread from a bit ago). I fail to see how this is a step forward and also how this "new" syntax can be considered "proxy events".

The "old" version was a very elegant and flexible solution to a common problem (and, in fact, was one of the things I liked so much about Ractive). For these reasons I humbly request that the syntax is not deprecated entirely, at least.

matthew-dean commented 9 years ago

@jarofghosts Agreed on both points. The fire() syntax is unnecessarily messy and obscures the function being called. The only thing I'd change is that the mustaching in the call seems unnecessary. That is:

<button on-click="select: {{ something }}">

should just be:

<button on-click="select: something">

There are no other "vars" / properties to reference here.

@Rich-Harris

the new syntax is far more concise and convenient

Nope.

matthew-dean commented 9 years ago

Oh, I see that that my suggestion was option 4. I really should read things.

matthew-dean commented 9 years ago

The other thing I could see doing is making an event reference optional. I think newbies would find this MUCH more comfortable:

<button on-click="select(something)">

Need the event passed? No worries, make a locally scoped variable e available for function calls.

<button on-click="select(e, something)">

Now it looks like everything a JavaScript developer is used to. And it should directly call functions within the ractive, rather than having to set ractive.on("select") The listener pattern doesn't really make sense (unless these events bubble through components, which may be the case - I'm not yet a Ractive expert).

I also thought the 1B option was not bad.

martypdx commented 9 years ago

unless these events bubble through components

Yep, that's the main reason to use events vs methods.

Given the introduction of this.event, I'd say there's even less of reason to need event arguments.

So after almost a year, I have to say I now think Option 3 is the way to go. Use methods but allow the simple event synax: on-click='foo' as a short version ofon-click='fire("foo",event)'

matthew-dean commented 9 years ago

I'd say there's even less of reason to need event arguments.

Wait, what? I use event arguments all the time. How does this.event solve that?

evs-chris commented 9 years ago

I think he meant the event argument, not general event args.

evs-chris commented 9 years ago

I misread both the previous, apparently...

I'm for option 3 still too. Having this.event makes manually passing the special named event argument less painful because if you need it, it's just a context property away. It makes the transition from foo:{{var1}},{{var2}} to a method call a lot bit if you don't mind having event only methods on your instance (foo(var1, var2)). Literals are also a bit confusing in for args in the proxy syntax.

I also think fire should automatically propagate the event if it's called as a method event handler and there isn't an event argument in the appropriate position.

thomsbg commented 9 years ago

This issue is something I'd like to see a resolution to before the 0.8 release, one way or another.

After a year of working with ractive in a large application, I vote for any option that has a unified and consistent syntax between event handlers, decorators, and any other specially handled attribute. .

As much as possible, I'd like to limit the number of different syntaxes and concepts in templates. Have an opinion and stick to it. If it means that certain paradigms like proxying events become more verbose, than so be it.

At first, I thought the event proxying syntax was elegant and one thing that made ractive different than other libraries (in a good way). But in practice, I have made little use of event proxying.

90% of event handlers relate to behavior local to the component, so making the 10% case more verbose is fine with me.

IMO, we should promote isolation between components over sharing data and bubbled events. While useful tools, those concepts lead to messy code in larger applications in my experience. On Sat, Aug 29, 2015 at 8:43 PM Chris Reeves notifications@github.com wrote:

I misread both the previous, apparently...

I'm for option 3 still too. Having this.event makes manually passing the special named event argument less painful because if you need it, it's just a context property away. It makes the transition from foo:{{var1}},{{var2}} to a method call a lot bit if you don't mind having event only methods on your instance (foo(var1, var2)). Literals are also a bit confusing in for args in the proxy syntax.

I also think fire should automatically propagate the event if it's called as a method event handler and there isn't an event argument in the appropriate position.

— Reply to this email directly or view it on GitHub https://github.com/ractivejs/ractive/issues/1172#issuecomment-136066639.

Rich-Harris commented 9 years ago

A few quick thoughts:

I know that not everyone feels the same way about the proxy event syntax, but I'd implore you to try and look at it from the perspective of someone new to Ractive. They can either use a familiar function call syntax, or learn a (let's face it, fairly weird) Ractive-specific way of doing things.

So in summary my inclination is basically option 1 – move over to the method-call syntax, and (gradually – probably not as part of 0.8) deprecate the old way.

While I'm here

The one place where I'm continually frustrated with Ractive's event handling is with components:

<!-- this is impossible... -->
<TodoList on-createTodo='persist(id,newTodo)'/>

We don't have a way to use the event arguments in the component's directive, which makes effective composition really difficult. It occurs to me that we could do something like this:

<!-- supposing the `createTodo` event is fired with `id` and `newTodo` arguments -->
<TodoList on-createTodo='persist($1,$2)'/>

<!-- or if it's just fired with the `newTodo` argument -->
<TodoList on-createTodo='persist($1.id,$1)'/>

In other words, arguments like $1 and $2 are understood to have special values, the same way an event argument is replaced with this.event when the method is called.

matthew-dean commented 9 years ago

I'd implore you to try and look at it from the perspective of someone new to Ractive

That's a good point; the colon / curly brace syntax of firing events took a bit to get used to. And direct method calls would be great for situations like:

<input type="checkbox" twoway="false" checked="{{ something < 2 }}" on-change="set(this.keypath, this.value ? 2: 0)" />

Or however that's written, but some way to deal with simple binding variations or type casting. (Incidentally, is that possible currently?)

Rich-Harris commented 9 years ago

@matthew-dean you can do this sort of thing:

on-change="set('isChecked', event.node.checked ? 'yep' : 'nope')"

http://jsfiddle.net/rich_harris/4vqqca5h/

matthew-dean commented 9 years ago

@Rich-Harris Nice!! * Hurries to immediately implement *

thomsbg commented 9 years ago

Totally agree with all of those thoughts @Rich-Harris.

I like the thought behind $1 and $2 arguments. It's not particularly pretty, but gets the job done. I wonder if would be useful to support $@ as the events arguments as an array. That combined with an ES6 style spread operator allows you to pass on all the arguments, without having to know how many there are.

<todo-list on-newTodo="save(...$@)">

I usually turn my nose up at such perl-isms, maybe there's a more natural syntax.