From 7c9b9461f24d56d77b0ecb19e8203f7171a4563c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Morais?= Date: Tue, 12 Oct 2021 21:18:19 +0100 Subject: [PATCH] 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 #23 --- addon/-private/assert-function.js | 5 ++ addon/modifiers/did-insert.js | 4 ++ addon/modifiers/did-update.js | 8 +++ addon/modifiers/will-destroy.js | 8 ++- .../integration/modifiers/did-insert-test.js | 32 ++++++++++- .../integration/modifiers/did-update-test.js | 46 +++++++++++++++- .../modifiers/will-destroy-test.js | 55 ++++++++++++++++++- 7 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 addon/-private/assert-function.js diff --git a/addon/-private/assert-function.js b/addon/-private/assert-function.js new file mode 100644 index 0000000..b5bbbe2 --- /dev/null +++ b/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}"`); +} diff --git a/addon/modifiers/did-insert.js b/addon/modifiers/did-insert.js index de92290..b3cab17 100644 --- a/addon/modifiers/did-insert.js +++ b/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); }, diff --git a/addon/modifiers/did-update.js b/addon/modifiers/did-update.js index ce11898..ccd2201 100644 --- a/addon/modifiers/did-update.js +++ b/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); }, diff --git a/addon/modifiers/will-destroy.js b/addon/modifiers/will-destroy.js index 6a585a6..269d6a4 100644 --- a/addon/modifiers/will-destroy.js +++ b/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); }, }), diff --git a/tests/integration/modifiers/did-insert-test.js b/tests/integration/modifiers/did-insert-test.js index 2edc9b6..b892ae7 100644 --- a/tests/integration/modifiers/did-insert-test.js +++ b/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` +
+ {{yield}} +
+ ` + ); + + await render(hbs` + {{!-- template-lint-disable no-curly-component-invocation --}} + {{undefined-method-call}} + `); + + // Reset error capturing + setupOnerror(); + }); }); diff --git a/tests/integration/modifiers/did-update-test.js b/tests/integration/modifiers/did-update-test.js index ae92f14..7e4d7ca 100644 --- a/tests/integration/modifiers/did-update-test.js +++ b/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` +
+ `); + + // 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` +
+ `); + + // Remove the function to trigger an error on update + this.set('nonExistentMethod', undefined); + + // Reset error capturing + setupOnerror(); + }); }); diff --git a/tests/integration/modifiers/will-destroy-test.js b/tests/integration/modifiers/will-destroy-test.js index 8036e87..82e425b 100644 --- a/tests/integration/modifiers/will-destroy-test.js +++ b/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` +
+ `); + + // 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}} +
+ {{/if}} + `); + + // Remove the function to trigger an error on destroy + this.setProperties({ + nonExistentMethod: undefined, + show: false, + }); + + // Reset error capturing + setupOnerror(); + }); });