chore: Revert Mitt due to breaking changes (#5952)

Replacing the Node EventEmitter with Mitt caused more problems than anticipated for end users due to the API differences and the amount of people who relied on the EventEmitter API. In hindsight this clearly should have been explored more and then released as a breaking v4. This commit rolls us back to the built in Node EventEmitter library which we can release to get everyone back on stable builds. We can then consider our approach to migrating to Mitt and when we do do that we can release it as a breaking change and properly document the migration strategy and approach.
puppeteer · Jun 2, 2020 · 309d811 · 309d811
1 parent 81e3248
commit 309d811
Show file tree

Hide file tree

Showing 16 changed files with 23 additions and 202 deletions.
diff --git a/docs/api.md b/docs/api.md
@@ -335,13 +335,6 @@
   * [coverage.stopCSSCoverage()](#coveragestopcsscoverage)
   * [coverage.stopJSCoverage()](#coveragestopjscoverage)
 - [class: TimeoutError](#class-timeouterror)
-- [class: EventEmitter](#class-eventemitter)
-  * [eventEmitter.emit(event, [eventData])](#eventemitteremitevent-eventdata)
-  * [eventEmitter.listenerCount(event)](#eventemitterlistenercountevent)
-  * [eventEmitter.off(event, handler)](#eventemitteroffevent-handler)
-  * [eventEmitter.on(event, handler)](#eventemitteronevent-handler)
-  * [eventEmitter.once(event, handler)](#eventemitteronceevent-handler)
-  * [eventEmitter.removeListener(event, handler)](#eventemitterremovelistenerevent-handler)
 <!-- GEN:stop -->
 
 ### Overview
@@ -3942,35 +3935,6 @@ reported.
 
 TimeoutError is emitted whenever certain operations are terminated due to timeout, e.g. [page.waitForSelector(selector[, options])](#pagewaitforselectorselector-options) or [puppeteer.launch([options])](#puppeteerlaunchoptions).
 
-### class: EventEmitter
-
-A small EventEmitter class backed by [Mitt](https://github.com/developit/mitt/).
-
-#### eventEmitter.emit(event, [eventData])
-- `event` <[string]|[symbol]> the event to trigger.
-- `eventData` <[Object]> additional data to send with the event.
-
-#### eventEmitter.listenerCount(event)
-- `event` <[string]|[symbol]> the event to check for listeners.
-- returns: <[number]> the number of listeners for the given event.
-
-#### eventEmitter.off(event, handler)
-- `event` <[string]|[symbol]> the event to remove the handler from.
-- `handler` <[Function]> the event listener that will be removed.
-
-#### eventEmitter.on(event, handler)
-- `event` <[string]|[symbol]> the event to add the handler to.
-- `handler` <[Function]> the event listener that will be added.
-
-#### eventEmitter.once(event, handler)
-- `event` <[string]|[symbol]> the event to add the handler to.
-- `handler` <[Function]> the event listener that will be added.
-
-#### eventEmitter.removeListener(event, handler)
-- `event` <[string]|[symbol]> the event to remove the handler from.
-- `handler` <[Function]> the event listener that will be removed.
-
-This method is identical to `off` and maintained for compatibility with Node's EventEmitter. We recommend using `off` by default.
 
 
 [AXNode]: #accessibilitysnapshotoptions "AXNode"
@@ -4020,5 +3984,4 @@ This method is identical to `off` and maintained for compatibility with Node's E
 [selector]: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors "selector"
 [stream.Readable]: https://nodejs.org/api/stream.html#stream_class_stream_readable "stream.Readable"
 [string]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type "String"
-[symbol]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Symbol_type "Symbol"
 [xpath]: https://developer.mozilla.org/en-US/docs/Web/XPath "xpath"
diff --git a/package.json b/package.json
@@ -48,7 +48,6 @@
     "extract-zip": "^2.0.0",
     "https-proxy-agent": "^4.0.0",
     "mime": "^2.0.3",
-    "mitt": "^2.0.1",
     "progress": "^2.0.1",
     "proxy-from-env": "^1.0.0",
     "rimraf": "^3.0.2",

diff --git a/src/Browser.ts b/src/Browser.ts
@@ -16,7 +16,7 @@
 
 import { helper, assert } from './helper';
 import { Target } from './Target';
-import { EventEmitter } from './EventEmitter';
+import * as EventEmitter from 'events';
 import { Events } from './Events';
 import Protocol from './protocol';
 import { Connection } from './Connection';

diff --git a/src/BrowserFetcher.ts b/src/BrowserFetcher.ts
@@ -22,11 +22,11 @@ import * as childProcess from 'child_process';
 import * as https from 'https';
 import * as http from 'http';
 
-import extractZip from 'extract-zip';
-import debug from 'debug';
-import removeRecursive from 'rimraf';
+import * as extractZip from 'extract-zip';
+import * as debug from 'debug';
+import * as removeRecursive from 'rimraf';
 import * as URL from 'url';
-import ProxyAgent from 'https-proxy-agent';
+import * as ProxyAgent from 'https-proxy-agent';
 import { getProxyForUrl } from 'proxy-from-env';
 
 import { helper, assert } from './helper';

diff --git a/src/Connection.ts b/src/Connection.ts
@@ -15,12 +15,12 @@
  */
 import { assert } from './helper';
 import { Events } from './Events';
-import debug from 'debug';
+import * as debug from 'debug';
 const debugProtocol = debug('puppeteer:protocol');
 
 import Protocol from './protocol';
 import type { ConnectionTransport } from './ConnectionTransport';
-import { EventEmitter } from './EventEmitter';
+import * as EventEmitter from 'events';
 
 interface ConnectionCallback {
   resolve: Function;

diff --git a/src/EventEmitter.ts b/src/EventEmitter.ts
diff --git a/src/FrameManager.ts b/src/FrameManager.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { EventEmitter } from './EventEmitter';
+import * as EventEmitter from 'events';
 import { helper, assert, debugError } from './helper';
 import { Events } from './Events';
 import { ExecutionContext, EVALUATION_SCRIPT_URL } from './ExecutionContext';

diff --git a/src/NetworkManager.ts b/src/NetworkManager.ts
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { EventEmitter } from './EventEmitter';
+import * as EventEmitter from 'events';
 import { helper, assert, debugError } from './helper';
 import Protocol from './protocol';
 import { Events } from './Events';

diff --git a/src/Page.ts b/src/Page.ts
@@ -15,7 +15,7 @@
  */
 
 import * as fs from 'fs';
-import { EventEmitter } from './EventEmitter';
+import * as EventEmitter from 'events';
 import * as mime from 'mime';
 import { Events } from './Events';
 import { Connection, CDPSession } from './Connection';

diff --git a/src/WebSocketTransport.ts b/src/WebSocketTransport.ts
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import NodeWebSocket from 'ws';
+import * as NodeWebSocket from 'ws';
 import type { ConnectionTransport } from './ConnectionTransport';
 
 export class WebSocketTransport implements ConnectionTransport {

diff --git a/src/WebWorker.ts b/src/WebWorker.ts
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { EventEmitter } from './EventEmitter';
+import { EventEmitter } from 'events';
 import { debugError } from './helper';
 import { ExecutionContext } from './ExecutionContext';
 import { JSHandle } from './JSHandle';

diff --git a/src/api.ts b/src/api.ts
@@ -29,7 +29,6 @@ module.exports = {
   Dialog: require('./Dialog').Dialog,
   ElementHandle: require('./JSHandle').ElementHandle,
   ExecutionContext: require('./ExecutionContext').ExecutionContext,
-  EventEmitter: require('./EventEmitter').EventEmitter,
   FileChooser: require('./FileChooser').FileChooser,
   Frame: require('./FrameManager').Frame,
   JSHandle: require('./JSHandle').JSHandle,

diff --git a/src/helper.ts b/src/helper.ts
@@ -14,12 +14,11 @@
  * limitations under the License.
  */
 import { TimeoutError } from './Errors';
-import debug from 'debug';
+import * as debug from 'debug';
 import * as fs from 'fs';
 import { CDPSession } from './Connection';
 import { promisify } from 'util';
 import Protocol from './protocol';
-import type { CommonEventEmitter } from './EventEmitter';
 
 const openAsync = promisify(fs.open);
 const writeAsync = promisify(fs.write);
@@ -132,13 +131,13 @@ function installAsyncStackHooks(classType: AnyClass): void {
 }
 
 export interface PuppeteerEventListener {
-  emitter: CommonEventEmitter;
+  emitter: NodeJS.EventEmitter;
   eventName: string | symbol;
   handler: (...args: any[]) => void;
 }
 
 function addEventListener(
-  emitter: CommonEventEmitter,
+  emitter: NodeJS.EventEmitter,
   eventName: string | symbol,
   handler: (...args: any[]) => void
 ): PuppeteerEventListener {
@@ -148,7 +147,7 @@ function addEventListener(
 
 function removeEventListeners(
   listeners: Array<{
-    emitter: CommonEventEmitter;
+    emitter: NodeJS.EventEmitter;
     eventName: string | symbol;
     handler: (...args: any[]) => void;
   }>
@@ -167,7 +166,7 @@ function isNumber(obj: unknown): obj is number {
 }
 
 async function waitForEvent<T extends any>(
-  emitter: CommonEventEmitter,
+  emitter: NodeJS.EventEmitter,
   eventName: string | symbol,
   predicate: (event: T) => boolean,
   timeout: number,

diff --git a/src/launcher/BrowserRunner.ts b/src/launcher/BrowserRunner.ts
@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-import debug from 'debug';
+import * as debug from 'debug';
 
-import removeFolder from 'rimraf';
+import * as removeFolder from 'rimraf';
 import * as childProcess from 'child_process';
 import { helper, assert, debugError } from '../helper';
 import type { LaunchOptions } from './LaunchOptions';

diff --git a/tsconfig.json b/tsconfig.json
@@ -5,8 +5,7 @@
     "outDir": "./lib",
     "target": "ESNext",
     "moduleResolution": "node",
-    "module": "CommonJS",
-    "esModuleInterop": true
+    "module": "CommonJS"
   },
   "include": [
     "src"

diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js
@@ -180,31 +180,6 @@ const expectedNonExistingMethods = new Map([
   ['Page', new Set(['emulateMedia'])],
 ]);
 
-/* Methods that are defined in code but are not documented */
-const expectedNotFoundMethods = new Map([
-  /* all the methods from our EventEmitter that we don't document for each subclass */
-  [
-    'Browser',
-    new Set(['emit', 'listenerCount', 'off', 'on', 'once', 'removeListener']),
-  ],
-  [
-    'BrowserContext',
-    new Set(['emit', 'listenerCount', 'off', 'on', 'once', 'removeListener']),
-  ],
-  [
-    'CDPSession',
-    new Set(['emit', 'listenerCount', 'off', 'on', 'once', 'removeListener']),
-  ],
-  [
-    'Page',
-    new Set(['emit', 'listenerCount', 'off', 'on', 'once', 'removeListener']),
-  ],
-  [
-    'WebWorker',
-    new Set(['emit', 'listenerCount', 'off', 'on', 'once', 'removeListener']),
-  ],
-]);
-
 /**
  * @param {!Documentation} actual
  * @param {!Documentation} expected
@@ -230,24 +205,14 @@ function compareDocumentations(actual, expected) {
     const methodDiff = diff(actualMethods, expectedMethods);
 
     for (const methodName of methodDiff.extra) {
-      const nonExistingMethodsForClass = expectedNonExistingMethods.get(
-        className
-      );
-      if (
-        nonExistingMethodsForClass &&
-        nonExistingMethodsForClass.has(methodName)
-      )
+      const missingMethodsForClass = expectedNonExistingMethods.get(className);
+      if (missingMethodsForClass && missingMethodsForClass.has(methodName))
         continue;
 
       errors.push(`Non-existing method found: ${className}.${methodName}()`);
     }
-
-    for (const methodName of methodDiff.missing) {
-      const missingMethodsForClass = expectedNotFoundMethods.get(className);
-      if (missingMethodsForClass && missingMethodsForClass.has(methodName))
-        continue;
+    for (const methodName of methodDiff.missing)
       errors.push(`Method not found: ${className}.${methodName}()`);
-    }
 
     for (const methodName of methodDiff.equal) {
       const actualMethod = actualClass.methods.get(methodName);
@@ -647,48 +612,6 @@ function compareDocumentations(actual, expected) {
           expectedName: 'VisionDeficiency',
         },
       ],
-      [
-        'Method EventEmitter.emit() event',
-        {
-          actualName: 'string|symbol',
-          expectedName: 'Object',
-        },
-      ],
-      [
-        'Method EventEmitter.listenerCount() event',
-        {
-          actualName: 'string|symbol',
-          expectedName: 'Object',
-        },
-      ],
-      [
-        'Method EventEmitter.off() event',
-        {
-          actualName: 'string|symbol',
-          expectedName: 'Object',
-        },
-      ],
-      [
-        'Method EventEmitter.on() event',
-        {
-          actualName: 'string|symbol',
-          expectedName: 'Object',
-        },
-      ],
-      [
-        'Method EventEmitter.once() event',
-        {
-          actualName: 'string|symbol',
-          expectedName: 'Object',
-        },
-      ],
-      [
-        'Method EventEmitter.removeListener() event',
-        {
-          actualName: 'string|symbol',
-          expectedName: 'Object',
-        },
-      ],
     ]);
 
     const expectedForSource = expectedNamingMismatches.get(source);