Accept Function in "on-<event>" attributes

jasonmacdonald commented 7 years ago

New Feature

Description

Currently, the only thing you can pass to the onClick attribute is a string which has to be a method on the parent component class. Now that we can access the component directly in the template, I'd like to propose being able to pass a function to call instead of a string, or at least to pass the context for which the method should be invoked.

Context

This would open up the opportunity to call methods exposed by child components without having to write methods on the main class just to facilitate a click taking an action on another component
```
<child-comp key='child'></child>
<a onClick(args => component.getComponent('child').method(args), 'some arg'>Click Me</a>
```
When transcluding components, you are forced to jump through hoops in order to call a method on the component which is transcluding the content. This would allow you to expose the component to the transcluded content and call the method on it, instead of the parent.

<list data=[1,2,3,4,5] >
   <card onClick(() => list.setSelected(item)) data=item class=[selected]  />
</list>

Inside list

class {
   setSelected(item){ this.state.selected = item }
}
<for(item in input.data)>
   <include(input.renderBody)  list=component item=item selected=(item==state.selected) />
</for>

Currently, in order to set the selected item on List when the Card is clicked, you have to do a chain like:

Card Click -> Call Parent Save -> Save state on Parent -> Pass state to list -> list: save state -> pass selected to Card as var on include

This is all because the context of "card" is not list, despite it being used as a renderer (child) for List. If you could at least reference the component and pass it's method to the event handler, it would allow for better reuse of components without having to bind everything up to the parent component and then back down, just to set it's own internal state. I'd much rather have Card be able to set state and call methods on List, then have list emit an event that can be listened to by parents if they are so inclined.

Now, I'm not suggesting that we just bind transcluded content to the components state, as I'm sure there are legit use cases for treating it separate. This would simply allow you to do either.

As another example, take a carousel which provides Forward and Backward methods, but leaves it up to you to call them. Perhaops even offering a transcluded region within the component to place them.

e.g.

<carousel items=data.items>
   <@nav>
      <a onClick(() => carousel.prev())>Prev</a> | 
      <a onClick(() => carousel.next())>Next</a>
   </@nav>
</carousel>

This style of composed widgets allows me to reuse carousel in many ways, without having to include a particular nav type into the component itself. I could provide another component in @nav that handled it as a paged list, for example.

Possible Implementation

I would suspect this is an easy change as a simple type check on the first argument should tell you if you are trying to call a method on the parent component (String) or a function (Function).

Discussion

I'll admit I don't love the need for the arrow function, but you need to supply context for the method execution, so it's either that or signature like event('method', context, ...args), or perhaps an object event({m: 'method', c: context}).

patrick-steele-idem commented 7 years ago

I could be misinterpreting your comments, but I just want to make it clear that the following works in Marko v4 because Marko v4 supports binding additional arguments to an event handler (see: http://markojs.com/docs/components/#codeon-eventmethodname-argscode):

class {
   setSelected(item){ this.state.selected = item }
}

<list data=[1,2,3,4,5] >
   <card onClick('setSelected', item) data=item class=[selected]  />
</list>

The reason for not allow functions to be used as the first argument to an event handler is that functions cannot be serialized down to the browser if the page is rendered on the server (functions are not serializable as part of JSON). With Marko v4, if the page is rendered on the server then event handlers will work as expected when the application boots in the browser. This is because we serialize the method name to the browser (it's just a String) and use that to handle the event in the browser.

Does that clarify?

jasonmacdonald commented 7 years ago

It clarifies why only strings are support, thanks! Unfortunately, your example is exactly what I'm trying to avoid. The selected state is not a property of the outer most component class{}, it's a property of the List component (<list>). I'm trying to set the value for List from card, without having to write the boiler plate on the outer most parent (class{}).

In order to have a click on card set List's selected property, I have to write...

class {
   setSelected(item){ this.state.selected = item } // boilerplate
}

<list data=[1,2,3,4,5] selected=state.selected>
   <card onClick('setSelected', item) data=item class=[selected]  />
</list>

This might be OK for a single component in a template, but when this component has 10+ child components that also transcludes stuff - says it's a Page component with multiple components in it's body - then I end up with tons of these event handlers and state values. And the only reason I need them is to pass the state from the transcluded child to it's parent component. I believe this also causes the whole outer parent (class{}) to rerender because of a state change, when the change was really internal to the list component.

What I'm really trying to achieve is this:

class {
   // no state or event handlers
}
<list data=[1,2,3,4,5] >
   <card onClick(list.setSelected, item) data=item class=[selected]  />
</list>

Clicking a card would just set List.setSelected = card.item without bleeding functions/state into the outer class{} to facilitate.

The only way I can do this now is to put card inside list's template, then card can call list functions directly, but then my re-usability is less because I can't supply different cards for different appearances.

Does that make sense? I have lots of examples where I'm using transclusion in this way, but the fact that the transcluded component/template is not in the context of the component that's including it, means I have to write boiler plate everytime. :(

jasonmacdonald commented 7 years ago

I can see this comment in the output of AttachBubbleEvents which "seems" to indicate you can provide a component ID for the method

// Attributes will have the following form:
// on<event_type>("<target_method>|<component_id>")

But I can't seem to figure out how to invoke it. Is there a way to say what component the target_method should be called on??? This would solve my problem if I could target a specific component_id.

It seems to be referenced again in getEventAttribute as

// <method_name> <component_id>[ <extra_args_index]

But I never seem to be able to enter the if block for this because virtualAttrs is always set with the parent ID already.

gilbert commented 7 years ago

Here's a solution that I've used before:

class {
   // no state or event handlers
}
<list data=[1,2,3,4,5] >
   <card onClick('setState', 'selected', item) data=item class=[selected]  />
</list>

jasonmacdonald commented 7 years ago

Thanks @mindeavor , I assume this would still require passing in selected to the list component to set it's state? It's better, but I still couldn't call methods on List from the event, which is the other part I'd like to do.

All we need is a way to tell onClick which component we want the method called on - assuming key=id is set ont he target we want...

onClick('id:method')
onClick('id method')
onClick('id|method')
onClick('method') eventId='id'

Anything like this that let us target the component to be called would solve all my issues :)

But thanks for the tip @mindeavor

jasonmacdonald commented 7 years ago

@patrick-steele-idem Any advice? Is it possible to set a target component for the method execution? The code seems to elude to it, but I can't figure out where it's set. I'm guessing in the AST somewhere?

TJKoury commented 7 years ago

You actually can serialize functions, I'm working on a dashboard using VueJS that takes advantage of that.

guilhermeaiolfi commented 7 years ago

It seems a reasonable request. You end up with highly coupled components. But there are those situations where you can't use a child component by itself (think about

marko-js / marko