Merge pull request #20653 from NullVoxPopuli/deprecate-action

Implement RFC#1006, Deprecate `(action)` and `{{action}}`

Author: kategengler

packages/@ember/-internals/deprecations/index.ts

@@ -97,6 +97,15 @@ export const DEPRECATIONS = {
     until: '6.0.0',
     url: 'https://deprecations.emberjs.com/v5.x/#toc_deprecate-implicit-route-model',
   }),
+  DEPRECATE_TEMPLATE_ACTION: deprecation({
+    id: 'template-action',
+    url: 'https://deprecations.emberjs.com/id/template-action',
+    until: '6.0.0',
+    for: 'ember-source',
+    since: {
+      available: '5.9.0',
+    },
+  }),
 };
 
 export function deprecateUntil(message: string, deprecation: DeprecationObject) {

packages/@ember/-internals/glimmer/lib/helpers/action.ts

@@ -4,6 +4,7 @@
 import { get } from '@ember/-internals/metal';
 import type { AnyFn } from '@ember/-internals/utility-types';
 import { assert } from '@ember/debug';
+import { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations';
 import { flaggedInstrument } from '@ember/instrumentation';
 import { join } from '@ember/runloop';
 import { DEBUG } from '@glimmer/env';
@@ -278,6 +279,10 @@ export const ACTIONS = new WeakSet();
   @public
 */
 export default internalHelper((args: CapturedArguments): Reference<Function> => {
+  deprecateUntil(
+    `Usage of the \`(action)\` helper is deprecated. Migrate to native functions and function invocation.`,
+    DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION
+  );
   let { named, positional } = args;
   // The first two argument slots are reserved.
   // pos[0] is the context (or `this`)

packages/@ember/-internals/glimmer/lib/modifiers/action.ts

@@ -1,4 +1,5 @@
 import type { InternalOwner } from '@ember/-internals/owner';
+import { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations';
 import { uuid } from '@ember/-internals/utils';
 import { ActionManager, EventDispatcher, isSimpleClick } from '@ember/-internals/views';
 import { assert } from '@ember/debug';
@@ -204,6 +205,10 @@ class ActionModifierManager implements InternalModifierManager<ActionState, obje
   }
 
   install(actionState: ActionState): void {
+    deprecateUntil(
+      `Usage of the \`{{action}}\` modifier is deprecated. Migrate to native functions and function invocation.`,
+      DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION
+    );
     let { element, actionId, positional } = actionState;
 
     let actionName;

packages/@ember/-internals/glimmer/tests/integration/application/actions-test.js

@@ -1,9 +1,16 @@
-import { moduleFor, ApplicationTestCase, RenderingTestCase, runTask } from 'internal-test-helpers';
+import {
+  testUnless,
+  moduleFor,
+  ApplicationTestCase,
+  RenderingTestCase,
+  runTask,
+} from 'internal-test-helpers';
 
 import Controller from '@ember/controller';
 import { getDebugFunction, setDebugFunction } from '@ember/debug';
 
 import { Component } from '../../utils/helpers';
+import { DEPRECATIONS } from '../../../../deprecations';
 
 const originalDebug = getDebugFunction('debug');
 const noop = function () {};
@@ -14,16 +21,21 @@ moduleFor(
     constructor() {
       setDebugFunction('debug', noop);
       super(...arguments);
+
+      expectDeprecation(
+        /Usage of the `\{\{action\}\}` modifier is deprecated./,
+        DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
+      );
     }
 
     teardown() {
       setDebugFunction('debug', originalDebug);
     }
 
-    ['@test actions in top level template application template target application controller'](
-      assert
-    ) {
-      assert.expect(1);
+    [`${testUnless(
+      DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
+    )} actions in top level template application template target application controller`](assert) {
+      assert.expect(2);
 
       this.add(
         'controller:application',
@@ -46,8 +58,10 @@ moduleFor(
       });
     }
 
-    ['@test actions in nested outlet template target their controller'](assert) {
-      assert.expect(1);
+    [`${testUnless(
+      DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
+    )} actions in nested outlet template target their controller`](assert) {
+      assert.expect(2);
 
       this.add(
         'controller:application',

packages/@ember/-internals/glimmer/tests/integration/components/contextual-components-test.js

@@ -2,6 +2,7 @@ import { DEBUG } from '@glimmer/env';
 import { moduleFor, RenderingTestCase, applyMixins, strip, runTask } from 'internal-test-helpers';
 
 import { isEmpty } from '@ember/utils';
+import { action } from '@ember/object';
 import { A as emberA } from '@ember/array';
 
 import { Component } from '../../utils/helpers';
@@ -773,18 +774,16 @@ moduleFor(
 
       this.registerComponent('my-action-component', {
         ComponentClass: Component.extend({
-          actions: {
-            changeValue() {
-              this.incrementProperty('myProp');
-            },
-          },
+          changeValue: action(function () {
+            this.incrementProperty('myProp');
+          }),
         }),
         template: strip`
         {{#my-component my-attr=this.myProp as |api|}}
           {{api.my-nested-component}}
         {{/my-component}}
         <br>
-        <button onclick={{action 'changeValue'}}>Change value</button>`,
+        <button onclick={{this.changeValue}}>Change value</button>`,
       });
 
       this.render('{{my-action-component myProp=this.model.myProp}}', {
@@ -839,13 +838,12 @@ moduleFor(
     ['@test parameters in a contextual component are mutable when value is a param'](assert) {
       // This checks that a `(mut)` is added to parameters and attributes to
       // contextual components when it is a param.
-
       this.registerComponent('change-button', {
         ComponentClass: Component.extend().reopenClass({
           positionalParams: ['val'],
         }),
         template: strip`
-        <button {{action (action (mut this.val) 10)}} class="my-button">
+        <button {{on "click" (fn (mut this.val) 10)}} class="my-button">
           Change to 10
         </button>`,
       });
@@ -892,15 +890,13 @@ moduleFor(
       this.registerComponent('outer-component', {
         ComponentClass: Component.extend({
           message: 'hello',
-          actions: {
-            change() {
-              this.set('message', 'goodbye');
-            },
-          },
+          change: action(function () {
+            this.set('message', 'goodbye');
+          }),
         }),
         template: strip`
         message: {{this.message}}{{inner-component message=this.message}}
-        <button onclick={{action "change"}} />`,
+        <button onclick={{this.change}} />`,
       });
 
       this.registerComponent('inner-component', {
@@ -1447,7 +1443,7 @@ class MutableParamTestGenerator {
             positionalParams: ['val'],
           }),
           template: strip`
-          <button {{action (action (mut this.val) 10)}} class="my-button">
+          <button {{on "click" (fn (mut this.val) 10)}} class="my-button">
             Change to 10
           </button>`,
         });

packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js

@@ -7,8 +7,10 @@ import {
   equalsElement,
   runTask,
   runLoopSettled,
+  testUnless,
 } from 'internal-test-helpers';
 
+import { action } from '@ember/object';
 import { run } from '@ember/runloop';
 import { DEBUG } from '@glimmer/env';
 import { tracked } from '@ember/-internals/metal';
@@ -20,6 +22,7 @@ import { A as emberA } from '@ember/array';
 
 import { Component, compile, htmlSafe } from '../../utils/helpers';
 import { backtrackingMessageFor } from '../../utils/debug-stack';
+import { DEPRECATIONS } from '../../../../deprecations';
 
 moduleFor(
   'Components test: curly components',
@@ -1441,25 +1444,23 @@ moduleFor(
             componentInstance = this;
           },
 
-          actions: {
-            click() {
-              let currentCounter = this.get('counter');
+          myClick: action(function () {
+            let currentCounter = this.get('counter');
 
-              assert.equal(currentCounter, 0, 'the current `counter` value is correct');
+            assert.equal(currentCounter, 0, 'the current `counter` value is correct');
 
-              let newCounter = currentCounter + 1;
-              this.set('counter', newCounter);
+            let newCounter = currentCounter + 1;
+            this.set('counter', newCounter);
 
-              assert.equal(
-                this.get('counter'),
-                newCounter,
-                "getting the newly set `counter` property works; it's equal to the value we just set and not `undefined`"
-              );
-            },
-          },
+            assert.equal(
+              this.get('counter'),
+              newCounter,
+              "getting the newly set `counter` property works; it's equal to the value we just set and not `undefined`"
+            );
+          }),
         }),
         template: `
-          <button {{action "click"}}>foobar</button>
+          <button {{on "click" this.myClick}}>foobar</button>
         `,
       });
 
@@ -3146,9 +3147,16 @@ moduleFor(
       runTask(() => set(this.context, 'foo', 5));
     }
 
-    ['@test returning `true` from an action does not bubble if `target` is not specified (GH#14275)'](
+    [`${testUnless(
+      DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
+    )} returning \`true\` from an action does not bubble if \`target\` is not specified (GH#14275)`](
       assert
     ) {
+      expectDeprecation(
+        /Usage of the `\{\{action\}\}` modifier is deprecated./,
+        DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
+      );
+
       this.registerComponent('display-toggle', {
         ComponentClass: Component.extend({
           actions: {
@@ -3173,8 +3181,15 @@ moduleFor(
       runTask(() => this.$('button').click());
     }
 
-    ['@test returning `true` from an action bubbles to the `target` if specified'](assert) {
-      assert.expect(4);
+    [`${testUnless(
+      DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
+    )} returning \`true\` from an action bubbles to the \`target\` if specified`](assert) {
+      assert.expect(5);
+
+      expectDeprecation(
+        /Usage of the `\{\{action\}\}` modifier is deprecated./,
+        DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
+      );
 
       this.registerComponent('display-toggle', {
         ComponentClass: Component.extend({

packages/@ember/-internals/glimmer/tests/integration/components/dynamic-components-test.js

@@ -1,10 +1,11 @@
 import { DEBUG } from '@glimmer/env';
-import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers';
+import { moduleFor, RenderingTestCase, strip, runTask, testUnless } from 'internal-test-helpers';
 
 import { set, computed } from '@ember/object';
 
 import { Component } from '../../utils/helpers';
 import { backtrackingMessageFor } from '../../utils/debug-stack';
+import { DEPRECATIONS } from '../../../../deprecations';
 
 moduleFor(
   'Components test: dynamic components',
@@ -429,7 +430,9 @@ moduleFor(
       this.assertText('foo-bar Caracas Caracas arepas!');
     }
 
-    ['@test component helper with actions'](assert) {
+    [`${testUnless(
+      DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
+    )} component helper with actions`](assert) {
       this.registerComponent('inner-component', {
         template: 'inner-component {{yield}}',
         ComponentClass: Component.extend({
@@ -449,6 +452,11 @@ moduleFor(
         }),
       });
 
+      expectDeprecation(
+        /Usage of the `\(action\)` helper is deprecated./,
+        DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
+      );
+
       let actionTriggered = 0;
       this.registerComponent('outer-component', {
         template:

packages/@ember/-internals/glimmer/tests/integration/components/input-angle-test.js

@@ -1,5 +1,5 @@
 import { moduleFor, RenderingTestCase, runDestroy, runTask } from 'internal-test-helpers';
-import { set } from '@ember/object';
+import { action, set } from '@ember/object';
 
 class InputRenderingTest extends RenderingTestCase {
   $input() {
@@ -280,18 +280,16 @@ moduleFor(
       // this.assertSelectionRange(8, 8); //NOTE: this fails in IE, the range is 0 -> 0 (TEST_SUITE=sauce)
     }
 
-    ['@test sends an action with `<Input @enter={{action "foo"}} />` when <enter> is pressed'](
+    [`@test sends an action with \`<Input @enter={{this.foo}} />\` when <enter> is pressed`](
       assert
     ) {
       assert.expect(2);
 
-      this.render(`<Input @enter={{action 'foo'}} />`, {
-        actions: {
-          foo(value, event) {
-            assert.ok(true, 'action was triggered');
-            assert.ok(event instanceof Event, 'Native event was passed');
-          },
-        },
+      this.render(`<Input @enter={{this.foo}} />`, {
+        foo: action(function (value, event) {
+          assert.ok(true, 'action was triggered');
+          assert.ok(event instanceof Event, 'Native event was passed');
+        }),
       });
 
       this.triggerEvent('keyup', {
@@ -302,12 +300,10 @@ moduleFor(
     ['@test sends `insert-newline` when <enter> is pressed'](assert) {
       assert.expect(2);
 
-      this.render(`<Input @insert-newline={{action 'foo'}} />`, {
-        actions: {
-          foo(value, event) {
-            assert.ok(true, 'action was triggered');
-            assert.ok(event instanceof Event, 'Native event was passed');
-          },
+      this.render(`<Input @insert-newline={{this.foo}} />`, {
+        foo(value, event) {
+          assert.ok(true, 'action was triggered');
+          assert.ok(event instanceof Event, 'Native event was passed');
         },
       });
 
@@ -316,18 +312,16 @@ moduleFor(
       });
     }
 
-    ['@test sends an action with `<Input @escape-press={{action "foo"}} />` when <escape> is pressed'](
+    ['@test sends an action with `<Input @escape-press={{this.foo}} />` when <escape> is pressed'](
       assert
     ) {
       assert.expect(2);
 
-      this.render(`<Input @escape-press={{action 'foo'}} />`, {
-        actions: {
-          foo(value, event) {
-            assert.ok(true, 'action was triggered');
-            assert.ok(event instanceof Event, 'Native event was passed');
-          },
-        },
+      this.render(`<Input @escape-press={{this.foo}} />`, {
+        foo: action(function (value, event) {
+          assert.ok(true, 'action was triggered');
+          assert.ok(event instanceof Event, 'Native event was passed');
+        }),
       });
 
       this.triggerEvent('keyup', { key: 'Escape' });