Skip to content

Commit

Permalink
Improve not-a-function error message
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gnclmorais committed Oct 12, 2021
1 parent f21c434 commit 7c9b946
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 4 deletions.
5 changes: 5 additions & 0 deletions addon/-private/assert-function.js
Original file line number Diff line number Diff line change
@@ -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}"`);
}
4 changes: 4 additions & 0 deletions addon/modifiers/did-insert.js
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -52,6 +54,8 @@ export default setModifierManager(
createModifier() {},

installModifier(_state, element, { positional: [fn, ...args], named }) {
assertFunction('did-insert', fn);

fn(element, args, named);
},

Expand Down
8 changes: 8 additions & 0 deletions addon/modifiers/did-update.js
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -90,6 +96,8 @@ export default setModifierManager(

let [fn, ...positional] = args.positional;

assertFunction('did-update', fn);

fn(element, positional, args.named);
},

Expand Down
8 changes: 7 additions & 1 deletion addon/modifiers/will-destroy.js
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
},

Expand All @@ -56,6 +60,8 @@ export default setModifierManager(
destroyModifier({ element }, args) {
let [fn, ...positional] = args.positional;

assertFunction('did-destroy', fn);

fn(element, positional, args.named);
},
}),
Expand Down
32 changes: 31 additions & 1 deletion tests/integration/modifiers/did-insert-test.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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();
});
});
46 changes: 45 additions & 1 deletion tests/integration/modifiers/did-update-test.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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();
});
});
55 changes: 54 additions & 1 deletion tests/integration/modifiers/will-destroy-test.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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();
});
});

0 comments on commit 7c9b946

Please sign in to comment.