RFC to deprecate component#sendAction

[cibernox]

Rendered

[simonihmig]

Totally in favor of this, as we have discussed before! 👍

Maybe for some more context, it should be mentioned that not only the pure sendAction() method is deprecated, but essentially alongside it also the underlying concept of "bubbling string actions", which the use of sendAction() was required for?

Which is IMHO way to magic, and an incompatible way to solve the same problem closure actions do (e.g. no return values).

[miguelcobain]

Should we document what the alternative sendAction is?

[Gaurav0]

Not sure how to send an action from a controller to a route or from a route to a higher route without this.

[cibernox]

@miguelcobain I'm not sure if the RFC should contain an in depth explanation of closure actions, but I wrote a blog post about it a while ago.

[cibernox]

@Gaurav0 I think you're conflating send with sendAction. The latter is only available in components, and it's the one being replaced by closure actions. send it's used to call actions on controllers and routes, and I don't advocate its removal (yet)

[miguelcobain]

@cibernox I just thought it would be clearer if the RFC had a "before and after" example, for the sake of completeness.
Could be useful for people that want to migrate their code, writing codemods or even just for understanding what this deprecation is about in a practical sense.

[josemarluedke]

I'm very much in favor to this deprecation. I also think that would be beneficial to have some examples of migrating off sendAction.

[Gaurav0]

@cibernox Yes, you are correct, of course. 😊

[mydea]

I am very much in favor of this, but I would suggest to maybe think about including the route-action helper from https://github.com/DockYard/ember-route-action-helper by default? As without it, you cannot use bubbling actions on routes anymore, which we do use quite often and find pretty helpful. Otherwise, we'd need to move all our actions to the controllers, which is not always desirable for us.

[miguelcobain]

Just wanted to bring up the question of optional actions. sendAction calls an action if present. It's useful in multiple scenarios.

So that means that:

this.sendAction('salute');

is not equivalent to

this.salute();

because this.salute(); will error if salute is not passed in as a component argument, while this.sendAction('salute'); won't.

Will we recommend users to go with the following for these cases?

if (this.salute) {
  this.salute();
}

I believe this would be the code that better represents what this.sendAction('salute'); does.
It also assumes #281 RFC, if I'm not mistaken.

[jasonbelmonti]

Just wanted to bring up the question of optional actions. sendAction calls an action if present. It's useful in multiple scenarios.

Agreed.

[simonihmig]

Will we recommend users to go with the following for these cases?

An alternative would be to define salute() {} as a noop function on the component, so you can remove the guard. That would be defined on the component's prototype (not on each instance), so should not be very wasteful (see also the Ember.K deprecation RFC). This also provides a more obvious place to add docs for that action.

[cibernox]

Just wanted to bring up the question of optional actions. sendAction calls an action if present. It's useful in multiple scenarios.

@miguelcobain I added the changes on the examples to clarify that.
However I do think that the approach taken by sendAction of calling the action only if it exists may be counterproductive when it comes to code clarity. I prefer to guard with an if or to define an empty function, as it makes very clear wheter the action is optional or mandatory. I like that being more explicit.

[miguelcobain]

@cibernox I agree 100%. This RFC definitely fits in the trend of making ember "less magical".

[jasonbelmonti]

@simonihmig @miguelcobain @cibernox: While I agree that sendAction is "magical" and makes code less clear, sendAction is an attempt at addressing some important ideas:

1. Easily invoking optional (potentially undefined) actions
2. Preventing components from being tightly coupled to their invocation context

Consider this antipattern:

if (this.salute) {
  // by relying upon the return value of `this.salute`
  const result = this.salute();
  // this component implementation is tightly coupled to the implementation of `salute`
  this._postProcessSalute(result):
}

In the case of sendAction, the inability to play with returned values from the passed action is probably a good thing. It helps encourage DDAU patterns that delegate action handling up the component hierarchy.

[simonihmig]

In the case of sendAction, the inability to play with returned values from the passed action is probably a good thing.

I wouldn't agree here in general. Certainly you can always mess up your design/architecture if you are not careful enough, as you have shown in your example. But there are cases where a (simple) return value is useful IMO. Here just two examples where I used this in ember-bootstrap:

• return false to prevent some default behavior: in a modal I call the onHide action when the user wants to close it (close button is clicked or ESC is pressed), and by default (99.9% of the use cases) it is closed automatically. But if the user has a need to keep it open and instead trigger some other logic or explicitly close it later, he can return false to opt out of the default behavior.
• return a Promise: a button calls onClick, and if a Promise is returned, it can go into some kind of loading state while the Promise is pending (without actually caring about the fulfilled value).

Here the return values are completely optional, and if given only simple values are expected according to the components specification, so do not necessarily create any tight coupling. So the possibility to return a value seems useful to me, and AFAIR was one of the motivations to introduce closure actions back then.

[cibernox]

In the case of sendAction, the inability to play with returned values from the passed action is probably a good thing.

I want to add to what @simonihmig said that having return values when you invoke a received function is not a violation of the DDAU principle. Functions are as good data as any other, so they can be passed down, and it's perfectly normal to expect functions to have return values.

[knownasilya]

Yeah it enables you to work very well with async code when an action returns a promise. It opens up plenty of creative ways to use actions.

[rwjblue]

We discussed this in today’s core team meeting, and we were all enthusiastic about moving forward here.

We are moving this into the final comment period and will merge the RFC (which enables implementation to begin) in about a week (barring any new issues / concerns).

[akashdsouza]

For actions in route, if I didn't want to write a dummy action with send() to simply bubble it up, would doing something like this be the right solution?

// application/template.hbs
{{my-component editUser=(action send 'editUser')}}

// my-component/component.js
import Ember from 'ember';
export default Ember.Component.extend({
  actions: {
     editUser() {
        this.editUser();
     }
  }
});

[Caltor]

@akashdsouza I think you need to have this.get ('editUser')(); in your action.

[knownasilya]

@Caltor you don't need .get for computed properties any more in 3.x (0 or 1 not sure). Although you do for models since they use a proxy.

[Caltor]

@knownasilya ah sorry we're still on 2.18.

[JackEllis]

I use it a few times in a big codebase I work on but I see no problem with insisting on closure actions. And then this (https://github.com/DockYard/ember-route-action-helper) also exists

[jrock2004]

I just went through an upgrade from 2.4 to 1.18 and had to change alot of stuff to closure actions. https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/closure-actions.md has a good guide of the old bad way to the new closure action way.

[rwjblue]

We discussed this at todays core team meeting, and are in favor of moving forward.

Thanks to everyone who participated in this RFC!

[ohhorob]

Rendered

The link is broken; Is there another reference to the content of this RFC?

[pangratz]

@ohhorob All RFCs are published on https://emberjs.github.io/rfcs, this one is at https://emberjs.github.io/rfcs/0335-deprecate-send-action.html