Recommend Arrows instead of @action

[NullVoxPopuli]

Propose Recommending arrow functions instead of the @action decorator.

Rendered

Summary

This pull request is proposing a new RFC.

To succeed, it will need to pass into the Exploring Stage), followed by the Accepted Stage.

A Proposed or Exploring RFC may also move to the Closed Stage if it is withdrawn by the author or if it is rejected by the Ember team. This requires an "FCP to Close" period.

An FCP is required before merging this PR to advance to Accepted.

Upon merging this PR, automation will open a draft PR for this RFC to move to the Ready for Released Stage.

Exploring Stage Description

This stage is entered when the Ember team believes the concept described in the RFC should be pursued, but the RFC may still need some more work, discussion, answers to open questions, and/or a champion before it can move to the next stage.

An RFC is moved into Exploring with consensus of the relevant teams. The relevant team expects to spend time helping to refine the proposal. The RFC remains a PR and will have an Exploring label applied.

An Exploring RFC that is successfully completed can move to Accepted with an FCP is required as in the existing process. It may also be moved to Closed with an FCP.

Accepted Stage Description

To move into the "accepted stage" the RFC must have complete prose and have successfully passed through an "FCP to Accept" period in which the community has weighed in and consensus has been achieved on the direction. The relevant teams believe that the proposal is well-specified and ready for implementation. The RFC has a champion within one of the relevant teams.

If there are unanswered questions, we have outlined them and expect that they will be answered before Ready for Release.

When the RFC is accepted, the PR will be merged, and automation will open a new PR to move the RFC to the Ready for Release stage. That PR should be used to track implementation progress and gain consensus to move to the next stage.

Checklist to move to Exploring

• The team believes the concepts described in the RFC should be pursued.
• The label S-Proposed is removed from the PR and the label S-Exploring is added.
• The Ember team is willing to work on the proposal to get it to Accepted

Checklist to move to Accepted

• This PR has had the Final Comment Period label has been added to start the FCP
• The RFC is announced in #news-and-announcements in the Ember Discord.
• The RFC has complete prose, is well-specified and ready for implementation.
  • All sections of the RFC are filled out.
  • Any unanswered questions are outlined and expected to be answered before Ready for Release.
  • "How we teach this?" is sufficiently filled out.
• The RFC has a champion within one of the relevant teams.
• The RFC has consensus after the FCP period.

[bitxplora]

It might be a challenge, in a class-backed component where you need to access a member of the class, in which this will be required.

[jrjohnson]

I had no idea this would work, love it!

[NullVoxPopuli]

It might be a challenge, in a class-backed component where you need to access a member of the class, in which this will be required.

arrow functions would use the instance's this

I had no idea this would work, love it!

technically it worked years ago! (if you had no legacy code, and were using native classes) 🙈 🎉

[NullVoxPopuli]

A part 3 may be needed on inheritance -- which we should discourage, as misuse of inheritance is often folks mess up OOP. With components, composition is very nice, and with classes, dependency injection is very nice.

Here is what happens with arrow functions and you try to use "the same patterns" as you would with methods:

class A {
  message = "hi";
  greet = () => this.message;
}


class B extends A {
  greet = () => `:::: ${super.greet()}`;
}

let b = new B();  

console.log({
  b: b.greet(),
});

gives the error:

TypeError: (intermediate value).greet is not a function
    at B.greet (<anonymous>:9:35)

[NullVoxPopuli]

This happens because instance fields are not present on the prototype at all.
You can't do this either:

class B extends A {
  greet = () => `:::: ${A.prototype.greet.call(this)}`;
}

prototype.greet is undefined.

[NullVoxPopuli]

If folks want to use inheritance, despite warnings, this RFC doesn't deprecate @action

at some point what @action is doing will do less after classic paradigms are deprecated and removed

[MrChocolatine]

I was about to post about this, that it breaks inheritance.

But if this RFC is --just-- about recommending something while leaving the door open to use @action and class prototype methods, then fine. I don't think we should go against this pattern, even if it means keeping @action around for who knows how long.

[NullVoxPopuli]

yeah, until JS figures out better ergonomics around class-method-this-binding, we should keep @action

[rtablada]

I'm in huge favor of this! Barring a broader TC39 binding decorator for functions arrows are a huge win!

Something to consider is that action has a big learning curve for folks and there are a good number of devs in older apps that have to reason about the @action decorator as well as legacy code where action helpers are used.

There's a lot of legacy load where arrow functions are JUST JS.

[Panman82]

I'm in huge favor of this! Barring a broader TC39 binding decorator for functions arrows are a huge win!

Right! What ever happened to that? I thought it was supposed to be @bound or something like that...

[NullVoxPopuli]

I thought it was supposed to be @bound or something like that...

decorators would first need to be implemented, and it seems browsers are nearly there. Additionally, all build tools support native decorators already.

[Techn1x]

Very much in favour. Have been using arrow functions instead of actions in multiple large production apps for a year or two now without issue. We don't do inheritance in any meaningful capacity.

[Panman82]

The only thing I could find for @bound from TC39 is just an example: https://github.com/tc39/proposal-decorators?tab=readme-ov-file#example-bound

But I also found some interesting discussion about arrow functions vs decorator here: tc39/proposal-class-fields#80

[NullVoxPopuli]

There is certainly some nuance to methods in classes if you want the most performance possible.

Fwiw, I'm also ok with blocking this on waiting on the Glimmer VM to make real methods work (as described in a footnote in this RFC)

e.g.:

class Demo {
  exclaim(greeting) {
    return `${greeting}!!!`;
  }

  <template>
     {{this.exclaim "hi"}}
  </template>
}

would sorta "look like" this once the VM changes are done:

class Demo {
  exclaim(greeting) {
    return `${greeting}!!!`;
  }

  // NOTE: this is fake code, these apis don't exist
  template = glimmerVM(`{{this.exclaim "hi"}}`, this);
}

so in the VM, assuming we have this:

function glimmerVM(template, context) {
  /* implementation omitted */
}

and effectively, today we do:

context.exclaim("hi")

in the vm, but we need to do:

context.exclaim.call(context, "hi") 

and that would allow us to utilize real methods -- and then arrows only matter when we want to pass a function

[joukevandermaas]

I think you should add to the downsides section that this change makes existing online content incompatible with the docs. The new docs with this recommendation will now be using different syntax from existing forum posts, blog posts etc. which will make learning more difficult (why are they using different syntax? which one should I use?)

--

There seems to be some enthusiasm in this thread, but I would consider opportunity cost on a change like this. I don't see why going through all the docs to make this change is more valuable work than literally any other doc improvement. Having worked with a lot of juniors and having taught many people how to use Ember, this has basically never been a problem and it is very easy to explain what to do.

This could have been a good idea maybe when class components were new, but now there is years of existing patterns in the community and there is just no reason to change this IMO.

[NullVoxPopuli]

I think you should add to the downsides section that this change makes existing online content incompatible with the docs.

This is not true. @action isn't going away.

but now there is years of existing patterns in the community

we should not let our baggage hold us back from our dreams <3
(whatever those are!)

and there is just no reason to change this IMO.

the RFC explains the reason 🙃

[joukevandermaas]

I think you should add to the downsides section that this change makes existing online content incompatible with the docs.

This is not true. @action isn't going away.

It is true, at best you will only update the Ember docs but not every old forum post or blog article out there. So for a newcomer there will end up having two different syntaxes on display by default. In my experience people very often look at information besides the official Ember docs while learning. My whole argument is predicated on the fact that @action is not going away.

For an experienced JS developer, the two syntaxes are more or less the same and they can easily understand what's happening in either. My point is that this is not true for beginners, and so this arbitrary change will trip them up. So I'm not really speaking for myself but for beginning JS/Ember developers (which we need to attract to grow the community).

we should not let our baggage hold us back from our dreams <3 (whatever those are!)

I guess I would say I don't agree that it's baggage, I think it's a very clear syntax that explains exactly what is happening and is easy to teach. And I don't think the proposal is a dream either, I think it's harder to read and harder to teach.

For me the motivation from the RFC is completely reversed, it's much easier to type a class method and have my editor automatically add the import than it is to use a weird syntax that always trips up beginners (in my experience).

I'm not arguing for what people want to do in their own code-bases but I think the official docs should not be changed for such a marginal "benefit". In general I think JS community is much to liberal with making this types of changes that only cause churn, just because it looks "cleaner" or is slightly easier to type. So I guess people like me should at least say that on RFCs 😄

Anyway I am just trying to give my perspective here, I think maybe it comes across a bit harsh (? hard to tell sometimes), so apologies if that is the case. I have taught a lot of people JS and Ember over the years and I have learned these lessons from previous attempts to "improve" syntax.

[MrChocolatine]

I'm not arguing for what people want to do in their own code-bases but I think the official docs should not be changed for such a marginal "benefit". In general I think JS community is much to liberal with making this types of changes that only cause churn, just because it looks "cleaner" or is slightly easier to type.

This.

Yes.

[happydude]

As a non-expert, I didn't know this option was available. I tried it on one of my controllers and it worked fine. I ran into a bug recently where my willTransition on a route wasn't getting called, and it was because I forgot to add the @action decorator.

I gave the arrow syntax a try on a willTransition that was working with @action and the arrow syntax doesn't seem to work in that scenario.

[NullVoxPopuli]

I gave the arrow syntax a try on a willTransition that was working with @action and the arrow syntax doesn't seem to work in that scenario.

This tells me that willTransition (and friends, probably), rely on the legacy semantics where actions all exist on an object named actions in the class.

[Techn1x]

A happy middleground could be to use & recommend arrow functions in the docs, but have somewhere that addresses the fact that @action exists and may be used in external examples, but it does essentially the same thing. So if it's seen elsewhere it's no big deal. Users can still search for "action" and find some ember documentation on it explaining it. win win

If we continue to use only @action in docs, with the justification of avoiding official docs being incongruent with existing articles/posts etc then it'll be that way forever, that justification will never change. New posts will be made that continue to use the decorator since that's what the official docs use too. And if posts do use arrow functions, users are left in the dark about why since the docs don't address it.

Rather official docs should be driving the best way to do things. Additionally, historical docs are kept / versioned - historical uses of @action would still be found in (older) official docs.

I find it hard to get behind the reasoning of avoiding this change because of existing internet code examples potentially affecting new users learning ember. I feel this is easily counteracted by the fact there's one less needless emberism to learn. The two newbies on my team are happily refactoring away old @action usages & imports and making new components without it.

All that being said, if there are technical reasons why @action is still going to be needed in common use in certain codebases (like due to inheritance concerns, or actions hash things) that's a different story, and I can understand not wanting to explain in detail when to use one or the other.

[joukevandermaas]

I suppose the point is that there is insufficient reason given in the RFC to cause this kind of churn, and I motivated that by explaining why the churn is harmful. The same reasoning does not apply to every change, but indeed will forever apply to this change given that everything else stays the same.

For example the move from a dedicated actions hash to the @action decorator caused a lot of churn but was a very good change because it was much easier to explain and use. Also if the JS community at large was already using arrow function syntax in classes that would change things as well.

But ignoring the cost of something because the cost is not likely to change, is not a good way to think about it in my opinion.

[ef4]

We're proposing this for acceptance, in recognition of the fact that using arrows is already becoming standard practice among a lot of people who know they work and we want the guides to follow how ember code is really written.

[Eredrim]

I read this RFC and why not. My first opinion was this syntax is not clear enough to distinct actions from other class functions. However, cost benefits might be interesting, in particular for large apps. So I'm neither for nor against, but it should be really good to :

• Add a quick note in updated documentation explaining why decorator is deprecated (in a "zoe says" paragraph for instance)
• Update IDE/Text editor plugins to highlight controller arrow functions as "actions" and (if possible) bind template reference to them, just like with @action decorator
• Create a codemod to upgrade existing apps

I think people (ones they won't read this PR) don't have to only see a warning at build step, they deserve an explanation and keeping all the features they already have.

[ef4]

Add a quick note in updated documentation explaining why decorator is deprecated (in a "zoe says" paragraph for instance)

This RFC does not deprecate @action, right now this is just proposing that documentation teaches people to use the arrows instead. We could discuss whether we should also deprecate @action.

[NullVoxPopuli]

We could discuss whether we should also deprecate @action.

for now, I don't think we should deprecate anything.

A future RFC could deprecate the non-bind behaviors of @action, however

[lupestro]

I seem to have come very late to the party. :)

My only significant concern is that while the @action decorator works with inheritance, a mainstream vanilla JS class mechanism that one might expect to “always just work”, member arrow functions do not because their implementation doesn’t inhabit the prototype. You could have a bunch of people who use inheritance as pure abstraction without regard for what’s under the hood become terribly confused at why their code isn’t working.

When introducing the use of foo: (parms)=>{ implementation; } in the guides, this limitation should be called out as a potential footgun and @action should be recommended explicitly for situations like inheritance and the one or two others that have been called out. I’d like to see that mentioned in the RFC but I want to see it in the guides whether it appears in the RFC or not.

One thing isn’t clear to me - if I have a class with several hundred instances, what do arrow-valued members in the instances cost me in space+time, as compared to a function defined in the prototype with a decorator to bind it up? (I still want to accomplish an Ember app doing something useful in under 50k of parsed JS a year or two from now. At that point, questions like this will start to matter. In the meantime, I’m still waiting with bated breath to see the megabytes boil away from tree-shaking, but I thought it worthwhile to at least ask the question now.)

[NullVoxPopuli]

@action should be recommended explicitly for situations like inheritance and the one or two others that have been called out. I’d like to see that mentioned in the RFC but I want to see it in the guides whether it appears in the RFC or not.

Most recent commit has information on inheritance 🎉

if I have a class with several hundred instances, what do arrow-valued members in the instances cost me in space+time, as compared to a function defined in the prototype with a decorator to bind it up?

It's all the same, memory-wise -- you get a duplicate method per instance (just a matter of where (today, @action stores the duplicates in a weak map)). But I think using arrows would have a (very very) slight perf benefit in hot paths, because @action does a lot and arrows do not.

However, one thing we want to make work, and currently consider a bug (and I believe this is called out earlier in the discussion, is that we want normal methods to just work with the correct this, when invoked from the template.

For example:

class Demo extends Component {
  <template>
    {{this.value}}
    <button {{on 'click' this.increment}}>++</button>
  </template>

  @tracked value = 0;

  increment() {
    this.value++;
  }
}

and this would be memory optimized to only have one copy of increment on every instance.

[MrChocolatine]

Out of curiosity, is recommending arrow functions instead of @action something you would have done earlier? Instead of teaching people to use @action? From what I understand, as of today, we could use arrow functions everywhere without @action, or am I mistaken?

Why did we go with @action in the first place with Ember.js Octane? We could have said to use arrow functions (which we kind of do already, for instance with local Helpers and local Modifiers).
=> I assume part of the answer was to make the transition from the action Helper smoother.

Just asking to enrich my knowledge.

[ef4]

Feedback from RFC review:

• it would be good to first minimize the number of cases where you would even need an arrow
  • method invocation fix in glimmer, so that {{this.method arg}} is automatically this-bound just like javascript invocation
  • similarly, introducing a "closure-like" feature, possibly by adding special behavior to fn, so that capture can be done

[MelSumner]

It's tedious to maintain code that has arrow functions everywhere. I don't really understand why this change and I don't feel like the RFC rationalizes it enough to support it.

[NullVoxPopuli]

It's tedious to maintain code that has arrow functions everywhere.

more than action? if so, I don't agree.

or more than regular methods that should have worked the whole time? If so, I agree

[MelSumner]

@joukevandermaas makes some points that I would just otherwise repeat:

For me the motivation from the RFC is completely reversed, it's much easier to type a class method and have my editor automatically add the import than it is to use a weird syntax that always trips up beginners (in my experience).

I'm not arguing for what people want to do in their own code-bases but I think the official docs should not be changed for such a marginal "benefit". In general I think JS community is much to liberal with making this types of changes that only cause churn, just because it looks "cleaner" or is slightly easier to type. So I guess people like me should at least say that on RFCs 😄

[NullVoxPopuli]

i mean, if anything, we remove action from the docs entirely.
use plain methods.
arrows if you need to bind (should be rareish)

no imports needed.

the benefit to doing this plan (instead of arrows everywhere) is not marginal, is great, esp from a teaching perspective

[ef4]

The motivation here is that we have a divergence between what our docs say and what a lot of experienced ember devs are already doing. I would personally never tell a new ember developer to go learn about importing @action from @ember/object, until they encounter some legacy code that is already using that pattern.

Perhaps what this really means is that we need to more aggressively deprecate @action.

It's tedious to maintain code that has arrow functions everywhere.

I think this is true when there are multiple nested layers of arrow functions and arrow functions being passed around as arguments. But that is not the case for action handlers in components. They can only be one flat namespace on the component.

Syntactically, switching from a method to a class field containing an arrow only adds three characters. When you consider that it also lets you delete the decorator and the import for the decorator, you net less code.

I don't even disagree with people who think the pattern is wacky. But we can blame TC39 for that. When they added class syntax it should have auto-bound the methods. We work in the language we have.

In any case, I still think the status in #1045 (comment) reflects the current consensus next step on this RFC. If we solve those cases first, we may find that the remaining use case for arrows or action has shrunk to zero.