Improve not-a-function error message

When the passed `fn` to the helper is not a function, this commit
improves the error message users will see, hopefully helping with
the debugging process.

Fixes emberjs#23

Author: gnclmorais

addon/-private/assert-function.js

@@ -0,0 +1,5 @@
+export default function assertFunction(modifierName, maybeFunction) {
+  if (typeof maybeFunction === 'function') return;
+
+  throw new TypeError(`${modifierName} expected a function, instead received "${maybeFunction}"`);
+}

addon/modifiers/did-insert.js

@@ -1,6 +1,8 @@
 import { setModifierManager, capabilities } from '@ember/modifier';
 import { gte } from 'ember-compatibility-helpers';
 
+import assertFunction from '../-private/assert-function';
+
 /**
   The `{{did-insert}}` element modifier is activated when an element is
   inserted into the DOM.
@@ -52,6 +54,8 @@ export default setModifierManager(
     createModifier() {},
 
     installModifier(_state, element, { positional: [fn, ...args], named }) {
+      assertFunction('did-insert', fn);
+
       fn(element, args, named);
     },
 

addon/modifiers/did-update.js

@@ -1,6 +1,8 @@
 import { setModifierManager, capabilities } from '@ember/modifier';
 import { gte } from 'ember-compatibility-helpers';
 
+import assertFunction from '../-private/assert-function';
+
 /**
   The `{{did-update}}` element modifier is activated when any of its arguments
   are updated. It does not run on initial render.
@@ -67,6 +69,10 @@ export default setModifierManager(
       return { element: null };
     },
     installModifier(state, element, args) {
+      const [fn] = args.positional;
+
+      assertFunction('did-update', fn);
+
       // save element into state bucket
       state.element = element;
 
@@ -90,6 +96,8 @@ export default setModifierManager(
 
       let [fn, ...positional] = args.positional;
 
+      assertFunction('did-update', fn);
+
       fn(element, positional, args.named);
     },
 

addon/modifiers/will-destroy.js

@@ -1,6 +1,8 @@
 import { setModifierManager, capabilities } from '@ember/modifier';
 import { gte } from 'ember-compatibility-helpers';
 
+import assertFunction from '../-private/assert-function';
+
 /**
   The `{{will-destroy}}` element modifier is activated immediately before the element
   is removed from the DOM.
@@ -47,7 +49,9 @@ export default setModifierManager(
       return { element: null };
     },
 
-    installModifier(state, element) {
+    installModifier(state, element, { positional: [fn] }) {
+      assertFunction('did-destroy', fn);
+
       state.element = element;
     },
 
@@ -56,6 +60,8 @@ export default setModifierManager(
     destroyModifier({ element }, args) {
       let [fn, ...positional] = args.positional;
 
+      assertFunction('did-destroy', fn);
+
       fn(element, positional, args.named);
     },
   }),

tests/integration/modifiers/did-insert-test.js

@@ -1,6 +1,6 @@
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
-import { render } from '@ember/test-helpers';
+import { render, setupOnerror } from '@ember/test-helpers';
 import hbs from 'htmlbars-inline-precompile';
 
 // We want to use ember classic components in this test
@@ -84,4 +84,34 @@ module('Integration | Modifier | did-insert', function (hooks) {
 
     assert.dom('.alert').hasClass('fade-in');
   });
+
+  test('provides a useful error on insert', async function (assert) {
+    assert.expect(1);
+
+    // Setup error capturing
+    setupOnerror(function (err) {
+      assert.equal(
+        err.toString(),
+        `TypeError: did-insert expected a function, instead received "undefined"`
+      );
+    });
+
+    this.owner.register('component:undefined-method-call', Component);
+    this.owner.register(
+      'template:components/undefined-method-call',
+      hbs`
+      <div {{did-insert this.nonExistentMethod}} class="alert">
+        {{yield}}
+      </div>
+    `
+    );
+
+    await render(hbs`
+      {{!-- template-lint-disable no-curly-component-invocation --}}
+      {{undefined-method-call}}
+    `);
+
+    // Reset error capturing
+    setupOnerror();
+  });
 });

tests/integration/modifiers/did-update-test.js

@@ -1,6 +1,6 @@
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
-import { render } from '@ember/test-helpers';
+import { render, setupOnerror } from '@ember/test-helpers';
 import hbs from 'htmlbars-inline-precompile';
 
 module('Integration | Modifier | did-update', function (hooks) {
@@ -24,4 +24,48 @@ module('Integration | Modifier | did-update', function (hooks) {
 
     this.set('boundValue', 'update');
   });
+
+  test('provides a useful error on install', async function (assert) {
+    assert.expect(1);
+
+    // Setup error capturing
+    setupOnerror(function (err) {
+      assert.equal(
+        err.toString(),
+        `TypeError: did-update expected a function, instead received "undefined"`
+      );
+    });
+
+    await render(hbs`
+      <div {{did-update this.nonExistentMethod}}></div>
+    `);
+
+    // Reset error capturing
+    setupOnerror();
+  });
+
+  test('provides a useful error on update', async function (assert) {
+    assert.expect(1);
+
+    // Start with a valid function so that install works
+    this.set('nonExistentMethod', () => {});
+
+    // Setup error capturing
+    setupOnerror(function (err) {
+      assert.equal(
+        err.toString(),
+        `TypeError: did-update expected a function, instead received "undefined"`
+      );
+    });
+
+    await render(hbs`
+      <div {{did-update this.nonExistentMethod}}></div>
+    `);
+
+    // Remove the function to trigger an error on update
+    this.set('nonExistentMethod', undefined);
+
+    // Reset error capturing
+    setupOnerror();
+  });
 });

tests/integration/modifiers/will-destroy-test.js

@@ -1,6 +1,6 @@
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
-import { render } from '@ember/test-helpers';
+import { render, setupOnerror } from '@ember/test-helpers';
 import hbs from 'htmlbars-inline-precompile';
 
 module('Integration | Modifier | will-destroy', function (hooks) {
@@ -43,4 +43,57 @@ module('Integration | Modifier | will-destroy', function (hooks) {
     // trigger destroy
     this.set('show', false);
   });
+
+  test('provides a useful error on install', async function (assert) {
+    assert.expect(1);
+
+    // Setup error capturing
+    setupOnerror(function (err) {
+      assert.equal(
+        err.toString(),
+        `TypeError: did-destroy expected a function, instead received "undefined"`
+      );
+    });
+
+    await render(hbs`
+      <div {{will-destroy this.nonExistentMethod}}></div>
+    `);
+
+    // Prevent double error on test teardown
+    this.set('nonExistentMethod', () => {});
+
+    // Reset error capturing
+    setupOnerror();
+  });
+
+  test('provides a useful error on destroy', async function (assert) {
+    assert.expect(1);
+
+    // Start with a valid function so that install works
+    this.set('nonExistentMethod', () => {});
+
+    // Setup error capturing
+    setupOnerror(function (err) {
+      assert.equal(
+        err.toString(),
+        `TypeError: did-destroy expected a function, instead received "undefined"`
+      );
+    });
+
+    this.set('show', true);
+    await render(hbs`
+      {{#if this.show}}
+        <div {{will-destroy this.nonExistentMethod}}></div>
+      {{/if}}
+    `);
+
+    // Remove the function to trigger an error on destroy
+    this.setProperties({
+      nonExistentMethod: undefined,
+      show: false,
+    });
+
+    // Reset error capturing
+    setupOnerror();
+  });
 });