Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

triggerMethod is not consistent between views & behaviors #2650

Open
JSteunou opened this issue Jul 1, 2015 · 14 comments
Open

triggerMethod is not consistent between views & behaviors #2650

JSteunou opened this issue Jul 1, 2015 · 14 comments

Comments

@JSteunou
Copy link
Contributor

JSteunou commented Jul 1, 2015

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.

@paulfalgout
Copy link
Member

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?

@JSteunou
Copy link
Contributor Author

JSteunou commented Jul 1, 2015

Quoting myself

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

@JSteunou
Copy link
Contributor Author

JSteunou commented Jul 1, 2015

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

@paulfalgout
Copy link
Member

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

@JSteunou
Copy link
Contributor Author

JSteunou commented Jul 1, 2015

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.

@paulfalgout
Copy link
Member

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.

@JSteunou
Copy link
Contributor Author

JSteunou commented Jul 1, 2015

That's right.

@ElderMael
Copy link

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.

@Seebiscuit
Copy link

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...

@arthurjaouen
Copy link

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

@JSteunou
Copy link
Contributor Author

JSteunou commented Jan 8, 2016

How this will evolve in v3 @paulfalgout?

@paulfalgout
Copy link
Member

this isn't on the v3 roadmap

@JSteunou
Copy link
Contributor Author

JSteunou commented Jan 8, 2016

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.

@paulfalgout
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants