Implement class-based helpers in Glimmer

package.json

@@ -35,7 +35,7 @@
     "express": "^4.5.0",
     "finalhandler": "^0.4.0",
     "github": "^0.2.3",
-    "glimmer-engine": "tildeio/glimmer#a1d0a23",
+    "glimmer-engine": "tildeio/glimmer#d6ae47a",
     "glob": "~4.3.2",
     "htmlbars": "0.14.14",
     "qunit-extras": "^1.4.0",

packages/ember-glimmer/lib/environment.js

@@ -5,12 +5,17 @@ import { DynamicComponentSyntax } from './components/dynamic-component';
 import { OutletSyntax } from './components/outlet';
 import lookupComponent from './utils/lookup-component';
 import createIterable from './utils/iterable';
-import { RootReference, ConditionalReference } from './utils/references';
+import {
+  RootReference,
+  ConditionalReference,
+  SimpleHelperReference,
+  ClassBasedHelperReference
+} from './utils/references';
 
 import { default as concat } from './helpers/concat';
 import { default as inlineIf } from './helpers/inline-if';
 
-const helpers = {
+const builtInHelpers = {
   concat,
   if: inlineIf
 };
@@ -69,26 +74,21 @@ export default class extends Environment {
   }
 
   hasHelper(name) {
-    if (typeof helpers[name[0]] === 'function') {
-      return true;
-    } else {
-      return this.owner.hasRegistration(`helper:${name}`);
-    }
+    return !!builtInHelpers[name[0]] || this.owner.hasRegistration(`helper:${name}`);
   }
 
   lookupHelper(name) {
-    if (typeof helpers[name[0]] === 'function') {
-      return helpers[name[0]];
+    let helper = builtInHelpers[name[0]] || this.owner.lookup(`helper:${name}`);
+
+    // TODO: try to unify this into a consistent protocol to avoid wasteful closure allocations
+    if (helper.isInternalHelper) {
+      return (args) => helper.toReference(args);
+    } else if (helper.isHelperInstance) {
+      return (args) => new SimpleHelperReference(helper.compute, args);
+    } else if (helper.isHelperFactory) {
+      return (args) => new ClassBasedHelperReference(helper.create(), args);
     } else {
-      let helper = this.owner.lookup(`helper:${name}`);
-
-      if (helper && helper.isHelperInstance) {
-        return helper.compute;
-      } else if (helper && helper.isHelperFactory) {
-        throw new Error(`Not implemented: ${name} is a class-based helpers`);
-      } else {
-        throw new Error(`${name} is not a helper`);
-      }
+      throw new Error(`${name} is not a helper`);
     }
   }
 

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

@@ -1,3 +1,5 @@
+import { helper } from '../helper';
+
 /**
 @module ember
 @submodule ember-templates
@@ -19,6 +21,8 @@
   @for Ember.Templates.helpers
   @since 1.13.0
 */
-export default function concat(args) {
+function concat(args) {
   return args.join('');
 }
+
+export default helper(concat);

packages/ember-glimmer/lib/helpers/inline-if.js

@@ -3,8 +3,9 @@
 @submodule ember-templates
 */
 
-import { toBool as emberToBool } from './if-unless';
 import { assert } from 'ember-metal/debug';
+import emberToBool from '../utils/to-bool';
+import { InternalHelperReference } from '../utils/references';
 
 /**
   The inline `if` helper conditionally renders a single property or string.
@@ -22,18 +23,38 @@ import { assert } from 'ember-metal/debug';
   @for Ember.Templates.helpers
   @public
 */
-export default function inlineIf(args) {
-  assert(
-   'The inline form of the `if` and `unless` helpers expect two or ' +
-   'three arguments, e.g. `{{if trialExpired \'Expired\' expiryDate}}` ',
-   args.length === 2 || args.length === 3
- );
+function inlineIf({ positional }) {
+  let condition = positional.at(0).value();
 
-  if (emberToBool(args[0])) {
-    return args[1];
+  if (emberToBool(condition)) {
+    return positional.at(1).value();
   } else {
-    //TODO: always return `args[2]` post glimmer2: https://github.com/emberjs/ember.js/pull/12920#discussion_r53213383
-    let falsyArgument = args[2];
-    return falsyArgument === undefined ? '' : falsyArgument;
+    return positional.at(2).value();
   }
 }
+
+function simpleInlineIf({ positional }) {
+  let condition = positional.at(0).value();
+
+  if (emberToBool(condition)) {
+    return positional.at(1).value();
+  } else {
+    // TODO: this should probably be `undefined`: https://github.com/emberjs/ember.js/pull/12920#discussion_r53213383
+    return '';
+  }
+}
+
+export default {
+  isInternalHelper: true,
+  toReference(args) {
+    switch (args.positional.length) {
+      case 2: return new InternalHelperReference(simpleInlineIf, args);
+      case 3: return new InternalHelperReference(inlineIf, args);
+      default:
+        assert(
+          'The inline form of the `if` and `unless` helpers expect two or ' +
+          'three arguments, e.g. `{{if trialExpired \'Expired\' expiryDate}}`'
+        );
+    }
+  }
+};

packages/ember-glimmer/lib/utils/references.js

@@ -1,6 +1,6 @@
 import { get } from 'ember-metal/property_get';
 import { ConditionalReference as GlimmerConditionalReference } from 'glimmer-runtime';
-import { toBool as emberToBool } from '../helpers/if-unless';
+import emberToBool from './to-bool';
 
 // @implements PathReference
 export class RootReference {
@@ -20,8 +20,7 @@ export class RootReference {
     return new PropertyReference(this, propertyKey);
   }
 
-  destroy() {
-  }
+  destroy() {}
 }
 
 // @implements PathReference
@@ -43,8 +42,7 @@ class PropertyReference {
     return new PropertyReference(this, propertyKey);
   }
 
-  destroy() {
-  }
+  destroy() {}
 }
 
 // @implements PathReference
@@ -59,3 +57,69 @@ export class ConditionalReference extends GlimmerConditionalReference {
     return emberToBool(predicate);
   }
 }
+
+// @implements PathReference
+export class SimpleHelperReference {
+  constructor(helper, args) {
+    this.helper = helper;
+    this.args = args;
+  }
+
+  isDirty() { return true; }
+
+  value() {
+    let { helper, args: { positional, named } } = this;
+
+    return helper(positional.value(), named.value());
+  }
+
+  get(propertyKey) {
+    return new PropertyReference(this, propertyKey);
+  }
+
+  destroy() {}
+}
+
+// @implements PathReference
+export class ClassBasedHelperReference {
+  constructor(instance, args) {
+    this.instance = instance;
+    this.args = args;
+  }
+
+  isDirty() { return true; }
+
+  value() {
+    let { instance, args: { positional, named } } = this;
+
+    return instance.compute(positional.value(), named.value());
+  }
+
+  get(propertyKey) {
+    return new PropertyReference(this, propertyKey);
+  }
+
+  destroy() {}
+}
+
+// @implements PathReference
+export class InternalHelperReference {
+  constructor(helper, args) {
+    this.helper = helper;
+    this.args = args;
+  }
+
+  isDirty() { return true; }
+
+  value() {
+    let { helper, args } = this;
+
+    return helper(args);
+  }
+
+  get(propertyKey) {
+    return new PropertyReference(this, propertyKey);
+  }
+
+  destroy() {}
+}

packages/ember-glimmer/lib/helpers/if-unless.js → packages/ember-glimmer/lib/utils/to-bool.js

@@ -1,7 +1,7 @@
 import { isArray } from 'ember-runtime/utils';
 import { get } from 'ember-metal/property_get';
 
-export function toBool(predicate) {
+export default function toBool(predicate) {
   if (!predicate) {
     return false;
   }

packages/ember-glimmer/tests/integration/helpers/custom-helper-test.js

@@ -10,7 +10,7 @@ moduleFor('Helpers test: custom helpers', class extends RenderingTest {
     this.assertText('hello world');
   }
 
-  ['@htmlbars it can resolve custom class-based helpers']() {
+  ['@test it can resolve custom class-based helpers']() {
     this.registerHelper('hello-world', {
       compute() {
         return 'hello world';