[BUGFIX release] Ensure closure actions are const

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

@@ -3,14 +3,17 @@
 @submodule ember-glimmer
 */
 import { symbol } from 'ember-utils';
-import { CachedReference } from '../utils/references';
 import {
-  Error as EmberError,
+  assert,
   run,
   get,
   flaggedInstrument,
-  isNone
+  isNone,
+  runInDebug
 } from 'ember-metal';
+import { UnboundReference } from '../utils/references';
+import { EvaluatedPositionalArgs } from 'glimmer-runtime';
+import { isConst } from 'glimmer-reference';
 
 export const INVOKE = symbol('INVOKE');
 export const ACTION = symbol('ACTION');
@@ -261,119 +264,118 @@ export const ACTION = symbol('ACTION');
   @for Ember.Templates.helpers
   @public
 */
-export class ClosureActionReference extends CachedReference {
-  static create(args) {
-    // TODO: Const reference optimization.
-    return new ClosureActionReference(args);
-  }
+export default function(vm, args) {
+  let { named, positional } = args;
 
-  constructor(args) {
-    super();
+  // The first two argument slots are reserved.
+  // pos[0] is the context (or `this`)
+  // pos[1] is the action name or function
+  // Anything else is an action argument.
+  let context = positional.at(0);
+  let action = positional.at(1);
 
-    this.args = args;
-    this.tag = args.tag;
-  }
+  // TODO: Is there a better way of doing this?
+  let debugKey = action._propertyKey;
 
-  compute() {
-    let { named, positional } = this.args;
-    let positionalValues = positional.value();
-
-    let target = positionalValues[0];
-    let rawActionRef = positional.at(1);
-    let rawAction = positionalValues[1];
-
-    // The first two argument slots are reserved.
-    // pos[0] is the context (or `this`)
-    // pos[1] is the action name or function
-    // Anything else is an action argument.
-    let actionArgs = positionalValues.slice(2);
-
-    // on-change={{action setName}}
-    // element-space actions look to "controller" then target. Here we only
-    // look to "target".
-    let actionType = typeof rawAction;
-    let action = rawAction;
-
-    if (rawActionRef[INVOKE]) {
-      target = rawActionRef;
-      action = rawActionRef[INVOKE];
-    } else if (isNone(rawAction)) {
-      throw new EmberError(`Action passed is null or undefined in (action) from ${target}.`);
-    } else if (actionType === 'string') {
-      // on-change={{action 'setName'}}
-      let actionName = rawAction;
-
-      action = null;
-
-      if (named.has('target')) {
-        // on-change={{action 'setName' target=alternativeComponent}}
-        target = named.get('target').value();
-      }
+  let restArgs;
 
-      if (target['actions']) {
-        action = target.actions[actionName];
-      }
+  if (positional.length === 2) {
+    restArgs = EvaluatedPositionalArgs.empty();
+  } else {
+    restArgs = EvaluatedPositionalArgs.create(positional.values.slice(2));
+  }
 
-      if (!action) {
-        throw new EmberError(`An action named '${actionName}' was not found in ${target}`);
-      }
-    } else if (action && typeof action[INVOKE] === 'function') {
-      target = action;
-      action = action[INVOKE];
-    } else if (actionType !== 'function') {
-      // TODO: Is there a better way of doing this?
-      let rawActionLabel = rawActionRef._propertyKey || rawAction;
-      throw new EmberError(`An action could not be made for \`${rawActionLabel}\` in ${target}. Please confirm that you are using either a quoted action name (i.e. \`(action '${rawActionLabel}')\`) or a function available in ${target}.`);
-    }
+  let target = named.has('target') ? named.get('target') : context;
+  let processArgs = makeArgsProcessor(named.has('value') && named.get('value'), restArgs);
 
-    let valuePath = named.get('value').value();
+  let fn;
 
-    return createClosureAction(target, action, valuePath, actionArgs);
+  if (typeof action[INVOKE] === 'function') {
+    fn = makeClosureAction(action, action, action[INVOKE], processArgs, debugKey);
+  } else if (isConst(target) && isConst(action)) {
+    fn = makeClosureAction(context.value(), target.value(), action.value(), processArgs, debugKey);
+  } else {
+    fn = makeDynamicClosureAction(context.value(), target, action, processArgs, debugKey);
   }
-}
 
-export default function(vm, args) {
-  return ClosureActionReference.create(args);
+  fn[ACTION] = true;
+
+  return new UnboundReference(fn);
 }
 
-export function createClosureAction(target, action, valuePath, actionArgs) {
-  let closureAction;
-  let actionArgLength = actionArgs.length;
+function NOOP(args) { return args; }
 
-  if (actionArgLength > 0) {
-    closureAction = function(...passedArguments) {
-      let args = new Array(actionArgLength + passedArguments.length);
+function makeArgsProcessor(valuePathRef, actionArgsRef) {
+  let mergeArgs = null;
 
-      for (let i = 0; i < actionArgLength; i++) {
-        args[i] = actionArgs[i];
-      }
+  if (actionArgsRef.length > 0) {
+    mergeArgs = function(args) {
+      return actionArgsRef.value().concat(args);
+    };
+  }
 
-      for (let i = 0; i < passedArguments.length; i++) {
-        args[i + actionArgLength] = passedArguments[i];
-      }
+  let readValue = null;
+
+  if (valuePathRef) {
+    readValue = function(args) {
+      let valuePath = valuePathRef.value();
 
       if (valuePath && args.length > 0) {
         args[0] = get(args[0], valuePath);
       }
 
-      let payload = { target, args, label: 'glimmer-closure-action' };
-      return flaggedInstrument('interaction.ember-action', payload, () => {
-        return run.join(target, action, ...args);
-      });
+      return args;
     };
-  } else {
-    closureAction = function(...args) {
-      if (valuePath && args.length > 0) {
-        args[0] = get(args[0], valuePath);
-      }
+  }
 
-      let payload = { target, args, label: 'glimmer-closure-action' };
-      return flaggedInstrument('interaction.ember-action', payload, () => {
-        return run.join(target, action, ...args);
-      });
+  if (mergeArgs && readValue) {
+    return function(args) {
+      return readValue(mergeArgs(args));
     };
+  } else {
+    return mergeArgs || readValue || NOOP;
+  }
+}
+
+function makeDynamicClosureAction(context, targetRef, actionRef, processArgs, debugKey) {
+  // We don't allow undefined/null values, so this creates a throw-away action to trigger the assertions
+  runInDebug(function() {
+    makeClosureAction(context, targetRef.value(), actionRef.value(), processArgs, debugKey);
+  });
+
+  return function(...args) {
+    return makeClosureAction(context, targetRef.value(), actionRef.value(), processArgs, debugKey)(...args);
+  };
+}
+
+function makeClosureAction(context, target, action, processArgs, debugKey) {
+  let self, fn;
+
+  assert(`Action passed is null or undefined in (action) from ${target}.`, !isNone(action));
+
+  if (typeof action[INVOKE] === 'function') {
+    self = action;
+    fn   = action[INVOKE];
+  } else {
+    let typeofAction = typeof action;
+
+    if (typeofAction === 'string') {
+      self = target;
+      fn   = target.actions && target.actions[action];
+
+      assert(`An action named '${action}' was not found in ${target}`, fn);
+    } else if (typeofAction === 'function') {
+      self = context;
+      fn   = action;
+    } else {
+      assert(`An action could not be made for \`${debugKey || action}\` in ${target}. Please confirm that you are using either a quoted action name (i.e. \`(action '${debugKey || 'myAction'}')\`) or a function available in ${target}.`, false);
+    }
   }
 
-  closureAction[ACTION] = true;
-  return closureAction;
+  return function(...args) {
+    let payload = { target: self, args, label: 'glimmer-closure-action' };
+    return flaggedInstrument('interaction.ember-action', payload, () => {
+      return run.join(self, fn, ...processArgs(args));
+    });
+  };
 }