Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions packages/@glimmer/runtime/lib/modifiers/on.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
CheckString,
CheckUndefined,
} from '@glimmer/debug';
import { buildUntouchableThis, localAssert } from '@glimmer/debug-util';
import { buildUntouchableThis } from '@glimmer/debug-util';
import { registerDestructor } from '@glimmer/destroyable';
import { setInternalModifierManager } from '@glimmer/manager';
import { valueForRef } from '@glimmer/reference';
Expand Down Expand Up @@ -57,17 +57,30 @@ export class OnModifierState {
updateListener(): void {
let { element, args, listener } = this;

localAssert(
args.positional[0],
'You must pass a valid DOM event name as the first argument to the `on` modifier'
);
let selector: string | undefined;
if (DEBUG) {
const el = this.element;
selector =
el.tagName.toLowerCase() +
(el.id ? `#${el.id}` : '') +
Array.from(el.classList)
.map((c) => `.${c}`)
.join('');
}

let arg0 = args.positional[0];
let eventName = check(
Comment thread
NullVoxPopuli marked this conversation as resolved.
valueForRef(args.positional[0]),
arg0 ? valueForRef(arg0) : undefined,
CheckString,
() => 'You must pass a valid DOM event name as the first argument to the `on` modifier'
);

if (DEBUG && !eventName) {
throw new Error(
`You must pass a valid DOM event name as the first argument to the \`on\` modifier on ${selector}`
);
}

let arg1 = args.positional[1];
let userProvidedCallback = check(
arg1 ? valueForRef(arg1) : undefined,
Expand All @@ -79,16 +92,17 @@ export class OnModifierState {
}
) as EventListener;

localAssert(
typeof userProvidedCallback === 'function',
`You must pass a function as the second argument to the \`on\` modifier; you passed ${
userProvidedCallback === null ? 'null' : typeof userProvidedCallback
}. While rendering:\n\n${args.positional[1]?.debugLabel ?? `{unlabeled value}`}`
);
if (DEBUG && typeof userProvidedCallback !== 'function') {
throw new Error(
`You must pass a function as the second argument to the \`on\` modifier; you passed ${
userProvidedCallback === null ? 'null' : typeof userProvidedCallback
}. While rendering:\n\n${args.positional[1]?.debugLabel ?? '(unknown)'} on ${selector}`
);
}

if (DEBUG && args.positional.length !== 2) {
throw new Error(
`You can only pass two positional arguments (event name and callback) to the \`on\` modifier, but you provided ${args.positional.length}. Consider using the \`fn\` helper to provide additional arguments to the \`on\` callback.`
`You can only pass two positional arguments (event name and callback) to the \`on\` modifier, but you provided ${args.positional.length}. Consider using the \`fn\` helper to provide additional arguments to the \`on\` callback on ${selector}`
);
}

Expand Down Expand Up @@ -124,7 +138,7 @@ export class OnModifierState {
throw new Error(
`You can only \`once\`, \`passive\` or \`capture\` named arguments to the \`on\` modifier, but you provided ${Object.keys(
extra
).join(', ')}.`
).join(', ')} on ${selector}`
);
}
} else {
Expand Down
50 changes: 50 additions & 0 deletions smoke-tests/scenarios/basic-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,56 @@ function basicTest(scenarios: Scenarios, appName: string) {
let allNodes = flattenTree(tree);
let names = allNodes.filter(n => n.type === 'component').map(n => n.name);
assert.true(names.includes('HelloWorld'), 'HelloWorld component name is preserved in the render tree (found: ' + names.join(', ') + ')');
});
});
`,
'on-modifier-error-test.gjs': `
Comment thread
NullVoxPopuli marked this conversation as resolved.
import { module, test } from 'qunit';
import { render, setupOnerror, resetOnerror } from '@ember/test-helpers';
import { setupRenderingTest } from 'ember-qunit';
import { on } from '@ember/modifier';

module('on modifier | error handling', function (hooks) {
setupRenderingTest(hooks);

hooks.afterEach(function () {
resetOnerror();
});

test('throws helpful error when callback is missing', async function (assert) {
assert.expect(1);
const noop = undefined;
setupOnerror((error) => {
assert.true(
/You must pass a function as the second argument to the \`on\` modifier/.test(error.message),
'Expected helpful error message, got: ' + error.message
);
});
await render(<template><div {{on "click" noop}}>Click</div></template>);
});

test('throws helpful error when event name is missing', async function (assert) {
assert.expect(1);
const noop = () => {};
setupOnerror((error) => {
assert.true(
/You must pass a valid DOM event name as the first argument to the \`on\` modifier/.test(error.message),
'Expected helpful error message, got: ' + error.message
);
});
await render(<template><div {{on}}>Click</div></template>);
});

test('error message includes element selector', async function (assert) {
assert.expect(1);
const noop = undefined;
setupOnerror((error) => {
assert.true(
/button#my-id\\.class1\\.class2/.test(error.message),
'Expected element selector in error, got: ' + error.message
);
});
await render(<template><button id="my-id" class="class1 class2" {{on "click" noop}}>Click</button></template>);
});
});
`,
Expand Down
Loading