diff --git a/package.json b/package.json index e0ad5a2f9f1..b01ee55a34f 100644 --- a/package.json +++ b/package.json @@ -390,4 +390,4 @@ } }, "packageManager": "pnpm@10.5.0" -} +} \ No newline at end of file diff --git a/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js b/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js index 0799463916f..e7c25a768b9 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js @@ -2,6 +2,8 @@ import { moduleFor, RenderingTestCase, runTask } from 'internal-test-helpers'; import { getInternalModifierManager } from '@glimmer/manager'; import { on } from '@glimmer/runtime'; +import { DEBUG } from '@glimmer/env'; + import { Component } from '../../utils/helpers'; moduleFor( @@ -31,7 +33,7 @@ moduleFor( ); } - ['@test it adds an event listener'](assert) { + [`@test it adds an event listener`](assert) { let count = 0; this.render('', { @@ -242,6 +244,30 @@ moduleFor( this.assertCounts({ adds: 1, removes: 1 }); } + + [`@test it throws a helpful error when callback is undefined`](assert) { + if (DEBUG) { + let expectedMessage = + /You must pass a function as the second argument to the `on` modifier/; + assert.throws(() => { + this.render(''); + }, expectedMessage); + } else { + assert.expect(0); + } + } + + [`@test it throws a helpful error when callback is null`](assert) { + if (DEBUG) { + let expectedMessage = + /You must pass a function as the second argument to the `on` modifier/; + assert.throws(() => { + this.render(''); + }, expectedMessage); + } else { + assert.expect(0); + } + } } ); diff --git a/packages/@glimmer/runtime/lib/modifiers/on.ts b/packages/@glimmer/runtime/lib/modifiers/on.ts index 6f7c79b5c42..7c438230b19 100644 --- a/packages/@glimmer/runtime/lib/modifiers/on.ts +++ b/packages/@glimmer/runtime/lib/modifiers/on.ts @@ -68,17 +68,24 @@ export class OnModifierState { () => 'You must pass a valid DOM event name as the first argument to the `on` modifier' ); + let arg1 = args.positional[1]; + let userProvidedCallback = check( + arg1 ? valueForRef(arg1) : undefined, + CheckFunction, + (actual) => { + return `You must pass a function as the second argument to the \`on\` modifier; you passed ${ + actual === null ? 'null' : typeof actual + }. While rendering:\n\n${args.positional[1]?.debugLabel ?? `{unlabeled value}`}`; + } + ) as EventListener; + localAssert( - args.positional[1], - 'You must pass a function as the second argument to the `on` modifier' + 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}`}` ); - let userProvidedCallback = check(valueForRef(args.positional[1]), CheckFunction, (actual) => { - return `You must pass a function as the second argument to the \`on\` modifier; you passed ${ - actual === null ? 'null' : typeof actual - }. While rendering:\n\n${args.positional[1]?.debugLabel ?? `{unlabeled value}`}`; - }) as EventListener; - 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.`