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.
This commit is contained in:
Jack Franklin 2020-06-02 09:32:02 +01:00 committed by GitHub
parent 81e32481e9
commit 309d8115c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 23 additions and 202 deletions

View File

@ -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"

View File

@ -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",

View File

@ -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';

View File

@ -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';

View File

@ -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;

View File

@ -1,61 +0,0 @@
import mitt, { Emitter, EventType, Handler } from 'mitt';
export interface CommonEventEmitter {
on(event: EventType, handler: Handler): void;
off(event: EventType, handler: Handler): void;
/* To maintain parity with the built in NodeJS event emitter which uses removeListener
* rather than `off`.
* If you're implementing new code you should use `off`.
*/
removeListener(event: EventType, handler: Handler): void;
emit(event: EventType, eventData?: any): void;
once(event: EventType, handler: Handler): void;
listenerCount(event: string): number;
}
export class EventEmitter implements CommonEventEmitter {
private emitter: Emitter;
private listenerCounts = new Map<EventType, number>();
constructor() {
this.emitter = mitt(new Map());
}
on(event: EventType, handler: Handler): void {
this.emitter.on(event, handler);
const existingCounts = this.listenerCounts.get(event);
if (existingCounts) {
this.listenerCounts.set(event, existingCounts + 1);
} else {
this.listenerCounts.set(event, 1);
}
}
off(event: EventType, handler: Handler): void {
this.emitter.off(event, handler);
const existingCounts = this.listenerCounts.get(event);
this.listenerCounts.set(event, existingCounts - 1);
}
removeListener(event: EventType, handler: Handler): void {
this.off(event, handler);
}
emit(event: EventType, eventData?: any): void {
this.emitter.emit(event, eventData);
}
once(event: EventType, handler: Handler): void {
const onceHandler: Handler = (eventData) => {
handler(eventData);
this.off(event, onceHandler);
};
this.on(event, onceHandler);
}
listenerCount(event: EventType): number {
return this.listenerCounts.get(event) || 0;
}
}

View File

@ -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';

View File

@ -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';

View File

@ -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';

View File

@ -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 {

View File

@ -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';

View File

@ -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,

View File

@ -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,

View File

@ -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';

View File

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

View File

@ -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);