backbone.marionette icon indicating copy to clipboard operation
backbone.marionette copied to clipboard

triggerMethod is not consistent between views & behaviors

Open JSteunou opened this issue 9 years ago • 14 comments

triggerMethod called from a view with the matching method in the same view return the value returned by the method but triggerMethod called from a view with the matching method in a behavior of that view does not return the value returned by the method.

Would be nice to return all values in an Array of all methods called if many.

JSteunou avatar Jul 01 '15 15:07 JSteunou

I've always considered the return an odd functionality for triggerMethod as there can be multiple handlers. And behaviors is an example here as well. What if both the behavior and the view handle the onEventMethod? What would be returned?

paulfalgout avatar Jul 01 '15 15:07 paulfalgout

Quoting myself

Would be nice to return all values in an Array of all methods called if many.

JSteunou avatar Jul 01 '15 15:07 JSteunou

Or Array of [view returned value, {behaviorName: behavior returned value, ...}]

JSteunou avatar Jul 01 '15 15:07 JSteunou

so would this then be [undefined, {behaviorName: behavior returned value, ...}] when the view is not handling the trigger?

paulfalgout avatar Jul 01 '15 15:07 paulfalgout

Why not... It's just some naive solution.

My point being that triggerMethod is not consistent. Another solution would be to make it never return an object.

JSteunou avatar Jul 01 '15 15:07 JSteunou

Yep. I'm not really arguing against this or anything. I agree it is not consistent. I'm just not sure of a clear consistent fix. Anything I imagine would be a breaking change as well.

paulfalgout avatar Jul 01 '15 16:07 paulfalgout

That's right.

JSteunou avatar Jul 01 '15 16:07 JSteunou

This bit me a few days ago. I had to make the the view implement the method in the Behavior. E.g.

initialize: function() {
   this.view.onSomeTrigger = this.someTriggerHandler.bind(this);
}

And then use it in the view.

var retval = this.triggerMethod('some:trigger');  // this will execute someTriggerHandler in the behavior

Any advance on this?

I have seen the current implementation and I see that there is a return value being saved if the view has a handler method and then it proceeds to call the view behavior and parent views.

Why not check if the view return is undefined and then try to execute the behavior/parent view and get a return value?

Problem is with different behaviors implementing the same method.

ElderMael avatar Jul 22 '15 03:07 ElderMael

So, I want this feature too. I'm not harping about the inconsistent behavior coming out of triggerMethod, rather I want triggerMethod to pass along returns. I like to use Promises on my handlers, and triggerMethod is almost useless without a return value, in that respect.

This one, I think, is very tricky. For one, triggerMethod is found all over the place. And, then, we have methods (which are very useful, true) like this:

triggerMethod: function() {
    var ret = Marionette._triggerMethod(this, arguments);

    this._triggerEventOnBehaviors(arguments);
    this._triggerEventOnParentLayout(arguments[0], _.rest(arguments));

    return ret;
},

Where, both _triggerEventOnBehaviors and _triggerEventOnParentLayout are juggernauts (the first, because it loops through potentially multiple behaviors, the second because it could have all kinds of recursive (see childEvents) and deep behavior (it probably triggers the a parent LayoutView's parent, right?).

So, while I started this post thinking I'd put some thought into building up triggerMethods, I'm signing off thinking the triggerMethod should consistently not return a value. :) Go figure...

Seebiscuit avatar Jul 30 '15 20:07 Seebiscuit

I do agree with the fact that triggerMethod shouldn't return any value at all. That being said, a simple workaround would be :

view.triggerMethod('some:trigger', obj);

And then in any view or behavior :

onSomeTrigger: function(obj) {
    obj.return_value = 'Hello world !'; // Or make it an array if you expect multiple return values
}

It's kind of hacky but it does the job

arthurjaouen avatar Oct 29 '15 15:10 arthurjaouen

How this will evolve in v3 @paulfalgout?

JSteunou avatar Jan 08 '16 13:01 JSteunou

this isn't on the v3 roadmap

paulfalgout avatar Jan 08 '16 16:01 paulfalgout

So for now in next triggerMethod still have the same 2 behaviors than in v2? I did not look closely to see if there were re-written or not.

JSteunou avatar Jan 08 '16 16:01 JSteunou

I don't think the behavior or triggerMethod is changing at all for v3, but along with possibly just being called trigger on a Marionette.Events mixin things will probably change for a v4. And whatever v4 is, it'll be smaller and on a shorter release cycle.

paulfalgout avatar Jan 08 '16 22:01 paulfalgout