Make actions use target=this implicitly

packages/ember-glimmer/lib/helpers/action.js

@@ -278,15 +278,14 @@ export class ClosureActionReference extends CachedReference {
     let { named, positional } = this.args;
     let positionalValues = positional.value();
 
-    let target = positionalValues[0];
-    let rawActionRef = positional.at(1);
-    let rawAction = positionalValues[1];
+    let target = named.get('target').value();
+    let rawActionRef = positional.at(0);
+    let rawAction = positionalValues[0];
 
-    // The first two argument slots are reserved.
-    // pos[0] is the context (or `this`)
-    // pos[1] is the action name or function
+    // The first argument slot is reserved.
+    // pos[0] is the action name or function
     // Anything else is an action argument.
-    let actionArgs = positionalValues.slice(2);
+    let actionArgs = positionalValues.slice(1);
 
     // on-change={{action setName}}
     // element-space actions look to "controller" then target. Here we only
@@ -305,11 +304,6 @@ export class ClosureActionReference extends CachedReference {
 
       action = null;
 
-      if (named.has('target')) {
-        // on-change={{action 'setName' target=alternativeComponent}}
-        target = named.get('target').value();
-      }
-
       if (target['actions']) {
         action = target.actions[actionName];
       }

packages/ember-glimmer/lib/modifiers/action.js

@@ -66,14 +66,13 @@ export let ActionHelper = {
 };
 
 export class ActionState {
-  constructor(element, actionId, actionName, actionArgs, namedArgs, positionalArgs, implicitTarget, dom) {
+  constructor(element, actionId, actionName, actionArgs, namedArgs, positionalArgs, dom) {
     this.element = element;
     this.actionId = actionId;
     this.actionName = actionName;
     this.actionArgs = actionArgs;
     this.namedArgs = namedArgs;
     this.positional = positionalArgs;
-    this.implicitTarget = implicitTarget;
     this.dom = dom;
     this.eventName = this.getEventName();
   }
@@ -92,25 +91,12 @@ export class ActionState {
     return result;
   }
 
-  getTarget() {
-    let { implicitTarget, namedArgs } = this;
-    let target;
-
-    if (namedArgs.has('target')) {
-      target = namedArgs.get('target').value();
-    } else {
-      target = implicitTarget.value();
-    }
-
-    return target;
-  }
-
   handler(event) {
     let { actionName, namedArgs } = this;
     let bubbles = namedArgs.get('bubbles');
     let preventDefault = namedArgs.get('preventDefault');
     let allowedKeys = namedArgs.get('allowedKeys');
-    let target = this.getTarget();
+    let target = namedArgs.get('target').value();
 
     if (!isAllowedEvent(event, allowedKeys.value())) {
       return true;
@@ -168,12 +154,10 @@ export class ActionState {
 export default class ActionModifierManager {
   create(element, args, dynamicScope, dom) {
     let { named, positional } = args;
-    let implicitTarget;
     let actionName;
     let actionNameRef;
-    if (positional.length > 1) {
-      implicitTarget = positional.at(0);
-      actionNameRef = positional.at(1);
+    if (positional.length > 0) {
+      actionNameRef = positional.at(0);
 
       if (actionNameRef[INVOKE]) {
         actionName = actionNameRef;
@@ -192,9 +176,9 @@ export default class ActionModifierManager {
     }
 
     let actionArgs = [];
-    // The first two arguments are (1) `this` and (2) the action name.
+    // The first argument is the action name.
     // Everything else is a param.
-    for (let i = 2; i < positional.length; i++) {
+    for (let i = 1; i < positional.length; i++) {
       actionArgs.push(positional.at(i));
     }
 
@@ -206,7 +190,6 @@ export default class ActionModifierManager {
       actionArgs,
       named,
       positional,
-      implicitTarget,
       dom
     );
   }
@@ -223,7 +206,7 @@ export default class ActionModifierManager {
   update(actionState) {
     let { positional } = actionState;
 
-    let actionNameRef = positional.at(1);
+    let actionNameRef = positional.at(0);
 
     if (!actionNameRef[INVOKE]) {
       actionState.actionName = actionNameRef.value();

packages/ember-glimmer/tests/integration/helpers/closure-action-test.js

@@ -777,6 +777,53 @@ moduleFor('Helpers test: closure {{action}}', class extends RenderingTest {
     this.assert.ok(actionCalled, 'action called on otherComponent');
   }
 
+  ['@test closure actions support a target [GH#14469]']() {
+    let innerComponent;
+    let actionCalled = false;
+    let actualName;
+    let expectedName = 'the foobar';
+
+    let InnerComponent = Component.extend({
+      init() {
+        this._super(...arguments);
+        innerComponent = this;
+      },
+      fireAction() {
+        this.attrs.submit();
+      }
+    });
+
+    let OuterComponent = Component.extend({
+      name: 'outer',
+      foobar: {
+        name: 'the foobar',
+        myName() {
+          actionCalled = true;
+          actualName = this.name;
+        }
+      }
+    });
+
+    this.registerComponent('inner-component', {
+      ComponentClass: InnerComponent,
+      template: 'inner'
+    });
+
+    this.registerComponent('outer-component', {
+      ComponentClass: OuterComponent,
+      template: `{{inner-component submit=(action foobar.myName target=foobar)}}`
+    });
+
+    this.render('{{outer-component}}');
+
+    this.runTask(() => {
+      innerComponent.fireAction();
+    });
+
+    this.assert.ok(actionCalled, 'action called on OuterComponent\'s foobar object');
+    this.assert.equal(actualName, expectedName, 'the foobar is the context');
+  }
+
   ['@test value can be used with action over actions']() {
     let newValue = 'yelping yehuda';
 

packages/ember-template-compiler/lib/plugins/transform-action-syntax.js

@@ -1,3 +1,5 @@
+import hashPairForKey from '../system/hash-pair-for-key';
+
 /**
  @module ember
  @submodule ember-glimmer
@@ -15,11 +17,13 @@
   with
 
   ```handlebars
- <button {{action this 'foo'}}>
- <button onblur={{action this 'foo'}}>
- <button onblur={{action this (action this 'foo') 'bar'}}>
+ <button {{action 'foo' target=this}}>
+ <button onblur={{action 'foo' target=this}}>
+ <button onblur={{action (action 'foo' target=this) 'bar' target=this}}>
   ```
 
+  If an action already has a target it is left unmodified.
+
   @private
   @class TransformActionSyntax
 */
@@ -40,17 +44,17 @@ TransformActionSyntax.prototype.transform = function TransformActionSyntax_trans
   traverse(ast, {
     ElementModifierStatement(node) {
       if (isAction(node)) {
-        insertThisAsFirstParam(node, b);
+        ensureTarget(node, b);
       }
     },
     MustacheStatement(node) {
       if (isAction(node)) {
-        insertThisAsFirstParam(node, b);
+        ensureTarget(node, b);
       }
     },
     SubExpression(node) {
       if (isAction(node)) {
-        insertThisAsFirstParam(node, b);
+        ensureTarget(node, b);
       }
     }
   });
@@ -62,6 +66,9 @@ function isAction(node) {
   return node.path.original === 'action';
 }
 
-function insertThisAsFirstParam(node, builders) {
-  node.params.unshift(builders.path('this'));
+function ensureTarget(node, builders) {
+  if (!hashPairForKey(node.hash, 'target')) {
+    let thisTarget = builders.pair('target', builders.path('this'));
+    node.hash.pairs.push(thisTarget);
+  }
 }

packages/ember-template-compiler/lib/system/hash-pair-for-key.js

@@ -0,0 +1,10 @@
+export default function hashPairForKey(hash, key) {
+  for (let i = 0; i < hash.pairs.length; i++) {
+    let pair = hash.pairs[i];
+    if (pair.key === key) {
+      return pair;
+    }
+  }
+
+  return false;
+}