[BUGFIX beta] Ensure helpers have a consistent API.

Having class based helpers return a different API from "simple" helpers
is quite confusing. It makes testing a helper significantly different
based on the type of helper, and also makes refactoring from simple to
class-based a hazzard.

This change ensures that both types of helper return the same basic
interface, and enables us to use output similar to the following
regardless of the specific type of helper that was exported:

```js
module('foo | Helper', function(hooks) {
  module('unit tests', function(hooks) {
    setupRenderingTest(hooks);

    test('returns some-value', async function(assert) {
      await render(hbs`{{foo 'input' 'values'}}`);

      assert.equal(this.element.textContent, 'some-value', 'helper works!');
    });
  });

  module('unit tests', function(hooks) {
    setupTest(hooks);

    test('returns some-value', function(assert) {
      let helper = this.owner.factoryFor('helper:foo').create();

      assert.equal(helper.compute(['input', 'values']), 'some-value', 'helper works!');
    });
  });
});
```

packages/ember-application/tests/system/dependency_injection/default_resolver_test.js

@@ -98,7 +98,7 @@ moduleFor('Ember.Application Dependency Injection - Integration - default resolv
 
     let lookedUpShorthandHelper = this.applicationInstance.factoryFor('helper:shorthand').class;
 
-    assert.ok(lookedUpShorthandHelper.isHelperInstance, 'shorthand helper isHelper');
+    assert.ok(lookedUpShorthandHelper.isHelperFactory, 'shorthand helper isHelper');
 
     let lookedUpHelper = this.applicationInstance.factoryFor('helper:complete').class;
 
@@ -115,7 +115,7 @@ moduleFor('Ember.Application Dependency Injection - Integration - default resolv
 
     let lookedUpShorthandHelper = this.applicationInstance.factoryFor('helper:shorthand').class;
 
-    assert.ok(lookedUpShorthandHelper.isHelperInstance, 'shorthand helper isHelper');
+    assert.ok(lookedUpShorthandHelper.isHelperFactory, 'shorthand helper isHelper');
 
     let lookedUpHelper = this.applicationInstance.factoryFor('helper:complete').class;
 

packages/ember-glimmer/lib/environment.ts

@@ -1,8 +1,10 @@
 import {
   Reference,
+  PathReference,
 } from '@glimmer/reference';
 import {
   AttributeManager,
+  CapturedArguments,
   CompilableLayout,
   CompiledDynamicProgram,
   compileLayout,
@@ -15,6 +17,7 @@ import {
   PrimitiveReference,
   PartialDefinition,
   Simple,
+  VM
 } from '@glimmer/runtime';
 import {
   Destroyable, Opaque,
@@ -285,13 +288,18 @@ export default class Environment extends GlimmerEnvironment {
     let helperFactory = owner.factoryFor(`helper:${name}`, options) || owner.factoryFor(`helper:${name}`);
 
     // TODO: try to unify this into a consistent protocol to avoid wasteful closure allocations
-    if (helperFactory.class.isHelperInstance) {
-      return (_vm, args) => SimpleHelperReference.create(helperFactory.class.compute, args.capture());
+    let HelperReference: {
+      create: (HelperFactory: any, vm: VM, args: CapturedArguments ) => PathReference<Opaque>;
+    };
+    if (helperFactory.class.isSimpleHelperFactory) {
+      HelperReference = SimpleHelperReference;
     } else if (helperFactory.class.isHelperFactory) {
-      return (vm, args) => ClassBasedHelperReference.create(helperFactory, vm, args.capture());
+      HelperReference = ClassBasedHelperReference;
     } else {
       throw new Error(`${name} is not a helper`);
     }
+
+    return (vm, args) => HelperReference.create(helperFactory, vm, args.capture());
   }
 
   hasModifier(name: string) {

packages/ember-glimmer/lib/helper.ts

@@ -104,6 +104,29 @@ Helper.reopenClass({
   isHelperFactory: true,
 });
 
+export interface HelperInstance {
+  isHelperInstance: true;
+  compute: Function;
+}
+
+export class SimpleHelperFactory {
+  isHelperFactory = true;
+  isSimpleHelperFactory = true;
+
+  private instance: HelperInstance;
+
+  constructor(compute: (positionalValue: any[], namedValue: any[]) => any) {
+    this.instance = {
+      isHelperInstance: true,
+      compute,
+    };
+  }
+
+  create(): HelperInstance {
+    return this.instance;
+  }
+}
+
 /**
   In many cases, the ceremony of a full `Helper` class is not required.
   The `helper` method create pure-function helpers without instances. For
@@ -127,10 +150,7 @@ Helper.reopenClass({
   @since 1.13.0
 */
 export function helper(helperFn: (params: any[], hash?: any) => string) {
-  return {
-    isHelperInstance: true,
-    compute: helperFn,
-  };
+  return new SimpleHelperFactory(helperFn);
 }
 
 export default Helper;

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

@@ -32,7 +32,10 @@ import {
   EMBER_GLIMMER_DETECT_BACKTRACKING_RERENDER,
   MANDATORY_SETTER,
 } from 'ember/features';
-import { RECOMPUTE_TAG } from '../helper';
+import {
+  RECOMPUTE_TAG,
+  SimpleHelperFactory,
+} from '../helper';
 import emberToBool from './to-bool';
 
 export const UPDATE = symbol('UPDATE');
@@ -329,7 +332,9 @@ export class SimpleHelperReference extends CachedReference {
   public helper: (positionalValue: any, namedValue: any) => any;
   public args: any;
 
-  static create(helper: (positionalValue: any, namedValue: any) => any, args: CapturedArguments) {
+  static create(Helper: SimpleHelperFactory, _vm: VM, args: CapturedArguments) {
+    let helper = Helper.create();
+
     if (isConst(args)) {
       let { positional, named } = args;
 
@@ -341,15 +346,15 @@ export class SimpleHelperReference extends CachedReference {
         maybeFreeze(namedValue);
       }
 
-      let result = helper(positionalValue, namedValue);
+      let result = helper.compute(positionalValue, namedValue);
 
       if (typeof result === 'object' && result !== null || typeof result === 'function') {
         return new RootReference(result);
       } else {
         return PrimitiveReference.create(result);
       }
     } else {
-      return new SimpleHelperReference(helper, args);
+      return new SimpleHelperReference(helper.compute, args);
     }
   }
 

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

@@ -564,6 +564,32 @@ moduleFor('Helpers test: custom helpers', class extends RenderingTest {
 
     equal(destroyCount, 1, 'destroy is called after a view is destroyed');
   }
+
+  ['@test simple helper can be invoked manually via `owner.factoryFor(...).create().compute()'](assert) {
+    this.registerHelper('some-helper', () => {
+      assert.ok(true, 'some-helper helper invoked');
+      return 'lolol';
+    });
+
+    let instance = this.owner.factoryFor('helper:some-helper').create();
+
+    assert.equal(typeof instance.compute, 'function', 'expected instance.compute to be present');
+    assert.equal(instance.compute(), 'lolol', 'can invoke `.compute`');
+  }
+
+  ['@test class-based helper can be invoked manually via `owner.factoryFor(...).create().compute()']() {
+    this.registerHelper('some-helper', {
+      compute() {
+        assert.ok(true, 'some-helper helper invoked');
+        return 'lolol';
+      }
+    });
+
+    let instance = this.owner.factoryFor('helper:some-helper').create();
+
+    assert.equal(typeof instance.compute, 'function', 'expected instance.compute to be present');
+    assert.equal(instance.compute(), 'lolol', 'can invoke `.compute`');
+  }
 });
 
 // these feature detects prevent errors in these tests