From 12ee70f2cfe66cc630ae9e06872ed5c278a357ff Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Tue, 7 Jul 2020 16:43:55 +0100 Subject: [PATCH] chore: migrate NetworkManager events (#6174) This is part of the effort to remove `Events.ts` in favour of defining events next to the class that emits them. In this case these events are internal, so there's no docs changes, but it's still worth doing such that we can remove the Events.ts file in the long term once all the different events are migrated. --- src/common/Events.ts | 13 +++++++++++++ src/common/LifecycleWatcher.ts | 3 ++- src/common/NetworkManager.ts | 26 +++++++++++++++++++------- src/common/Page.ts | 14 +++++++------- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/common/Events.ts b/src/common/Events.ts index 318c4353072c1..90d8bae785933 100644 --- a/src/common/Events.ts +++ b/src/common/Events.ts @@ -14,6 +14,19 @@ * limitations under the License. */ +/** + * IMPORTANT: we are mid-way through migrating away from this Events.ts file + * in favour of defining events next to the class that emits them. + * + * However we need to maintain this file for now because the legacy DocLint + * system relies on them. Be aware in the mean time if you make a change here + * you probably need to replicate it in the relevant class. For example if you + * add a new Page event, you should update the PageEmittedEvents enum in + * src/common/Page.ts. + * + * Chat to @jackfranklin if you're unsure. + */ + export const Events = { Page: { Close: 'close', diff --git a/src/common/LifecycleWatcher.ts b/src/common/LifecycleWatcher.ts index 52eb995dd14c4..a1a6bb4a172bc 100644 --- a/src/common/LifecycleWatcher.ts +++ b/src/common/LifecycleWatcher.ts @@ -21,6 +21,7 @@ import { TimeoutError } from './Errors'; import { FrameManager, Frame } from './FrameManager'; import { HTTPRequest } from './HTTPRequest'; import { HTTPResponse } from './HTTPResponse'; +import { NetworkManagerEmittedEvents } from './NetworkManager'; export type PuppeteerLifeCycleEvent = | 'load' @@ -117,7 +118,7 @@ export class LifecycleWatcher { ), helper.addEventListener( this._frameManager.networkManager(), - Events.NetworkManager.Request, + NetworkManagerEmittedEvents.Request, this._onRequest.bind(this) ), ]; diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 3f3c829e8d6de..68eda72bf3476 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -17,7 +17,6 @@ import { EventEmitter } from './EventEmitter'; import { assert } from './assert'; import { helper, debugError } from './helper'; import Protocol from '../protocol'; -import { Events } from './Events'; import { CDPSession } from './Connection'; import { FrameManager } from './FrameManager'; import { HTTPRequest } from './HTTPRequest'; @@ -31,6 +30,19 @@ export interface Credentials { password: string; } +/** + * We use symbols to prevent any external parties listening to these events. + * They are internal to Puppeteer. + * + * @internal + */ +export const NetworkManagerEmittedEvents = { + Request: Symbol('NetworkManager.Request'), + Response: Symbol('NetworkManager.Response'), + RequestFailed: Symbol('NetworkManager.RequestFailed'), + RequestFinished: Symbol('NetworkManager.RequestFinished'), +} as const; + /** * @internal */ @@ -264,7 +276,7 @@ export class NetworkManager extends EventEmitter { redirectChain ); this._requestIdToRequest.set(event.requestId, request); - this.emit(Events.NetworkManager.Request, request); + this.emit(NetworkManagerEmittedEvents.Request, request); } _onRequestServedFromCache( @@ -286,8 +298,8 @@ export class NetworkManager extends EventEmitter { ); this._requestIdToRequest.delete(request._requestId); this._attemptedAuthentications.delete(request._interceptionId); - this.emit(Events.NetworkManager.Response, response); - this.emit(Events.NetworkManager.RequestFinished, request); + this.emit(NetworkManagerEmittedEvents.Response, response); + this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } _onResponseReceived(event: Protocol.Network.responseReceivedPayload): void { @@ -296,7 +308,7 @@ export class NetworkManager extends EventEmitter { if (!request) return; const response = new HTTPResponse(this._client, request, event.response); request._response = response; - this.emit(Events.NetworkManager.Response, response); + this.emit(NetworkManagerEmittedEvents.Response, response); } _onLoadingFinished(event: Protocol.Network.loadingFinishedPayload): void { @@ -310,7 +322,7 @@ export class NetworkManager extends EventEmitter { if (request.response()) request.response()._resolveBody(null); this._requestIdToRequest.delete(request._requestId); this._attemptedAuthentications.delete(request._interceptionId); - this.emit(Events.NetworkManager.RequestFinished, request); + this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } _onLoadingFailed(event: Protocol.Network.loadingFailedPayload): void { @@ -323,6 +335,6 @@ export class NetworkManager extends EventEmitter { if (response) response._resolveBody(null); this._requestIdToRequest.delete(request._requestId); this._attemptedAuthentications.delete(request._interceptionId); - this.emit(Events.NetworkManager.RequestFailed, request); + this.emit(NetworkManagerEmittedEvents.RequestFailed, request); } } diff --git a/src/common/Page.ts b/src/common/Page.ts index 10237ea79a45e..a927308c2a6ef 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -33,7 +33,7 @@ import { Browser, BrowserContext } from './Browser'; import { Target } from './Target'; import { createJSHandle, JSHandle, ElementHandle } from './JSHandle'; import { Viewport } from './PuppeteerViewport'; -import { Credentials } from './NetworkManager'; +import { Credentials, NetworkManagerEmittedEvents } from './NetworkManager'; import { HTTPRequest } from './HTTPRequest'; import { HTTPResponse } from './HTTPResponse'; import { Accessibility } from './Accessibility'; @@ -486,16 +486,16 @@ export class Page extends EventEmitter { ); const networkManager = this._frameManager.networkManager(); - networkManager.on(Events.NetworkManager.Request, (event) => + networkManager.on(NetworkManagerEmittedEvents.Request, (event) => this.emit(PageEmittedEvents.Request, event) ); - networkManager.on(Events.NetworkManager.Response, (event) => + networkManager.on(NetworkManagerEmittedEvents.Response, (event) => this.emit(PageEmittedEvents.Response, event) ); - networkManager.on(Events.NetworkManager.RequestFailed, (event) => + networkManager.on(NetworkManagerEmittedEvents.RequestFailed, (event) => this.emit(PageEmittedEvents.RequestFailed, event) ); - networkManager.on(Events.NetworkManager.RequestFinished, (event) => + networkManager.on(NetworkManagerEmittedEvents.RequestFinished, (event) => this.emit(PageEmittedEvents.RequestFinished, event) ); this._fileChooserInterceptors = new Set(); @@ -1339,7 +1339,7 @@ export class Page extends EventEmitter { const { timeout = this._timeoutSettings.timeout() } = options; return helper.waitForEvent( this._frameManager.networkManager(), - Events.NetworkManager.Request, + NetworkManagerEmittedEvents.Request, (request) => { if (helper.isString(urlOrPredicate)) return urlOrPredicate === request.url(); @@ -1359,7 +1359,7 @@ export class Page extends EventEmitter { const { timeout = this._timeoutSettings.timeout() } = options; return helper.waitForEvent( this._frameManager.networkManager(), - Events.NetworkManager.Response, + NetworkManagerEmittedEvents.Response, (response) => { if (helper.isString(urlOrPredicate)) return urlOrPredicate === response.url();