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

Global Action Bubbling for Components #42

Closed
wants to merge 2 commits into from
Closed

Conversation

mgenev
Copy link

@mgenev mgenev commented Mar 30, 2015

RFC

@alexjp
Copy link

alexjp commented Mar 30, 2015

+1

1 similar comment
@jmishra17
Copy link

+1

@opsb
Copy link

opsb commented Mar 30, 2015

One alternative is to skip bubbling completely and dispatch events to the route directly(ok we all know the inspiration for this idea). This seems to be the intent of actions up data down anyway, mutating the view is supposed to be handled by sending actions to the route which modifies the model which causes a rerender.

@workmanw
Copy link
Contributor

+1 for the concept. We have this pattern time and time again in our app. It became so tedious we added reopened the component and added a sendToController function. A quick search of our app indicates we have this use case about 80 times.

Ember.Component.reopen({
  sendToController: function() {
    var args = [].slice.call(arguments);
    var targetObject = this.get('targetObject');
    while(targetObject && !targetObject.isController) {
      targetObject = targetObject.get('targetObject');
    }
    if(targetObject) {
      targetObject.send.apply(targetObject, args);
    } else {
      Ember.asset('`sendToController` is unable to find the nearest controller');
    }
  }
});

That said, I think this might be somewhat at odds with the routable components RFC. It's not completely clear from the RFC what will happen to action bubbling, but it's very clear that RFC indicates the recommended pattern will to passed "wrapped" actions downward to each component / subcomponent.

At either rate, I'm definitely in favor of improving the action pattern so that component don't have to daisy chaining dozens of action downward.

@mmun
Copy link
Member

mmun commented Mar 30, 2015

I'd prefer using services to directly dispatch. Not a fan of bubbling if you know ahead of time who the target is.

@opsb
Copy link

opsb commented Mar 30, 2015

@mmun are you suggesting that services should be injected into the components and actions dispatched against those?

@mmun
Copy link
Member

mmun commented Mar 30, 2015

@opsb Yes that's what I'm suggesting for this use case.

@opsb
Copy link

opsb commented Mar 30, 2015

That sounds like a pretty nice pattern actually.

@mmun
Copy link
Member

mmun commented Mar 30, 2015

Dispatching directly to the route seems reasonable too. I think there is a balance to be found.

@mgenev
Copy link
Author

mgenev commented Mar 30, 2015

I personally care little how it's implemented internally, I'd let those intimately familiar figure that out, what matters to me is that the user facing api allows me to do this easily.

@opsb
Copy link

opsb commented Mar 30, 2015

I think dispatching to the route would make for a much simpler out of the box experience than we've had until now. You can then introduce services as your app becomes more complex.

@workmanw
Copy link
Contributor

@mmun the biggest advantage of bubbling is you can handle the same action differently depending on where the user is in the app. We have two primary cases:

  1. In our case, we have security containers, such as project. When a user popover is opened in a security container the popover contains details about the users role. Meaning the popover does not have to take on all the information about the state of the app. The action that handles it can populate the necessary info.
  2. We have realtime and postmessage events sent to our app. We broadcast those events through the router giving each state a chance to handle the message.

It's hard to imagine life without action bubbling. It would devalue ember for us. Obviously not trying to over state it, but we get so much milage from them.

@mgenev
Copy link
Author

mgenev commented Mar 30, 2015

@workmanw excellent contributions to the discussion. Thanks!

@mmun
Copy link
Member

mmun commented Mar 30, 2015

@workmanw Does that contradict anything I've said?

@workmanw
Copy link
Contributor

@mmun Sorry, your last response about "Dispatching directly to the routes" hadn't come through as I was typing. I kind of thought based on your first response you were advocating to get rid of action bubbling. I've heard others in the community express that desire and I was just trying to make my use cases known. Hopefully I haven't detracted from the issue at hand here.

[edit: english]

@opsb
Copy link

opsb commented Mar 30, 2015

In my personal experience I've never found any value in action bubbling so I'd prefer to get rid of it. I wonder if perhaps a middle route would be to provide a sendToRoute(similar to the sendToController that you mentioned @workmanw ) method that's available on components. I also wonder if perhaps there's other ways to handle the use cases you mentioned. Would you mind exploring the 2 scenarios a little to see if they could be solved without action bubbling?

@mmun
Copy link
Member

mmun commented Mar 30, 2015

I'd be very hesitant to remove actions bubbling through routes. It has a lot of uses and we should see how its usage evolves in Ember 2.0 with the new alternatives that will be available.

I don't really like that actions sent to a controller get redirected to the router. I would prefer this to be explicit. When controllers are removed it will no longer be a factor though.

Like I said, I don't like using bubbling as a mechanism for message passing if you have a specific target for the message known ahead of time. Otherwise, bubbling is cool with me.

@opsb
Copy link

opsb commented Mar 30, 2015

Agreed, bubbling through routes makes a lot of sense, it's specifically bubbling up through the page that I'm not a fan of.

@workmanw
Copy link
Contributor

@opsb Without action bubbling I would likely convert to a stacked "pub/sub" pattern. Where each route or routable component would use a mixin to subscribe to a service upon creation and unsubscribe upon destruction. Then my "broadcasts" or "action bubbling" would be proxied through that service, each item on the stack one by one. This would be a more code for me, but I could accomplish it this way. (I've already given this a bit of thought).

@mmun Yea, I would be completely okay if "action bubbling" type actions had to be explicitly sent directly to the router. That seems perfectly acceptable to me.

@opsb
Copy link

opsb commented Mar 30, 2015

@workmanw I hadn't realised you were referring to bubbling through routes and I don't see any reason for that to go away (it's not something I use very often but I definitely see the value in it).

Correct me if I'm wrong but I think we're in agreement that by default actions should be dispatched directly to the route where they then bubble up through the routes (services being another option that can be introduced when appropriate).

@workmanw
Copy link
Contributor

@opsb Yea I should have been more clear about that. I was referring to bubbling through routes. I had heard some whispers that this could be replaced by routable components with a model that requires action handler functions to be explicitly passed down.

But back to this topic. I know there is a private routing service that will become public as some point. Perhaps that's the way this should go. Maybe sendToRoute (or whatever you call it) becomes a function on the component class that will transmit an action to the deepest route and allow it to bubble from there. I guess that's effectively what's being suggested here. I would get a tremendous amount of use out of this in my app.

Thanks for the good back and forth! I'm excited to see what others think and how this shakes out.

@opsb
Copy link

opsb commented Mar 31, 2015

@workmanw BTW thanks for that sendToComponent snippet, I've already put it to use cleaning up some nasty spots that were daisy-chaining the calls before.

@mgenev
Copy link
Author

mgenev commented May 4, 2015

Here are 2 more considerations for adding this functionality

  1. now that we have a dynamic component helper, each of these components that could be rendering have different actions. So for example, let's say I have a page and I have some buttons which would pop a top panel with the desired component depending on which button I click.
    {{#liquid-if showTopPanel class="geo-info" }}
      {{component topPanel}}
      {{else}}
        <div class="pull-left">
          <button {{action 'showStatusWizard'}} class="btn btn-info">Post Status</button>
          <button {{action 'showSearch'}} class="btn btn-info">Search</button>
        </div>
    {{/liquid-if}}

So far so good, however my components are:

  {{feed.post-status postStatus="postStatus"}}
  {{feed.search searchStatuses="searchStatuses"}}

So using the dynamic component helper above, do I just pass every action possibility so I can tie my components' actions to the route?

  1. Furthermore, I'm already nesting components 3 levels so I have to keep repeating that every level. So in my feed.post-status component, I have a wizard, so then my wizard too has to declare:
{{wizard-form postStatus="postStatus" steps=steps status=status}}

and my JS for the wizard AND the post-status have to include:

  this.sendAction('postStatus', this.get('status'));

Which means that my wizard which is meant to be generic is now coupled to this post-status!? That's completely beating the purpose of making reusable components. Am I doing this wrong? This whole searchStatuses="searchStatuses" for actions needs to go. It's confusing and detrimental.

@knownasilya
Copy link
Contributor

// in component.js
myService: Ember.inject.service()

or

{{my-component service=myService}}
<!-- in component.hbs -->
<button type="button" {{action 'test' target=myService}}>Test</button>

Where myService has a test action.

@MiguelMadero
Copy link

I think services are the way to go. Components are intentionally isolated and I think there's a good reason to keep them that way. Composition is really important when using components, however, when nesting components, having an intermediate component knowing about the actions raised by its child components when it doesn't need to handle them breaks that isolation.

I went a few times down the path of manually bubbling actions up, but that quickly got messy and always felt dirty. I looked into automatically doing that, but after a few tries, I realized that pushing some of this concerns to a service-like object (this was before we had services) to do all the coordination was a better pattern. In some instances we had requirements for a component to react to action from a sibling component, letting this "actions" go to a service, which would then push it to all of the interested components was at the end a better strategy. Yes this sounds a bit like Flux in principle. With my old strategy of manually bubbling or the suggested strategy of this RFC of automatically bubbling through the hierarchy or all the way to the route would mean that the parent has now additional responsibilities. With the service, this responsibility belongs to a third object. This lets us break the business logic based on our domain instead of being forced to do it based on route/component hierarchies.

@mgenev
Copy link
Author

mgenev commented May 10, 2015

So are you guys suggesting that I use a service which proxies the actions up to parent components and routes? Does anybody have a working example in a bin? If it isn't a global service which proxies all my actions up easily somehow, but I have to use different services on 'one of` basis, this approach would suck even more than having to redeclare my actions across every level, because then i'd have to introduce another file even every time.

@MiguelMadero
Copy link

@mgenev not really proxy the actions, but actually handle the actions.

@mgenev
Copy link
Author

mgenev commented May 10, 2015

That honestly doesn't make sense to me, seems like a hurdle, not help. I could be convinced otherwise if I see a working example, but how is a service better suited to handle the actions than the route they ultimately bubble to and belong in?

@mgenev
Copy link
Author

mgenev commented May 10, 2015

Even if it is actually more convenient to use a service for this, it would mean Ember's architecture is flawed, because a service is meant to be a global reusable object, not a crutch you have to add on to a route/components every time you need to bubble actions.

@MiguelMadero
Copy link

I think there're better ways of handling modals, but extending on the initial example of the RFC. That could be a modalService. Essentially a service responsible of showing/hiding modals.

Let's see another example. Let's say you have a nested hierarchy or route/component/component like patients, medications, medicationItem. Let's say that the medicationItem has a "delete" button. When we delete, we want to remove the medication from the list. Instead of bubbling the action all the way to the patients route, which IMO, shouldn't be responsible about dealing with medications, we could simply have a "dataService" or access the store (as a service) and modify this data. Then the changes would cascade to the medications list.

With the same hierarchy, let's say that you also have an option to edit a medication, which to make the example interesting, let's assume that it needs to transitionTo('patient.medication', medId); Again instead of having to bubble this action up to the route, we could have a routingService to take care of the transition. Then once we're done saving we could simply use the dataService or store to persist it and then navigateBack instead of relying on a parent route to handle this.

@davewasmer
Copy link
Contributor

@MiguelMadero I think this is the danger of services (they have their valid uses though). They shouldn't become a dumping ground of excess functionality. My understanding is that Ember is all about "paving the cowpaths", and the idea of every app needing to write a dataService or a routingService seems to fly in the face of that. For these types of common problems, there should ideally be an out-of-the-box solution.

I also have a question about your example medical app that perhaps reveals my ignorance of the whole "data down, actions up" approach. Wouldn't the deleteMedication action bubble up just to the medication-list component, and then that component would remove the item from the list, potentially triggering a more generic save up afterwards? Otherwise, don't we lose out on any sense of encapsulation?

@mgenev mgenev mentioned this pull request May 11, 2015
@rwjblue
Copy link
Member

rwjblue commented May 11, 2015

I believe that #50 addresses the main concern of all the manual wiring up of bubbling actions through various layers. You should no longer have to implement an action in a component simply to call this.sendAction('whateverName').

A snippet from #50:

{{! app/index/template.hbs }}
{{my-form submit=(action 'submit')}}


```hbs
{{! app/components/my-form/template.hbs }}
{{my-button on-click=attrs.submit}}
{{! app/components/my-button/template.hbs }}
<button></button>
// app/components/my-button/component.js
export default Ember.Component.extend({
  click: function(){
    this.attrs['on-click']();
    // for backwards compat, you may also this.sendAction();
  }
});

There are many other niceties that are added by #50 (like currying arguments through various layers) without implementing actions.

Here is a tiny demo: http://emberjs.jsbin.com/rwjblue/473/edit?html,js,output.

@rwjblue
Copy link
Member

rwjblue commented May 11, 2015

Thank you all for the awesome discussion here, it definitely informed the decisions/techniques implemented for #50.

@rwjblue rwjblue closed this May 11, 2015
@mgenev
Copy link
Author

mgenev commented May 11, 2015

Awesome, I look forward to all the improvements and examples with #50!

@mgenev
Copy link
Author

mgenev commented May 11, 2015

@rwjblue
doesn't this mean that you still have to manually send actions up?

// app/components/my-button/component.js
export default Ember.Component.extend({
  click: function(){
    this.attrs['on-click']();
    // for backwards compat, you may also this.sendAction();
  }
});

@rwjblue
Copy link
Member

rwjblue commented May 11, 2015

doesn't this mean that you still have to manually send actions up?

I am not sure that I follow you here. The action function is being invoked directly in this case, there is nothing being sent up...

@mgenev
Copy link
Author

mgenev commented May 12, 2015

@MiguelMadero I believe we already have a routing service - the router, and a data service - the store... I don't believe that you can make an app composed with completely isolated (encapsulated) components. Things need to talk to each other.

@mgenev
Copy link
Author

mgenev commented May 12, 2015

@rwjblue

I am not sure that I follow you here. The action function is being invoked directly in this case, there is nothing being sent up...

But it's to the same effect, you still have to redefine your click, and execute the function. The intent is that the button click submits the form, you have to define both the submit and the click and then tie them. You still have repetition.

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

Successfully merging this pull request may close these issues.

10 participants