From a614bc45aade47a907000516e64203c08b5c001b Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Tue, 21 Apr 2020 09:20:25 +0100 Subject: [PATCH] chore: migrate `src/Connection` to TypeScript (#5694) * chore: migrate `src/Connection` to TypeScript This commit migrates `src/Connection` to TypeScript. It also changes its exports to be ESM because TypeScript's support for exporting values to use as types via CommonJS is poor (by design) and so rather than battle that it made more sense to migrate the file to ESM. The good news is that TypeScript is still outputting to `lib/` as CommonJS, so the fact that we author in ESM is actually not a breaking change at all. So going forwards we will: * migrate TS files to use ESM for importing and exporting * continue to output to `lib/` as CommonJS * continue to use CommonJS requires when in a `src/*.js` file I'd also like to split `Connection.ts` into two; I think the `CDPSession` class belongs in its own file, but I will do that in another PR to avoid this one becoming bigger than it already is. I also turned off `@typescript-eslint/no-use-before-define` as I don't think it was adding value and Puppeteer's codebase seems to have a style of declaring helper functions at the bottom which is fine by me. Finally, I updated the DocLint tool so it knows of expected method mismatches. It was either that or come up with a smart way to support TypeScript generics in DocLint and given we don't want to use DocLint that much longer that didn't feel worth it. * Fix params being required --- .eslintrc.js | 3 +- src/Accessibility.js | 6 +- src/Browser.js | 9 +- src/{Connection.js => Connection.ts} | 138 ++++++++++++------------ src/Coverage.js | 9 +- src/Dialog.ts | 5 +- src/EmulationManager.js | 6 +- src/ExecutionContext.js | 5 +- src/FrameManager.js | 9 +- src/Input.js | 9 +- src/JSHandle.js | 7 +- src/NetworkManager.js | 9 +- src/Page.js | 10 +- src/Target.js | 7 +- src/Tracing.js | 5 +- src/Worker.js | 5 +- src/externs.d.ts | 6 -- src/helper.ts | 5 +- utils/doclint/check_public_api/index.js | 43 +++++++- 19 files changed, 188 insertions(+), 108 deletions(-) rename src/{Connection.js => Connection.ts} (68%) diff --git a/.eslintrc.js b/.eslintrc.js index 6677501ea9f2f..ccf940586e9ae 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -112,7 +112,8 @@ module.exports = { "@typescript-eslint/no-unused-vars": 2, "semi": 0, "@typescript-eslint/semi": 2, - "@typescript-eslint/no-empty-function": 0 + "@typescript-eslint/no-empty-function": 0, + "@typescript-eslint/no-use-before-define": 0 } } ] diff --git a/src/Accessibility.js b/src/Accessibility.js index 204fec701e932..883e2038f0979 100644 --- a/src/Accessibility.js +++ b/src/Accessibility.js @@ -14,6 +14,10 @@ * limitations under the License. */ +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); + /** * @typedef {Object} SerializedAXNode * @property {string} role @@ -53,7 +57,7 @@ class Accessibility { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client */ constructor(client) { this._client = client; diff --git a/src/Browser.js b/src/Browser.js index 1b6d600fd3332..0592ee76164d1 100644 --- a/src/Browser.js +++ b/src/Browser.js @@ -19,10 +19,13 @@ const {Target} = require('./Target'); const EventEmitter = require('events'); const {TaskQueue} = require('./TaskQueue'); const {Events} = require('./Events'); +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {Connection} = require('./Connection'); class Browser extends EventEmitter { /** - * @param {!Puppeteer.Connection} connection + * @param {!Connection} connection * @param {!Array} contextIds * @param {boolean} ignoreHTTPSErrors * @param {?Puppeteer.Viewport} defaultViewport @@ -36,7 +39,7 @@ class Browser extends EventEmitter { } /** - * @param {!Puppeteer.Connection} connection + * @param {!Connection} connection * @param {!Array} contextIds * @param {boolean} ignoreHTTPSErrors * @param {?Puppeteer.Viewport} defaultViewport @@ -277,7 +280,7 @@ class Browser extends EventEmitter { class BrowserContext extends EventEmitter { /** - * @param {!Puppeteer.Connection} connection + * @param {!Connection} connection * @param {!Browser} browser * @param {?string} contextId */ diff --git a/src/Connection.js b/src/Connection.ts similarity index 68% rename from src/Connection.js rename to src/Connection.ts index bd9be3d49d3c5..cb55228469ff1 100644 --- a/src/Connection.js +++ b/src/Connection.ts @@ -13,38 +13,51 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -const {assert} = require('./helper'); -const {Events} = require('./Events'); -const debugProtocol = require('debug')('puppeteer:protocol'); -const EventEmitter = require('events'); +import helper = require('./helper'); +const {assert} = helper; + +import EventsModule = require('./Events'); +const {Events} = EventsModule; + +import debug = require('debug'); +const debugProtocol = debug('puppeteer:protocol'); + +import EventEmitter = require('events'); + +interface ConnectionCallback { + resolve: Function; + reject: Function; + error: Error; + method: string; + +} + +export class Connection extends EventEmitter { + _url: string; + _transport: Puppeteer.ConnectionTransport; + _delay: number; + _lastId = 0; + _sessions: Map = new Map(); + _closed = false; + + _callbacks: Map = new Map(); -class Connection extends EventEmitter { /** * @param {string} url * @param {!Puppeteer.ConnectionTransport} transport * @param {number=} delay */ - constructor(url, transport, delay = 0) { + constructor(url: string, transport: Puppeteer.ConnectionTransport, delay = 0) { super(); this._url = url; - this._lastId = 0; - /** @type {!Map}*/ - this._callbacks = new Map(); this._delay = delay; this._transport = transport; this._transport.onmessage = this._onMessage.bind(this); this._transport.onclose = this._onClose.bind(this); - /** @type {!Map}*/ - this._sessions = new Map(); - this._closed = false; } - /** - * @param {!CDPSession} session - * @return {!Connection} - */ - static fromSession(session) { + static fromSession(session: CDPSession): Connection { return session._connection; } @@ -52,34 +65,22 @@ class Connection extends EventEmitter { * @param {string} sessionId * @return {?CDPSession} */ - session(sessionId) { + session(sessionId: string): CDPSession | null { return this._sessions.get(sessionId) || null; } - /** - * @return {string} - */ - url() { + url(): string { return this._url; } - /** - * @param {string} method - * @param {!Object=} params - * @return {!Promise} - */ - send(method, params = {}) { + send(method: string, params = {}): Promise { const id = this._rawSend({method, params}); return new Promise((resolve, reject) => { this._callbacks.set(id, {resolve, reject, error: new Error(), method}); }); } - /** - * @param {*} message - * @return {number} - */ - _rawSend(message) { + _rawSend(message: {}): number { const id = ++this._lastId; message = JSON.stringify(Object.assign({}, message, {id})); debugProtocol('SEND ► ' + message); @@ -87,10 +88,7 @@ class Connection extends EventEmitter { return id; } - /** - * @param {string} message - */ - async _onMessage(message) { + async _onMessage(message: string): Promise { if (this._delay) await new Promise(f => setTimeout(f, this._delay)); debugProtocol('◀ RECV ' + message); @@ -125,7 +123,7 @@ class Connection extends EventEmitter { } } - _onClose() { + _onClose(): void { if (this._closed) return; this._closed = true; @@ -140,7 +138,7 @@ class Connection extends EventEmitter { this.emit(Events.Connection.Disconnected); } - dispose() { + dispose(): void { this._onClose(); this._transport.close(); } @@ -149,45 +147,53 @@ class Connection extends EventEmitter { * @param {Protocol.Target.TargetInfo} targetInfo * @return {!Promise} */ - async createSession(targetInfo) { - const {sessionId} = await this.send('Target.attachToTarget', {targetId: targetInfo.targetId, flatten: true}); + async createSession(targetInfo: Protocol.Target.TargetInfo): Promise { + const {sessionId} = await this.send('Target.attachToTarget', {targetId: targetInfo.targetId, flatten: true}); return this._sessions.get(sessionId); } } -class CDPSession extends EventEmitter { - /** - * @param {!Connection} connection - * @param {string} targetType - * @param {string} sessionId - */ - constructor(connection, targetType, sessionId) { +interface CDPSessionOnMessageObject { + id?: number; + method: string; + params: {}; + error: {message: string; data: any}; + result?: any; + +} +export class CDPSession extends EventEmitter { + _connection: Connection; + _sessionId: string; + _targetType: string; + _callbacks: Map = new Map(); + + constructor(connection: Connection, targetType: string, sessionId: string) { super(); - /** @type {!Map}*/ - this._callbacks = new Map(); this._connection = connection; this._targetType = targetType; this._sessionId = sessionId; } - /** - * @param {string} method - * @param {!Object=} params - * @return {!Promise} - */ - send(method, params = {}) { + send(method: T, params?: Protocol.CommandParameters[T]): Promise { if (!this._connection) return Promise.reject(new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`)); - const id = this._connection._rawSend({sessionId: this._sessionId, method, params}); + + const id = this._connection._rawSend({ + sessionId: this._sessionId, + method, + /* TODO(jacktfranklin@): once this Firefox bug is solved + * we no longer need the `|| {}` check + * https://bugzilla.mozilla.org/show_bug.cgi?id=1631570 + */ + params: params || {} + }); + return new Promise((resolve, reject) => { this._callbacks.set(id, {resolve, reject, error: new Error(), method}); }); } - /** - * @param {{id?: number, method: string, params: Object, error: {message: string, data: any}, result?: *}} object - */ - _onMessage(object) { + _onMessage(object: CDPSessionOnMessageObject): void { if (object.id && this._callbacks.has(object.id)) { const callback = this._callbacks.get(object.id); this._callbacks.delete(object.id); @@ -201,13 +207,13 @@ class CDPSession extends EventEmitter { } } - async detach() { + async detach(): Promise { if (!this._connection) throw new Error(`Session already detached. Most likely the ${this._targetType} has been closed.`); await this._connection.send('Target.detachFromTarget', {sessionId: this._sessionId}); } - _onClosed() { + _onClosed(): void { for (const callback of this._callbacks.values()) callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`)); this._callbacks.clear(); @@ -222,7 +228,7 @@ class CDPSession extends EventEmitter { * @param {{error: {message: string, data: any}}} object * @return {!Error} */ -function createProtocolError(error, method, object) { +function createProtocolError(error: Error, method: string, object: { error: { message: string; data: any}}): Error { let message = `Protocol error (${method}): ${object.error.message}`; if ('data' in object.error) message += ` ${object.error.data}`; @@ -234,9 +240,7 @@ function createProtocolError(error, method, object) { * @param {string} message * @return {!Error} */ -function rewriteError(error, message) { +function rewriteError(error: Error, message: string): Error { error.message = message; return error; } - -module.exports = {Connection, CDPSession}; diff --git a/src/Coverage.js b/src/Coverage.js index 23bae694ddf02..daa5c43e8c5d1 100644 --- a/src/Coverage.js +++ b/src/Coverage.js @@ -15,6 +15,9 @@ */ const {helper, debugError, assert} = require('./helper'); +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); const {EVALUATION_SCRIPT_URL} = require('./ExecutionContext'); @@ -27,7 +30,7 @@ const {EVALUATION_SCRIPT_URL} = require('./ExecutionContext'); class Coverage { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client */ constructor(client) { this._jsCoverage = new JSCoverage(client); @@ -67,7 +70,7 @@ module.exports = {Coverage}; class JSCoverage { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client */ constructor(client) { this._client = client; @@ -166,7 +169,7 @@ class JSCoverage { class CSSCoverage { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client */ constructor(client) { this._client = client; diff --git a/src/Dialog.ts b/src/Dialog.ts index 8e0e0c0be45db..91204f25574d4 100644 --- a/src/Dialog.ts +++ b/src/Dialog.ts @@ -15,6 +15,7 @@ */ import helpers = require('./helper'); +import {CDPSession} from './Connection'; const {assert} = helpers; @@ -28,13 +29,13 @@ enum DialogType { class Dialog { static Type = DialogType; - private _client: Puppeteer.CDPSession; + private _client: CDPSession; private _type: DialogType; private _message: string; private _defaultValue: string; private _handled = false; - constructor(client: Puppeteer.CDPSession, type: DialogType, message: string, defaultValue = '') { + constructor(client: CDPSession, type: DialogType, message: string, defaultValue = '') { this._client = client; this._type = type; this._message = message; diff --git a/src/EmulationManager.js b/src/EmulationManager.js index 88c8a021c8e0f..a29ea7016eb7c 100644 --- a/src/EmulationManager.js +++ b/src/EmulationManager.js @@ -14,9 +14,13 @@ * limitations under the License. */ +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); + class EmulationManager { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client */ constructor(client) { this._client = client; diff --git a/src/ExecutionContext.js b/src/ExecutionContext.js index d100d969c8332..3283d9f5cad62 100644 --- a/src/ExecutionContext.js +++ b/src/ExecutionContext.js @@ -16,13 +16,16 @@ const {helper, assert} = require('./helper'); const {createJSHandle, JSHandle} = require('./JSHandle'); +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); const EVALUATION_SCRIPT_URL = '__puppeteer_evaluation_script__'; const SOURCE_URL_REGEX = /^[\040\t]*\/\/[@#] sourceURL=\s*(\S*?)\s*$/m; class ExecutionContext { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Protocol.Runtime.ExecutionContextDescription} contextPayload * @param {?Puppeteer.DOMWorld} world */ diff --git a/src/FrameManager.js b/src/FrameManager.js index d391b6253f73d..30849c703ac15 100644 --- a/src/FrameManager.js +++ b/src/FrameManager.js @@ -24,12 +24,15 @@ const {NetworkManager} = require('./NetworkManager'); // Used as a TypeDef // eslint-disable-next-line no-unused-vars const {TimeoutSettings} = require('./TimeoutSettings'); +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); const UTILITY_WORLD_NAME = '__puppeteer_utility_world__'; class FrameManager extends EventEmitter { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Puppeteer.Page} page * @param {boolean} ignoreHTTPSErrors * @param {!TimeoutSettings} timeoutSettings @@ -112,7 +115,7 @@ class FrameManager extends EventEmitter { return watcher.navigationResponse(); /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {string} url * @param {string} referrer * @param {string} frameId @@ -376,7 +379,7 @@ class FrameManager extends EventEmitter { class Frame { /** * @param {!FrameManager} frameManager - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {?Frame} parentFrame * @param {string} frameId */ diff --git a/src/Input.js b/src/Input.js index e7d36a8d89e64..c95173791df97 100644 --- a/src/Input.js +++ b/src/Input.js @@ -16,6 +16,9 @@ const {assert} = require('./helper'); const keyDefinitions = require('./USKeyboardLayout'); +// CDPSession is used only as a typedef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); /** * @typedef {Object} KeyDescription @@ -28,7 +31,7 @@ const keyDefinitions = require('./USKeyboardLayout'); class Keyboard { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client */ constructor(client) { this._client = client; @@ -183,7 +186,7 @@ class Keyboard { class Mouse { /** - * @param {Puppeteer.CDPSession} client + * @param {CDPSession} client * @param {!Keyboard} keyboard */ constructor(client, keyboard) { @@ -274,7 +277,7 @@ class Mouse { class Touchscreen { /** - * @param {Puppeteer.CDPSession} client + * @param {CDPSession} client * @param {Keyboard} keyboard */ constructor(client, keyboard) { diff --git a/src/JSHandle.js b/src/JSHandle.js index 78d4295b2c953..d6fcd1edcec39 100644 --- a/src/JSHandle.js +++ b/src/JSHandle.js @@ -15,6 +15,9 @@ */ const {helper, assert, debugError} = require('./helper'); +// CDPSession is used only as a typedef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); function createJSHandle(context, remoteObject) { const frame = context.frame(); @@ -28,7 +31,7 @@ function createJSHandle(context, remoteObject) { class JSHandle { /** * @param {!Puppeteer.ExecutionContext} context - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Protocol.Runtime.RemoteObject} remoteObject */ constructor(context, client, remoteObject) { @@ -142,7 +145,7 @@ class JSHandle { class ElementHandle extends JSHandle { /** * @param {!Puppeteer.ExecutionContext} context - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Protocol.Runtime.RemoteObject} remoteObject * @param {!Puppeteer.Page} page * @param {!Puppeteer.FrameManager} frameManager diff --git a/src/NetworkManager.js b/src/NetworkManager.js index 9f333009fcdd4..c68cb7fc03f7f 100644 --- a/src/NetworkManager.js +++ b/src/NetworkManager.js @@ -16,10 +16,13 @@ const EventEmitter = require('events'); const {helper, assert, debugError} = require('./helper'); const {Events} = require('./Events'); +// CDPSession is used only as a typedef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); class NetworkManager extends EventEmitter { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Puppeteer.FrameManager} frameManager */ constructor(client, ignoreHTTPSErrors, frameManager) { @@ -312,7 +315,7 @@ class NetworkManager extends EventEmitter { class Request { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {?Puppeteer.Frame} frame * @param {string} interceptionId * @param {boolean} allowInterception @@ -524,7 +527,7 @@ const errorReasons = { class Response { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Request} request * @param {!Protocol.Network.Response} responsePayload */ diff --git a/src/Page.js b/src/Page.js index e06da26885ced..10046313ffeb5 100644 --- a/src/Page.js +++ b/src/Page.js @@ -18,7 +18,9 @@ const fs = require('fs'); const EventEmitter = require('events'); const mime = require('mime'); const {Events} = require('./Events'); -const {Connection} = require('./Connection'); +// CDPSession is used only as a typedef +// eslint-disable-next-line no-unused-vars +const {Connection, CDPSession} = require('./Connection'); const {Dialog} = require('./Dialog'); const {EmulationManager} = require('./EmulationManager'); const {FrameManager} = require('./FrameManager'); @@ -39,7 +41,7 @@ const writeFileAsync = helper.promisify(fs.writeFile); class Page extends EventEmitter { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Puppeteer.Target} target * @param {boolean} ignoreHTTPSErrors * @param {?Puppeteer.Viewport} defaultViewport @@ -55,7 +57,7 @@ class Page extends EventEmitter { } /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client * @param {!Puppeteer.Target} target * @param {boolean} ignoreHTTPSErrors * @param {!TaskQueue} screenshotTaskQueue @@ -1361,7 +1363,7 @@ class ConsoleMessage { class FileChooser { /** - * @param {Puppeteer.CDPSession} client + * @param {CDPSession} client * @param {Puppeteer.ElementHandle} element * @param {!Protocol.Page.fileChooserOpenedPayload} event */ diff --git a/src/Target.js b/src/Target.js index caf2627b44721..540e701d25faf 100644 --- a/src/Target.js +++ b/src/Target.js @@ -17,6 +17,9 @@ const {Events} = require('./Events'); const {Page} = require('./Page'); const {Worker: PuppeteerWorker} = require('./Worker'); +// CDPSession is used only as a typedef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); // This import is used as a TypeDef, but ESLint's rule doesn't // understand that unfortunately. // eslint-disable-next-line no-unused-vars @@ -26,7 +29,7 @@ class Target { /** * @param {!Protocol.Target.TargetInfo} targetInfo * @param {!Puppeteer.BrowserContext} browserContext - * @param {!function():!Promise} sessionFactory + * @param {!function():!Promise} sessionFactory * @param {boolean} ignoreHTTPSErrors * @param {?Puppeteer.Viewport} defaultViewport * @param {!TaskQueue} screenshotTaskQueue @@ -63,7 +66,7 @@ class Target { } /** - * @return {!Promise} + * @return {!Promise} */ createCDPSession() { return this._sessionFactory(); diff --git a/src/Tracing.js b/src/Tracing.js index fa2276e60b254..2f14b374dc6d9 100644 --- a/src/Tracing.js +++ b/src/Tracing.js @@ -14,10 +14,13 @@ * limitations under the License. */ const {helper, assert} = require('./helper'); +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); class Tracing { /** - * @param {!Puppeteer.CDPSession} client + * @param {!CDPSession} client */ constructor(client) { this._client = client; diff --git a/src/Worker.js b/src/Worker.js index 766116523e56a..2a750b2712812 100644 --- a/src/Worker.js +++ b/src/Worker.js @@ -17,10 +17,13 @@ const EventEmitter = require('events'); const {debugError} = require('./helper'); const {ExecutionContext} = require('./ExecutionContext'); const {JSHandle} = require('./JSHandle'); +// Used as a TypeDef +// eslint-disable-next-line no-unused-vars +const {CDPSession} = require('./Connection'); class Worker extends EventEmitter { /** - * @param {Puppeteer.CDPSession} client + * @param {CDPSession} client * @param {string} url * @param {function(string, !Array, Protocol.Runtime.StackTrace=):void} consoleAPICalled * @param {function(!Protocol.Runtime.ExceptionDetails):void} exceptionThrown diff --git a/src/externs.d.ts b/src/externs.d.ts index 97efb4e082824..7e2fb5728467e 100644 --- a/src/externs.d.ts +++ b/src/externs.d.ts @@ -1,4 +1,3 @@ -import { Connection as RealConnection, CDPSession as RealCDPSession } from './Connection.js'; import { Browser as RealBrowser, BrowserContext as RealBrowserContext} from './Browser.js'; import {Target as RealTarget} from './Target.js'; import {Page as RealPage} from './Page.js'; @@ -11,11 +10,6 @@ import { NetworkManager as RealNetworkManager, Request as RealRequest, Response import * as child_process from 'child_process'; declare global { module Puppeteer { - export class Connection extends RealConnection {} - export class CDPSession extends RealCDPSession { - on(event: T, listener: (arg: Protocol.Events[T]) => void): this; - send(message: T, parameters?: Protocol.CommandParameters[T]): Promise; - } export class Mouse extends RealMouse {} export class Keyboard extends RealKeyboard {} export class Touchscreen extends RealTouchscreen {} diff --git a/src/helper.ts b/src/helper.ts index c5788b8083d6a..b3e9e8fe4b977 100644 --- a/src/helper.ts +++ b/src/helper.ts @@ -19,6 +19,7 @@ import debug = require('debug'); const debugError = debug('puppeteer:error'); import fs = require('fs'); import promisify = require('./promisify'); +import {CDPSession} from './Connection'; const {TimeoutError} = Errors; @@ -71,7 +72,7 @@ function valueFromRemoteObject(remoteObject: Protocol.Runtime.RemoteObject): unk return remoteObject.value; } -async function releaseObject(client: Puppeteer.CDPSession, remoteObject: Protocol.Runtime.RemoteObject): Promise { +async function releaseObject(client: CDPSession, remoteObject: Protocol.Runtime.RemoteObject): Promise { if (!remoteObject.objectId) return; await client.send('Runtime.releaseObject', {objectId: remoteObject.objectId}).catch(error => { @@ -185,7 +186,7 @@ async function waitWithTimeout(promise: Promise, taskName: str } } -async function readProtocolStream(client: Puppeteer.CDPSession, handle: string, path?: string): Promise { +async function readProtocolStream(client: CDPSession, handle: string, path?: string): Promise { let eof = false; let file; if (path) diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index 8478d2e31ccbc..916a005dfa025 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -247,6 +247,41 @@ function compareDocumentations(actual, expected) { checkType(source + ' ' + actual.name, actual.type, expected.type); } + /** + * @param {string} source + * @param {!string} actualName + * @param {!string} expectedName + */ + function namingMisMatchInTypeIsExpected(source, actualName, expectedName) { + /* The DocLint tooling doesn't deal well with generics in TypeScript + * source files. We could fix this but the longterm plan is to + * auto-generate documentation from TS. So instead we document here + * the methods that use generics that DocLint trips up on and if it + * finds a mismatch that matches one of the cases below it doesn't + * error. This still means we're protected from accidental changes, as + * if the mismatch doesn't exactly match what's described below + * DocLint will fail. + */ + const expectedNamingMismatches = new Map([ + ['Method CDPSession.send() method', { + actualName: 'string', + expectedName: 'T' + }], + ['Method CDPSession.send() params', { + actualName: 'Object', + expectedName: 'CommandParameters[T]' + }], + ]); + + const expectedForSource = expectedNamingMismatches.get(source); + if (!expectedForSource) return false; + + const namingMismatchIsExpected = expectedForSource.actualName === actualName && expectedForSource.expectedName === expectedName; + + return namingMismatchIsExpected; + } + + /** * @param {string} source * @param {!Documentation.Type} actual @@ -260,8 +295,12 @@ function compareDocumentations(actual, expected) { const actualName = actual.name.replace(/[\? ]/g, ''); // TypeScript likes to add some spaces const expectedName = expected.name.replace(/\ /g, ''); - if (expectedName !== actualName) - errors.push(`${source} ${actualName} != ${expectedName}`); + if (expectedName !== actualName) { + const namingMismatchIsExpected = namingMisMatchInTypeIsExpected(source, actualName, expectedName); + + if (!namingMismatchIsExpected) + errors.push(`${source} ${actualName} != ${expectedName}`); + } const actualPropertiesMap = new Map(actual.properties.map(property => [property.name, property.type])); const expectedPropertiesMap = new Map(expected.properties.map(property => [property.name, property.type])); const propertiesDiff = diff(Array.from(actualPropertiesMap.keys()).sort(), Array.from(expectedPropertiesMap.keys()).sort());