From 3b85907bdb485a40536a3a58c68f243010bea9ad Mon Sep 17 00:00:00 2001
From: Zack Mulgrew <zack@zackthehuman.com>
Date: Sat, 15 Oct 2016 22:53:19 -0700
Subject: [PATCH 1/3] Ensure a target param for actions with implicit this

---
 packages/ember-glimmer/lib/helpers/action.js  |  7 +----
 .../lib/plugins/transform-action-syntax.js    | 29 +++++++++++++++++--
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/packages/ember-glimmer/lib/helpers/action.js b/packages/ember-glimmer/lib/helpers/action.js
index a9cfc712614..33ba0f100d2 100644
--- a/packages/ember-glimmer/lib/helpers/action.js
+++ b/packages/ember-glimmer/lib/helpers/action.js
@@ -278,7 +278,7 @@ export class ClosureActionReference extends CachedReference {
     let { named, positional } = this.args;
     let positionalValues = positional.value();
 
-    let target = positionalValues[0];
+    let target = named.get('target').value();
     let rawActionRef = positional.at(1);
     let rawAction = positionalValues[1];
 
@@ -305,11 +305,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];
       }
diff --git a/packages/ember-template-compiler/lib/plugins/transform-action-syntax.js b/packages/ember-template-compiler/lib/plugins/transform-action-syntax.js
index 2a26a0c14d4..87124b7dcd8 100644
--- a/packages/ember-template-compiler/lib/plugins/transform-action-syntax.js
+++ b/packages/ember-template-compiler/lib/plugins/transform-action-syntax.js
@@ -15,11 +15,13 @@
   with
 
   ```handlebars
- <button {{action this 'foo'}}>
- <button onblur={{action this 'foo'}}>
- <button onblur={{action this (action this 'foo') 'bar'}}>
+ <button {{action target=this 'foo'}}>
+ <button onblur={{action target=this 'foo'}}>
+ <button onblur={{action target=this (action target=this 'foo') 'bar'}}>
   ```
 
+  If an action already has a target it is left unmodified.
+
   @private
   @class TransformActionSyntax
 */
@@ -41,16 +43,19 @@ TransformActionSyntax.prototype.transform = function TransformActionSyntax_trans
     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 +67,24 @@ function isAction(node) {
   return node.path.original === 'action';
 }
 
+function ensureTarget(node, builders) {
+  if (!hashPairForKey(node.hash, 'target')) {
+    let thisTarget = builders.pair('target', builders.path('this'));
+    node.hash.pairs.push(thisTarget);
+  }
+}
+
+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;
+}
+
 function insertThisAsFirstParam(node, builders) {
   node.params.unshift(builders.path('this'));
 }

From 8e087234ce95d1cf9a2e3693cd5404ca3dc61ec0 Mon Sep 17 00:00:00 2001
From: Zack Mulgrew <zack@zackthehuman.com>
Date: Sun, 16 Oct 2016 08:48:09 -0700
Subject: [PATCH 2/3] Add test for issue #14469 to test closure action targets

---
 .../helpers/closure-action-test.js            | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/packages/ember-glimmer/tests/integration/helpers/closure-action-test.js b/packages/ember-glimmer/tests/integration/helpers/closure-action-test.js
index 12495ae5507..34eeb6ea98e 100644
--- a/packages/ember-glimmer/tests/integration/helpers/closure-action-test.js
+++ b/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';
 

From ffbc7bfdd05fac5a31a635f60be5b6fa992bd77f Mon Sep 17 00:00:00 2001
From: Zack Mulgrew <zack@zackthehuman.com>
Date: Sat, 15 Oct 2016 23:02:31 -0700
Subject: [PATCH 3/3] Remove implicit position this param in actions

---
 packages/ember-glimmer/lib/helpers/action.js  | 11 +++----
 .../ember-glimmer/lib/modifiers/action.js     | 31 +++++--------------
 .../lib/plugins/transform-action-syntax.js    | 26 +++-------------
 .../lib/system/hash-pair-for-key.js           | 10 ++++++
 4 files changed, 27 insertions(+), 51 deletions(-)
 create mode 100644 packages/ember-template-compiler/lib/system/hash-pair-for-key.js

diff --git a/packages/ember-glimmer/lib/helpers/action.js b/packages/ember-glimmer/lib/helpers/action.js
index 33ba0f100d2..976fa237554 100644
--- a/packages/ember-glimmer/lib/helpers/action.js
+++ b/packages/ember-glimmer/lib/helpers/action.js
@@ -279,14 +279,13 @@ export class ClosureActionReference extends CachedReference {
     let positionalValues = positional.value();
 
     let target = named.get('target').value();
-    let rawActionRef = positional.at(1);
-    let rawAction = positionalValues[1];
+    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
diff --git a/packages/ember-glimmer/lib/modifiers/action.js b/packages/ember-glimmer/lib/modifiers/action.js
index 75e1fdbd584..21cbf699339 100644
--- a/packages/ember-glimmer/lib/modifiers/action.js
+++ b/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();
diff --git a/packages/ember-template-compiler/lib/plugins/transform-action-syntax.js b/packages/ember-template-compiler/lib/plugins/transform-action-syntax.js
index 87124b7dcd8..510bf96681a 100644
--- a/packages/ember-template-compiler/lib/plugins/transform-action-syntax.js
+++ b/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,9 +17,9 @@
   with
 
   ```handlebars
- <button {{action target=this 'foo'}}>
- <button onblur={{action target=this 'foo'}}>
- <button onblur={{action target=this (action target=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.
@@ -42,19 +44,16 @@ 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);
       }
     }
@@ -73,18 +72,3 @@ function ensureTarget(node, builders) {
     node.hash.pairs.push(thisTarget);
   }
 }
-
-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;
-}
-
-function insertThisAsFirstParam(node, builders) {
-  node.params.unshift(builders.path('this'));
-}
diff --git a/packages/ember-template-compiler/lib/system/hash-pair-for-key.js b/packages/ember-template-compiler/lib/system/hash-pair-for-key.js
new file mode 100644
index 00000000000..20f0fdb98d1
--- /dev/null
+++ b/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;
+}