Refactoring curly component argument processing

This cleans up and unifies the handling of curly comopnent arguments. It splits the work into two distinct phases - gatherArgs assembles an EvaluatedArgs that combines all variations of positional and named arguments into a single EvaluatedArgs object - ComponentArgs.create takes the EvaluatedArgs and produces the curly-component specific attrs & props structures with mutable cells, etc.
emberjs · Sep 27, 2016 · b034666 · b034666
1 parent ca66d19
commit b034666
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 102 deletions.
diff --git a/packages/ember-glimmer/lib/syntax/curly-component.js b/packages/ember-glimmer/lib/syntax/curly-component.js
@@ -1,10 +1,7 @@
-import { assign, OWNER } from 'ember-utils';
+import { OWNER } from 'ember-utils';
 import {
   StatementSyntax,
   ValueReference,
-  EvaluatedArgs,
-  EvaluatedNamedArgs,
-  EvaluatedPositionalArgs,
   ComponentDefinition
 } from 'glimmer-runtime';
 import {
@@ -28,7 +25,10 @@ import {
 import {
   setViewElement
 } from 'ember-views';
-import processArgs from '../utils/process-args';
+import {
+  gatherArgs,
+  ComponentArgs
+} from '../utils/process-args';
 import { privatize as P } from 'container';
 
 const DEFAULT_LAYOUT = P`template:components/-default`;
@@ -181,40 +181,14 @@ class CurlyComponentManager {
   prepareArgs(definition, args) {
     validatePositionalParameters(args.named, args.positional.values, definition.ComponentClass.positionalParams);
 
-    if (definition.args) {
-      let newNamed = args.named.map;
-      let newPositional = args.positional.values;
-
-      let oldNamed = definition.args.named.map;
-      let oldPositional = definition.args.positional.values;
-
-      // Merge positional arrays
-      let mergedPositional = [];
-
-      mergedPositional.push(...oldPositional);
-      mergedPositional.splice(0, newPositional.length, ...newPositional);
-
-      // Merge named maps
-      let mergedNamed = assign({}, oldNamed, newNamed);
-
-      // THOUGHT: It might be nice to have a static method on EvaluatedArgs that
-      // can merge two sets of args for us.
-      let mergedArgs = EvaluatedArgs.create(
-        EvaluatedPositionalArgs.create(mergedPositional),
-        EvaluatedNamedArgs.create(mergedNamed)
-      );
-
-      return mergedArgs;
-    }
-
-    return args;
+    return gatherArgs(args, definition);
   }
 
   create(environment, definition, args, dynamicScope, callerSelfRef, hasBlock) {
     let parentView = dynamicScope.view;
 
     let klass = definition.ComponentClass;
-    let processedArgs = processArgs(args, klass.positionalParams);
+    let processedArgs = ComponentArgs.create(args);
     let { attrs, props } = processedArgs.value();
 
     aliasIdToElementId(args, props);

diff --git a/packages/ember-glimmer/lib/utils/process-args.js b/packages/ember-glimmer/lib/utils/process-args.js
@@ -1,33 +1,80 @@
-import { symbol, EmptyObject } from 'ember-utils';
-import { CONSTANT_TAG } from 'glimmer-reference';
+import {
+  assign,
+  symbol,
+  EmptyObject
+} from 'ember-utils';
+import {
+  CONSTANT_TAG
+} from 'glimmer-reference';
 import { ARGS } from '../component';
 import { UPDATE } from './references';
 import { MUTABLE_CELL } from 'ember-views';
+import {
+  EvaluatedArgs,
+  EvaluatedPositionalArgs
+} from 'glimmer-runtime';
+
+// Maps all variants of positional and dynamically scoped arguments
+// into the named arguments. Input `args` and return value are both
+// `EvaluatedArgs`.
+export function gatherArgs(args, definition) {
+  let namedMap = gatherNamedMap(args, definition);
+  let positionalValues = gatherPositionalValues(args, definition);
+  return mergeArgs(namedMap, positionalValues, definition.ComponentClass);
+}
+
+function gatherNamedMap(args, definition) {
+  let namedMap = args.named.map;
+  if (definition.args) {
+    return assign({}, definition.args.named.map, namedMap);
+  } else {
+    return namedMap;
+  }
+}
 
-export default function processArgs(args, positionalParamsDefinition) {
-  if (!positionalParamsDefinition || positionalParamsDefinition.length === 0 || args.positional.length === 0) {
-    return SimpleArgs.create(args);
-  } else if (typeof positionalParamsDefinition === 'string') {
-    return RestArgs.create(args, positionalParamsDefinition);
+function gatherPositionalValues(args, definition) {
+  let positionalValues = args.positional.values;
+  if (definition.args) {
+    let oldPositional = definition.args.positional.values;
+    let newPositional = [];
+    newPositional.push(...oldPositional);
+    newPositional.splice(0, positionalValues.length, ...positionalValues);
+    return newPositional;
   } else {
-    return PositionalArgs.create(args, positionalParamsDefinition);
+    return positionalValues;
+  }
+}
+
+function mergeArgs(namedMap, positionalValues, componentClass) {
+  let positionalParamsDefinition = componentClass.positionalParams;
+
+  if (positionalParamsDefinition && positionalParamsDefinition.length > 0 && positionalValues.length > 0) {
+    if (typeof positionalParamsDefinition === 'string') {
+      namedMap = mergeRestArg(namedMap, positionalValues, positionalParamsDefinition);
+    } else {
+      namedMap = mergePositionalParams(namedMap, positionalValues, positionalParamsDefinition);
+    }
   }
+  return EvaluatedArgs.named(namedMap);
 }
 
 const EMPTY_ARGS = {
   tag: CONSTANT_TAG,
-
   value() {
     return { attrs: {}, props: { attrs: {}, [ARGS]: {} } };
   }
 };
 
-class SimpleArgs {
-  static create({ named }) {
-    if (named.keys.length === 0) {
+
+// ComponentArgs takes EvaluatedNamedArgs and converts them into the
+// inputs needed by CurlyComponents (attrs and props, with mutable
+// cells, etc).
+export class ComponentArgs {
+  static create(args) {
+    if (args.named.keys.length === 0) {
       return EMPTY_ARGS;
     } else {
-      return new SimpleArgs(named);
+      return new ComponentArgs(args.named);
     }
   }
 
@@ -64,6 +111,22 @@ class SimpleArgs {
   }
 }
 
+function mergeRestArg(namedMap, positionalValues, restArgName) {
+  let mergedNamed = assign({}, namedMap);
+  mergedNamed[restArgName] = EvaluatedPositionalArgs.create(positionalValues);
+  return mergedNamed;
+}
+
+function mergePositionalParams(namedMap, values, positionalParamNames) {
+  let mergedNamed = assign({}, namedMap);
+  let length = Math.min(values.length, positionalParamNames.length);
+  for (let i = 0; i < length; i++) {
+    let name = positionalParamNames[i];
+    mergedNamed[name] = values[i];
+  }
+  return mergedNamed;
+}
+
 const REF = symbol('REF');
 
 class MutableCell {
@@ -77,59 +140,3 @@ class MutableCell {
     this[REF][UPDATE](val);
   }
 }
-
-class RestArgs {
-  static create(args, restArgName) {
-    return new RestArgs(args, restArgName);
-  }
-
-  constructor(args, restArgName) {
-    this.tag = args.tag;
-    this.simpleArgs = SimpleArgs.create(args);
-    this.positionalArgs = args.positional;
-    this.restArgName = restArgName;
-  }
-
-  value() {
-    let { simpleArgs, positionalArgs, restArgName } = this;
-
-    let result = simpleArgs.value();
-
-    result.props[ARGS] = positionalArgs;
-    result.attrs[restArgName] = result.props[restArgName] = positionalArgs.value();
-
-    return result;
-  }
-}
-
-
-class PositionalArgs {
-  static create(args, positionalParamNames) {
-    if (args.positional.length < positionalParamNames.length) {
-      positionalParamNames = positionalParamNames.slice(0, args.positional.length);
-    }
-
-    return new PositionalArgs(args, positionalParamNames);
-  }
-
-  constructor(args, positionalParamNames) {
-    this.tag = args.tag;
-    this.simpleArgs = SimpleArgs.create(args);
-    this.positionalArgs = args.positional;
-    this.positionalParamNames = positionalParamNames;
-  }
-
-  value() {
-    let { simpleArgs, positionalArgs, positionalParamNames } = this;
-
-    let result = simpleArgs.value();
-
-    for (let i = 0; i < positionalParamNames.length; i++) {
-      let name = positionalParamNames[i];
-      let reference = result.props[ARGS][name] = positionalArgs.at(i);
-      result.attrs[name] = result.props[name] = reference.value();
-    }
-
-    return result;
-  }
-}