Merge pull request #21108 from johanrd/fix/20783

[BUGFIX] Regression [Ember 5.12+] Non helpful error when the function for on modifier was forgot as param

Author: NullVoxPopuli

packages/@glimmer/runtime/lib/modifiers/on.ts

@@ -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';
@@ -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(
-      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,
@@ -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}`
       );
     }
 
@@ -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 {

smoke-tests/scenarios/basic-test.ts

@@ -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': `
+              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>);
                 });
               });
             `,