Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jackfranklin committed Apr 21, 2020
1 parent 376d234 commit a614bc4
Show file tree
Hide file tree
Showing 19 changed files with 188 additions and 108 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.js
Expand Up @@ -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
}
}
]
Expand Down
6 changes: 5 additions & 1 deletion src/Accessibility.js
Expand Up @@ -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
Expand Down Expand Up @@ -53,7 +57,7 @@

class Accessibility {
/**
* @param {!Puppeteer.CDPSession} client
* @param {!CDPSession} client
*/
constructor(client) {
this._client = client;
Expand Down
9 changes: 6 additions & 3 deletions src/Browser.js
Expand Up @@ -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<string>} contextIds
* @param {boolean} ignoreHTTPSErrors
* @param {?Puppeteer.Viewport} defaultViewport
Expand All @@ -36,7 +39,7 @@ class Browser extends EventEmitter {
}

/**
* @param {!Puppeteer.Connection} connection
* @param {!Connection} connection
* @param {!Array<string>} contextIds
* @param {boolean} ignoreHTTPSErrors
* @param {?Puppeteer.Viewport} defaultViewport
Expand Down Expand Up @@ -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
*/
Expand Down
138 changes: 71 additions & 67 deletions src/Connection.js → src/Connection.ts
Expand Up @@ -13,84 +13,82 @@
* 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<string, CDPSession> = new Map();
_closed = false;

_callbacks: Map<number, ConnectionCallback> = 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<number, {resolve: function, reject: function, error: !Error, method: string}>}*/
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<string, !CDPSession>}*/
this._sessions = new Map();
this._closed = false;
}

/**
* @param {!CDPSession} session
* @return {!Connection}
*/
static fromSession(session) {
static fromSession(session: CDPSession): Connection {
return session._connection;
}

/**
* @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<?Object>}
*/
send(method, params = {}) {
send<ReturnType extends {}>(method: string, params = {}): Promise<ReturnType> {
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);
this._transport.send(message);
return id;
}

/**
* @param {string} message
*/
async _onMessage(message) {
async _onMessage(message: string): Promise<void> {
if (this._delay)
await new Promise(f => setTimeout(f, this._delay));
debugProtocol('◀ RECV ' + message);
Expand Down Expand Up @@ -125,7 +123,7 @@ class Connection extends EventEmitter {
}
}

_onClose() {
_onClose(): void {
if (this._closed)
return;
this._closed = true;
Expand All @@ -140,7 +138,7 @@ class Connection extends EventEmitter {
this.emit(Events.Connection.Disconnected);
}

dispose() {
dispose(): void {
this._onClose();
this._transport.close();
}
Expand All @@ -149,45 +147,53 @@ class Connection extends EventEmitter {
* @param {Protocol.Target.TargetInfo} targetInfo
* @return {!Promise<!CDPSession>}
*/
async createSession(targetInfo) {
const {sessionId} = await this.send('Target.attachToTarget', {targetId: targetInfo.targetId, flatten: true});
async createSession(targetInfo: Protocol.Target.TargetInfo): Promise<CDPSession> {
const {sessionId} = await this.send<Protocol.Target.attachedToTargetPayload>('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<number, ConnectionCallback> = new Map();

constructor(connection: Connection, targetType: string, sessionId: string) {
super();
/** @type {!Map<number, {resolve: function, reject: function, error: !Error, method: string}>}*/
this._callbacks = new Map();
this._connection = connection;
this._targetType = targetType;
this._sessionId = sessionId;
}

/**
* @param {string} method
* @param {!Object=} params
* @return {!Promise<?Object>}
*/
send(method, params = {}) {
send<T extends keyof Protocol.CommandParameters>(method: T, params?: Protocol.CommandParameters[T]): Promise<Protocol.CommandReturnValues[T]> {
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);
Expand All @@ -201,13 +207,13 @@ class CDPSession extends EventEmitter {
}
}

async detach() {
async detach(): Promise<void> {
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();
Expand All @@ -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}`;
Expand All @@ -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};
9 changes: 6 additions & 3 deletions src/Coverage.js
Expand Up @@ -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');

Expand All @@ -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);
Expand Down Expand Up @@ -67,7 +70,7 @@ module.exports = {Coverage};

class JSCoverage {
/**
* @param {!Puppeteer.CDPSession} client
* @param {!CDPSession} client
*/
constructor(client) {
this._client = client;
Expand Down Expand Up @@ -166,7 +169,7 @@ class JSCoverage {

class CSSCoverage {
/**
* @param {!Puppeteer.CDPSession} client
* @param {!CDPSession} client
*/
constructor(client) {
this._client = client;
Expand Down
5 changes: 3 additions & 2 deletions src/Dialog.ts
Expand Up @@ -15,6 +15,7 @@
*/

import helpers = require('./helper');
import {CDPSession} from './Connection';

const {assert} = helpers;

Expand All @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion src/EmulationManager.js
Expand Up @@ -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;
Expand Down

0 comments on commit a614bc4

Please sign in to comment.