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

Intent to RFC: Deprecate {{action}} #537

Closed
pzuraq opened this issue Sep 5, 2019 · 7 comments
Closed

Intent to RFC: Deprecate {{action}} #537

pzuraq opened this issue Sep 5, 2019 · 7 comments
Labels
Seeking Co-author T-deprecation T-framework RFCs that impact the ember.js library

Comments

@pzuraq
Copy link
Contributor

pzuraq commented Sep 5, 2019

  • Proposed Target: Ember v5.0.0
  • Alternative: {{on}} modifier and {{fn}} helper

{{action}} has been a valuable helper/modifier for quite some time, but has a variety of shortcomings:

  1. It uses a special template transform in order to have access to the this context of a template and bind it, something that no other modifier/helper can do.
  2. It uses the global event handler/event delegation to send events, which is a fairly large amount of code, and not leveraging the standardized event handling of modern browsers.
  3. It is not ergonomic for adding event listeners for event types other than the click event.
  4. It handles both adding event listeners and binding values to functions, mixing concerns and making it more difficult to teach.
  5. It doesn't provide a way to add options for events, such as passive and once, that can be passed to addEventListener.

The {{on}} and {{fn}} helpers address all of these concerns, and provide a much more minimal implementation in doing so that should be easier to maintain, and cost less in terms of bundle size overall.

Migration Path

Migrating to the {{on}} modifier should be pretty straightforward in general. The one tricky portion at the moment is an ability the {{action}} modifier had to pick a value from events directly:

<select {{on "change" (action (mut this.selectedValue) value="target.value")}}>
  {{#each this.options as |opt|}}
    <option value={{opt.value}} selected={{eq this.selectedValue opt.value}}>{{opt.label}}</option>
  {{/each}}
</select>

The standard solution here is to have users make a custom action that handles the event. However, this is a common use case, and there likely are more ergonomic patterns out there that we can explore as a community, such as ember-set-helper. For the purposes of deprecating action, this RFC shouldn't block on figuring out the details of these patterns.

Deprecation Timeline

Converting to {{on}} and {{fn}} is not automatable, so it will probably take some time for us to convert as a community. Given how prevalent the {{action}} helper/modifier is, extending the deprecation timeline to v5 makes the most sense at this time.

@buschtoens
Copy link
Contributor

It uses the global event handler/event delegation to send events, which is a fairly large amount of code, and not leveraging the standardized event handling of modern browsers.

Does this imply that the Event Delegation system will be removed in the same scope?

@pzuraq
Copy link
Contributor Author

pzuraq commented Sep 5, 2019

I believe the event delegation system is also tied to deprecating classic components, since their event handler hooks also use it (that’s another upcoming pre-RFC)

@mehulkar
Copy link
Contributor

Why 5.0? Seems like a key breaking change that could be part of 4.0! Is there some threshold of removals that has already been hit for the next major version?

@pzuraq pzuraq changed the title Pre-RFC: Deprecate {{action}} Seeking Co-author: Deprecate {{action}} Oct 8, 2019
@pzuraq pzuraq changed the title Seeking Co-author: Deprecate {{action}} Intent to RFC: Deprecate {{action}} Oct 8, 2019
@pzuraq pzuraq added T-deprecation T-framework RFCs that impact the ember.js library Seeking Co-author labels Oct 8, 2019
@locks locks added this to Deprecations in Deprecation Candidates Oct 9, 2019
@locks locks moved this from Deprecations to Reviewed in Deprecation Candidates Dec 7, 2020
@wagenet wagenet closed this as completed Jul 22, 2022
Deprecation Candidates automation moved this from Reviewed to Finished Jul 22, 2022
@wagenet wagenet reopened this Jul 23, 2022
Deprecation Candidates automation moved this from Finished to In Progress Jul 23, 2022
@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

This is high priority. We should fast-track an RFC draft for this.

@wagenet
Copy link
Member

wagenet commented Jul 29, 2022

I believe this falls under the umbrella of #832. I'm going to close this issue to push any discussion there. Despite being closed, we will certainly use this issue for reference and to inform the future RFC. Thank you all for the discussion!

@wagenet wagenet closed this as completed Jul 29, 2022
Deprecation Candidates automation moved this from In Progress to Finished Jul 29, 2022
@ptgamr
Copy link

ptgamr commented Sep 12, 2022

Hi @wagenet ,

Just a thing I realized when migrating away from {{action}} in our code base. There is a nice feature that if the action isn't defined in the controller, or defined with a wrong name, this helper actually spit out an error that is easy to detect what is missing. (early detection of coding issue)

Passing for example <MyComponent onUpdate={{this.handleUpdate}} />, if the action is missing, then the component would just receive an "undefined" props, no error at all - and it's can only be detected at run time when the time user do the action.

The early error detection comes in handy when refactoring code, renaming action etc.. is there anyway that we can retain this behavior ?

@wagenet
Copy link
Member

wagenet commented Sep 12, 2022

@ptgamr The undefined behavior you're referencing is in line with how JS itself works. Switching to TypeScript (Glint for Ember templates) is generally the preferred solution to avoid these situations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Seeking Co-author T-deprecation T-framework RFCs that impact the ember.js library
Projects
No open projects
Development

No branches or pull requests

5 participants