events: improve argument handling, start passive

lib/internal/event_target.js

@@ -15,7 +15,7 @@ const {
     ERR_INVALID_ARG_TYPE,
     ERR_EVENT_RECURSION,
     ERR_OUT_OF_RANGE,
-    ERR_MISSING_ARGS
+    ERR_MISSING_ARGS,
   }
 } = require('internal/errors');
 
@@ -180,15 +180,31 @@ class EventTarget {
   [kRemoveListener](size, type, listener, capture) {}
 
   addEventListener(type, listener, options = {}) {
-    validateListener(listener);
-    type = String(type);
-
+    if (arguments.length < 2) {
+      throw new ERR_MISSING_ARGS('type', 'listener');
+    }
+    // We validateOptions before the shouldAddListeners check because the spec
+    // requires us to hit getters.
     const {
       once,
       capture,
       passive
     } = validateEventListenerOptions(options);
 
+    if (!shouldAddListener(listener)) {
+      // The DOM silently allows passing undefined as a second argument
+      // No error code for this since it is a Warning
+      // eslint-disable-next-line no-restricted-syntax
+      const w = new Error(`addEventListener called with ${listener}` +
+                          ' which has no effect.');
+      w.name = 'AddEventListenerArgumentTypeWarning';
+      w.target = this;
+      w.type = type;
+      process.emitWarning(w);
+      return;
+    }
+    type = String(type);
+
     let root = this[kEvents].get(type);
 
     if (root === undefined) {
@@ -219,9 +235,11 @@ class EventTarget {
   }
 
   removeEventListener(type, listener, options = {}) {
-    validateListener(listener);
+    if (!shouldAddListener(listener)) {
+      return;
+    }
     type = String(type);
-    const { capture } = validateEventListenerOptions(options);
+    const capture = !!(options?.capture);
     const root = this[kEvents].get(type);
     if (root === undefined || root.next === undefined)
       return;
@@ -402,12 +420,15 @@ Object.defineProperties(NodeEventTarget.prototype, {
 
 // EventTarget API
 
-function validateListener(listener) {
+function shouldAddListener(listener) {
   if (typeof listener === 'function' ||
       (listener != null &&
        typeof listener === 'object' &&
        typeof listener.handleEvent === 'function')) {
-    return;
+    return true;
+  }
+  if (listener === null || listener === undefined) {
+    return false;
   }
   throw new ERR_INVALID_ARG_TYPE('listener', 'EventListener', listener);
 }

test/parallel/test-eventtarget-whatwg-passive.js

@@ -0,0 +1,68 @@
+'use strict';
+
+const common = require('../common');
+
+const {
+  Event,
+  EventTarget,
+} = require('internal/event_target');
+
+const {
+  fail,
+  ok,
+  strictEqual
+} = require('assert');
+
+// Manually ported from WPT AddEventListenerOptions-passive.html
+{
+  const document = new EventTarget();
+  let supportsPassive = false;
+  const query_options = {
+    get passive() {
+      supportsPassive = true;
+      return false;
+    },
+    get dummy() {
+      fail('dummy value getter invoked');
+      return false;
+    }
+  };
+
+  document.addEventListener('test_event', null, query_options);
+  ok(supportsPassive);
+
+  supportsPassive = false;
+  document.removeEventListener('test_event', null, query_options);
+  strictEqual(supportsPassive, false);
+}
+{
+  function testPassiveValue(optionsValue, expectedDefaultPrevented) {
+    const document = new EventTarget();
+    let defaultPrevented;
+    function handler(e) {
+      if (e.defaultPrevented) {
+        fail('Event prematurely marked defaultPrevented');
+      }
+      e.preventDefault();
+      defaultPrevented = e.defaultPrevented;
+    }
+    document.addEventListener('test', handler, optionsValue);
+    // TODO the WHATWG test is more extensive here and tests dispatching on
+    // document.body, if we ever support getParent we should amend this
+    const ev = new Event('test', { bubbles: true, cancelable: true });
+    const uncanceled = document.dispatchEvent(ev);
+
+    strictEqual(defaultPrevented, expectedDefaultPrevented);
+    strictEqual(uncanceled, !expectedDefaultPrevented);
+
+    document.removeEventListener('test', handler, optionsValue);
+  }
+  testPassiveValue(undefined, true);
+  testPassiveValue({}, true);
+  testPassiveValue({ passive: false }, true);
+
+  common.skip('TODO: passive listeners is still broken');
+  testPassiveValue({ passive: 1 }, false);
+  testPassiveValue({ passive: true }, false);
+  testPassiveValue({ passive: 0 }, true);
+}

test/parallel/test-eventtarget.js

@@ -2,6 +2,9 @@
 'use strict';
 
 const common = require('../common');
+const { promisify } = require('util');
+const delay = promisify(setTimeout);
+
 const {
   Event,
   EventTarget,
@@ -16,11 +19,21 @@ const {
 } = require('assert');
 
 const { once } = require('events');
-
 // The globals are defined.
 ok(Event);
 ok(EventTarget);
 
+// The warning event has special behavior regarding attaching listeners
+let lastWarning;
+process.on('warning', (e) => {
+  lastWarning = e;
+});
+
+// Utility promise for parts of the test that need to wait for eachother -
+// Namely tests for warning events
+/* eslint-disable no-unused-vars */
+let asyncTest = Promise.resolve();
+
 // First, test Event
 {
   const ev = new Event('foo');
@@ -115,6 +128,43 @@ ok(EventTarget);
   const fn = common.mustCall((event) => strictEqual(event, ev));
   eventTarget.addEventListener('foo', fn, { once: true });
   eventTarget.dispatchEvent(ev);
+  const eventTarget = new EventTarget();
+  // Single argument throws
+  throws(() => eventTarget.addEventListener('foo'), TypeError);
+  // Null events - does not throw
+  eventTarget.addEventListener('foo', null);
+  eventTarget.removeEventListener('foo', null);
+  eventTarget.addEventListener('foo', undefined);
+  eventTarget.removeEventListener('foo', undefined);
+  // Strings, booleans
+  throws(() => eventTarget.addEventListener('foo', 'hello'), TypeError);
+  throws(() => eventTarget.addEventListener('foo', false), TypeError);
+  throws(() => eventTarget.addEventListener('foo', Symbol()), TypeError);
+  asyncTest = asyncTest.then(async () => {
+    const eventTarget = new EventTarget();
+    // Single argument throws
+    throws(() => eventTarget.addEventListener('foo'), TypeError);
+    // Null events - does not throw
+
+    eventTarget.addEventListener('foo', null);
+    eventTarget.removeEventListener('foo', null);
+
+    // Warnings always happen after nextTick, so wait for a timer of 0
+    await delay(0);
+    strictEqual(lastWarning.name, 'AddEventListenerArgumentTypeWarning');
+    strictEqual(lastWarning.target, eventTarget);
+    lastWarning = null;
+    eventTarget.addEventListener('foo', undefined);
+    await delay(0);
+    strictEqual(lastWarning.name, 'AddEventListenerArgumentTypeWarning');
+    strictEqual(lastWarning.target, eventTarget);
+    eventTarget.removeEventListener('foo', undefined);
+    // Strings, booleans
+    throws(() => eventTarget.addEventListener('foo', 'hello'), TypeError);
+    throws(() => eventTarget.addEventListener('foo', false), TypeError);
+    throws(() => eventTarget.addEventListener('foo', Symbol()), TypeError);
+  });
+
 }
 {
   const eventTarget = new NodeEventTarget();
@@ -309,8 +359,6 @@ ok(EventTarget);
     'foo',
     1,
     {},  // No handleEvent function
-    false,
-    undefined
   ].forEach((i) => {
     throws(() => target.addEventListener('foo', i), {
       code: 'ERR_INVALID_ARG_TYPE'
@@ -321,8 +369,6 @@ ok(EventTarget);
     'foo',
     1,
     {},  // No handleEvent function
-    false,
-    undefined
   ].forEach((i) => {
     throws(() => target.removeEventListener('foo', i), {
       code: 'ERR_INVALID_ARG_TYPE'
@@ -337,22 +383,23 @@ ok(EventTarget);
 }
 
 {
-  const target = new NodeEventTarget();
-
-  process.on('warning', common.mustCall((warning) => {
-    ok(warning instanceof Error);
-    strictEqual(warning.name, 'MaxListenersExceededWarning');
-    strictEqual(warning.target, target);
-    strictEqual(warning.count, 2);
-    strictEqual(warning.type, 'foo');
-    ok(warning.message.includes(
-      '2 foo listeners added to NodeEventTarget'));
-  }));
-
-  strictEqual(target.getMaxListeners(), NodeEventTarget.defaultMaxListeners);
-  target.setMaxListeners(1);
-  target.on('foo', () => {});
-  target.on('foo', () => {});
+  asyncTest = asyncTest.then(async () => {
+    const target = new NodeEventTarget();
+    strictEqual(target.getMaxListeners(), NodeEventTarget.defaultMaxListeners);
+    target.setMaxListeners(1);
+    target.on('foo', () => {});
+    target.on('foo', () => {});
+
+    // Warnings are always emitted asynchronously so wait for a tick
+    await delay(0);
+    ok(lastWarning instanceof Error);
+    strictEqual(lastWarning.name, 'MaxListenersExceededWarning');
+    strictEqual(lastWarning.target, target);
+    strictEqual(lastWarning.count, 2);
+    strictEqual(lastWarning.type, 'foo');
+    const warning = '2 foo listeners added to NodeEventTarget';
+    ok(lastWarning.message.includes(warning));
+  }).then(common.mustCall());
 }
 
 {