-
-
Notifications
You must be signed in to change notification settings - Fork 405
Deprecate the (action) template helper and modifier #1006
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
Changes from all commits
70bc8ca
e35074c
8b8c1e4
76f1583
e133d34
2a73a6f
02b44d4
7f0b3ea
4a5a73e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,272 @@ | ||
| --- | ||
| stage: accepted | ||
| start-date: 2024-02-13T00:00:00.000Z | ||
| release-date: | ||
| release-versions: | ||
| teams: # delete teams that aren't relevant | ||
| - cli | ||
| - data | ||
| - framework | ||
| - learning | ||
| - steering | ||
| - typescript | ||
| prs: | ||
| accepted: https://github.com/emberjs/rfcs/pull/1006 | ||
| project-link: | ||
| --- | ||
|
|
||
| <!--- | ||
| Directions for above: | ||
|
|
||
| stage: Leave as is | ||
| start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z | ||
| release-date: Leave as is | ||
| release-versions: Leave as is | ||
| teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies | ||
| prs: | ||
| accepted: Fill this in with the URL for the Proposal RFC PR | ||
| project-link: Leave as is | ||
| --> | ||
|
|
||
| # Deprecate `(action)` template helper and `{{action}}` modifier. | ||
|
|
||
| ## Summary | ||
|
|
||
| The `(action)` template helper and `{{action}}` modifier was common pre-Octane. Now that we have native classes and the `{{on}}` modifier, we no longer need to use `(action)` or `{{action}}` | ||
|
|
||
| ## Motivation | ||
|
|
||
| Remove legacy code with confusing semantics. | ||
|
|
||
| This is a part of _[Deprecating Ember Classic (pre-Octane)](https://github.com/emberjs/rfcs/issues/832)_. | ||
|
|
||
| ## Transition Path | ||
|
|
||
| This was written in the [Octave vs Classic cheatsheet](https://ember-learn.github.io/ember-octane-vs-classic-cheat-sheet/#component-properties__ddau) | ||
|
|
||
| <details><summary>that content here</summary> | ||
|
|
||
| ### Before (pre-Octane) | ||
|
|
||
| ```js | ||
| // parent-component.js | ||
| import Component from '@ember/component'; | ||
|
|
||
| export default Component.extend({ | ||
| count: 0 | ||
| }); | ||
|
|
||
| ``` | ||
| ```hbs | ||
| {{!-- parent-component.hbs --}} | ||
| {{child-component count=count}} | ||
| Count: {{this.count}} | ||
|
|
||
| ``` | ||
| ```js | ||
| // child-component.js | ||
| import Component from '@ember/component'; | ||
|
|
||
| export default Component.extend({ | ||
| actions: { | ||
| plusOne() { | ||
| this.set('count', this.get('count') + 1); | ||
| } | ||
| } | ||
| }); | ||
| ``` | ||
| ```hbs | ||
| {{!-- child-component.hbs --}} | ||
| <button type="button" {{action "plusOne"}}> | ||
| Click Me | ||
| </button> | ||
| ``` | ||
|
|
||
| ### After (post-Octane) | ||
| ```js | ||
| // parent-component.js | ||
| import Component from '@glimmer/component'; | ||
| import { tracked } from '@glimmer/tracking'; | ||
| import { action } from '@ember/object'; | ||
|
|
||
| export default class ParentComponent extends Component { | ||
| @tracked count = 0; | ||
|
|
||
| @action plusOne() { | ||
| this.count++; | ||
| } | ||
| } | ||
|
|
||
| ``` | ||
| ```hbs | ||
| {{!-- parent-component.hbs --}} | ||
| <ChildComponent @plusOne={{this.plusOne}} /> | ||
| Count: {{this.count}} | ||
|
|
||
| ``` | ||
| ```hbs | ||
| {{!-- child-component.hbs --}} | ||
| <button type="button" {{on "click" @plusOne}}> | ||
| Click Me | ||
| </button> | ||
|
|
||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| But what we could put in the deprecation app: | ||
|
|
||
| ### Scenario: `action` is passed a string | ||
|
|
||
| Before: | ||
| ```hbs | ||
| <button type="button" {{action "plusOne"}}> | ||
| Click Me | ||
| </button> | ||
| ``` | ||
|
|
||
| After | ||
|
|
||
| ```hbs | ||
| <button type="button" {{on 'click' this.plusOne}}> | ||
| Click Me | ||
| </button> | ||
| ``` | ||
| or, if `plusOne` is passed in as an argument | ||
| ```hbs | ||
| <button type="button" {{on 'click' @plusOne}}> | ||
| Click Me | ||
| </button> | ||
| ``` | ||
|
|
||
| If the `plusOne` action is in an actions object, it needs to move out: | ||
|
|
||
| Before: | ||
| ```js | ||
| import Component from '@glimmer/component'; | ||
|
|
||
| export default class Demo extends Component { | ||
| actions = { | ||
| plusOne() { | ||
| /* ... */ | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
| or | ||
| ```js | ||
| import Component from '@ember/component'; | ||
|
|
||
| export default class Demo extends Component { | ||
| actions = { | ||
| plusOne() { | ||
| /* ... */ | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
| or | ||
| ```js | ||
| import Component from '@ember/component'; | ||
|
|
||
| export default Component.extend({ | ||
| actions: { | ||
| plusOne() { | ||
| /* ... */ | ||
| } | ||
| } | ||
| }) | ||
| ``` | ||
|
Comment on lines
+144
to
+179
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like we should have independent "after" examples for each of these before cases. The way this is written right now suggests that you might need to upgrade to glimmer class-based components to get around this deprecation, which isn't technically true (even if encouraged 😂)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good, but this sounds like a detail for the deprecation guide PR 🎉 https://github.com/emberjs/rfcs/blob/master/deprecation-template.md#transition-path states that:
So since whether or not to choose |
||
|
|
||
| After: | ||
| ```js | ||
| import Component from '@glimmer/component'; | ||
| import { action } from '@ember/object'; | ||
|
|
||
| export default class Demo extends Component { | ||
| @action | ||
| plusOne() { | ||
| /* ... */ | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Note that `@action` is completely different from `(action)` or `{{action}}` (and is partly a motivator for deprecating `(action)` and `{{action}}`, to reduce ambiguity). | ||
|
|
||
| `@action` is binds the `this` on the method to the instance of the class. | ||
|
|
||
| ### Scenario: `action` is passed a function reference | ||
|
|
||
| Before: | ||
| ```hbs | ||
| <SomeComponent @update={{action this.plusOne}} /> | ||
| ``` | ||
|
|
||
| After | ||
|
|
||
| ```hbs | ||
| <SomeComponent @update={{this.plusOne}} /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may need to note that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. action decorator is only needed for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've mentioned this once during the first scenario -- do you think it's enough? |
||
| ``` | ||
|
|
||
| ### Scenario: `action` is passed parameters | ||
|
|
||
| Before: | ||
| ```hbs | ||
| <SomeComponent @update={{action this.plus 1}} /> | ||
| ``` | ||
|
|
||
| After: | ||
| ```hbs | ||
| <SomeComponent @update={{fn this.plus 1}} /> | ||
| ``` | ||
|
|
||
| ### Scenario: `action` is used with `mut` | ||
|
|
||
| Before: | ||
| ```hbs | ||
| <SomeComponent @update={{action (mut @value.property)}} /> | ||
| ``` | ||
| After: | ||
| ```js | ||
| // parent.js | ||
| import Component from '@glimmer/component'; | ||
| import { action } from '@ember/object'; | ||
|
|
||
| export default class SomeComponent extends Component { | ||
| @action | ||
| handleUpdate(value) { | ||
| this.args.property = value; | ||
| } | ||
| } | ||
| ``` | ||
| ```hbs | ||
| {{! parent.hbs }} | ||
| <SomeComponent @update={{this.handleUpdate}} /> | ||
| ``` | ||
NullVoxPopuli marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Related, [Combining function arguments with action functions](https://guides.emberjs.com/release/components/component-state-and-actions/#toc_combining-arguments-and-actions) | ||
|
|
||
| ### Related: `send` | ||
|
|
||
| When removing `(action)` or `{{action}}` with a _string_ name, you'll also need to verify that there are no [`send`](https://api.emberjs.com/ember/5.6/classes/Component/methods/send?anchor=send) calls with that same string. | ||
|
|
||
| ## How We Teach This | ||
|
|
||
| The guides already cover how to invoke functions in the modern way. | ||
|
|
||
| Remove: https://api.emberjs.com/ember/5.6/classes/Ember.Templates.helpers/methods/action?anchor=action | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| Older code will stop working once the deprecated code is removed. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| - adding an import so folks can keep using action in gjs. | ||
| I don't think we should do this because we want to clean up antiquated patterns, rather than encourage their continued existence. | ||
|
|
||
| ## Unresolved questions | ||
|
|
||
| - Could there be a codemod? | ||
| _Potentially_ for action usage that references `this.properties`. For string actions, it's impossible. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And at minima the codemod would be able to emit warnings in that case ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea |
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.