EventEmitter: Enforce Function Type of Listener

Summary:
Currently, `EventEmitter#addListener` defines the type (via TypeScript and Flow) of its second argument as a non-nullable function, and the implementation of `EventEmitter#emit` depends on this.

However, not everyone may be using a static type system. We're seeing some people encounter a poor developer experience when they supply non-function values to `EventEmitter#addListener`, because the type mismatch is not surfaced as an error until `EventEmitter#emit` throws later on.

This changes `EventEmitter#addListener` so that it throws when the second argument is not a function. This will make it easier for developers to identify the problematic call site because the stack trace will include the source of the non-function `listener` parameter.

Changelog:
[General][Changed] - `EventEmitter#addListener` now throws if the 2nd argument is not a function.

Reviewed By: rshest

Differential Revision: D43705465

fbshipit-source-id: 34ca91ac92862b31435bd12b7c9cb6da870a2a32

Libraries/vendor/emitter/EventEmitter.js

@@ -74,6 +74,11 @@ export default class EventEmitter<TEventToArgsMap: {...}>
     listener: (...args: $ElementType<TEventToArgsMap, TEvent>) => mixed,
     context: mixed,
   ): EventSubscription {
+    if (typeof listener !== 'function') {
+      throw new TypeError(
+        'EventEmitter.addListener(...): 2nd argument must be a function.',
+      );
+    }
     const registrations = allocate<_, _, TEventToArgsMap[TEvent]>(
       this._registry,
       eventType,

Libraries/vendor/emitter/__tests__/EventEmitter-test.js

@@ -98,6 +98,39 @@ describe('listeners', () => {
     emitter.emit('A');
     expect(listener).toHaveBeenCalledTimes(3);
   });
+
+  it('throws on non-function listeners', () => {
+    const emitter = new EventEmitter<{A: []}>();
+
+    expect(() => {
+      emitter.addListener('A', () => {});
+    }).not.toThrow();
+
+    expect(() => {
+      // $FlowExpectedError
+      emitter.addListener('A', null);
+    }).toThrow();
+
+    expect(() => {
+      // $FlowExpectedError
+      emitter.addListener('A', undefined);
+    }).toThrow();
+
+    expect(() => {
+      // $FlowExpectedError
+      emitter.addListener('A', 'abc');
+    }).toThrow();
+
+    expect(() => {
+      // $FlowExpectedError
+      emitter.addListener('A', 123);
+    }).toThrow();
+
+    expect(() => {
+      // $FlowExpectedError
+      emitter.addListener('A', 123);
+    }).toThrow();
+  });
 });
 
 describe('arguments and context', () => {
@@ -122,17 +155,15 @@ describe('arguments and context', () => {
     const emitter = new EventEmitter<{A: []}>();
 
     const context = {};
-    let result;
-    /* $FlowFixMe[missing-this-annot] The 'this' type annotation(s) required by
-     * Flow's LTI update could not be added via codemod */
-    const listener = jest.fn(function () {
-      result = this;
+    let that;
+    const listener = jest.fn(function (this: mixed) {
+      that = this;
     });
     emitter.addListener('A', listener, context);
 
     emitter.emit('A');
     expect(listener).toHaveBeenCalled();
-    expect(result).toBe(context);
+    expect(that).toBe(context);
   });
 });