From 1038c1285fea4ca21dfc25a9467ffefaac3938b6 Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Tue, 5 May 2020 15:26:14 +0100 Subject: [PATCH 1/4] chore: remove src/externs.d.ts It defined global types that we don't want to use, and instead we move to using interfaces that we import and reference just like with any other interface. This means other than Protocol (which I think is fine to leave as is), there are no other magic global types and you have to import any types or interfaces that you want. --- package.json | 2 +- src/Browser.ts | 7 ++++--- src/Connection.ts | 19 ++++++------------- src/ConnectionTransport.ts | 22 ++++++++++++++++++++++ src/EmulationManager.ts | 3 ++- src/Launcher.ts | 8 +++++--- src/Page.ts | 11 ++++++----- src/PipeTransport.ts | 3 ++- src/Puppeteer.ts | 3 ++- src/Target.ts | 5 +++-- src/WebSocketTransport.ts | 3 ++- src/externs.d.ts | 21 --------------------- src/viewport.ts | 23 +++++++++++++++++++++++ 13 files changed, 78 insertions(+), 52 deletions(-) create mode 100644 src/ConnectionTransport.ts delete mode 100644 src/externs.d.ts create mode 100644 src/viewport.ts diff --git a/package.json b/package.json index fa65219079ba2..ae74c2c3117a0 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "eslint": "([ \"$CI\" = true ] && eslint --ext js --ext ts --quiet -f codeframe . || eslint --ext js --ext ts .)", "lint": "npm run eslint && npm run tsc && npm run doc", "doc": "node utils/doclint/cli.js", - "tsc": "tsc --version && tsc -p . && cp src/protocol.d.ts lib/ && cp src/externs.d.ts lib/", + "tsc": "tsc --version && tsc -p . && cp src/protocol.d.ts lib/", "apply-next-version": "node utils/apply_next_version.js", "test-types": "node utils/doclint/generate_types && tsc --version && tsc -p utils/doclint/generate_types/test/", "update-protocol-d-ts": "node utils/protocol-types-generator update", diff --git a/src/Browser.ts b/src/Browser.ts index 2df47f6799946..4ca05b425b67e 100644 --- a/src/Browser.ts +++ b/src/Browser.ts @@ -22,17 +22,18 @@ import {Events} from './Events'; import {Connection} from './Connection'; import {Page} from './Page'; import {ChildProcess} from 'child_process'; +import type {Viewport} from './Viewport'; type BrowserCloseCallback = () => Promise | void; export class Browser extends EventEmitter { - static async create(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: ChildProcess, closeCallback?: BrowserCloseCallback): Promise { + static async create(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Viewport, process?: ChildProcess, closeCallback?: BrowserCloseCallback): Promise { const browser = new Browser(connection, contextIds, ignoreHTTPSErrors, defaultViewport, process, closeCallback); await connection.send('Target.setDiscoverTargets', {discover: true}); return browser; } _ignoreHTTPSErrors: boolean; - _defaultViewport?: Puppeteer.Viewport; + _defaultViewport?: Viewport; _process?: ChildProcess; _screenshotTaskQueue = new TaskQueue(); _connection: Connection; @@ -42,7 +43,7 @@ export class Browser extends EventEmitter { // TODO: once Target is in TypeScript we can type this properly. _targets: Map; - constructor(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: ChildProcess, closeCallback?: BrowserCloseCallback) { + constructor(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Viewport, process?: ChildProcess, closeCallback?: BrowserCloseCallback) { super(); this._ignoreHTTPSErrors = ignoreHTTPSErrors; this._defaultViewport = defaultViewport; diff --git a/src/Connection.ts b/src/Connection.ts index f6bc2e4e02010..57e68684e3fac 100644 --- a/src/Connection.ts +++ b/src/Connection.ts @@ -14,14 +14,12 @@ * limitations under the License. */ import {assert} from './helper'; - -import EventsModule = require('./Events'); -const {Events} = EventsModule; - -import debug = require('debug'); +import {Events} from './Events'; +import * as debug from 'debug'; const debugProtocol = debug('puppeteer:protocol'); -import EventEmitter = require('events'); +import type {ConnectionTransport} from './ConnectionTransport'; +import * as EventEmitter from 'events'; interface ConnectionCallback { resolve: Function; @@ -33,7 +31,7 @@ interface ConnectionCallback { export class Connection extends EventEmitter { _url: string; - _transport: Puppeteer.ConnectionTransport; + _transport: ConnectionTransport; _delay: number; _lastId = 0; _sessions: Map = new Map(); @@ -41,12 +39,7 @@ export class Connection extends EventEmitter { _callbacks: Map = new Map(); - /** - * @param {string} url - * @param {!Puppeteer.ConnectionTransport} transport - * @param {number=} delay - */ - constructor(url: string, transport: Puppeteer.ConnectionTransport, delay = 0) { + constructor(url: string, transport: ConnectionTransport, delay = 0) { super(); this._url = url; this._delay = delay; diff --git a/src/ConnectionTransport.ts b/src/ConnectionTransport.ts new file mode 100644 index 0000000000000..fc3deddb40cdf --- /dev/null +++ b/src/ConnectionTransport.ts @@ -0,0 +1,22 @@ +/** + * Copyright 2020 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export interface ConnectionTransport { + send(string); + close(); + onmessage?: (message: string) => void; + onclose?: () => void; +} diff --git a/src/EmulationManager.ts b/src/EmulationManager.ts index 07a78a2b71917..58f043a4e4e5c 100644 --- a/src/EmulationManager.ts +++ b/src/EmulationManager.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import {CDPSession} from './Connection'; +import type {Viewport} from './Viewport'; export class EmulationManager { _client: CDPSession; @@ -24,7 +25,7 @@ export class EmulationManager { this._client = client; } - async emulateViewport(viewport: Puppeteer.Viewport): Promise { + async emulateViewport(viewport: Viewport): Promise { const mobile = viewport.isMobile || false; const width = viewport.width; const height = viewport.height; diff --git a/src/Launcher.ts b/src/Launcher.ts index dfb67d7cd7cee..015fe9b7edaaf 100644 --- a/src/Launcher.ts +++ b/src/Launcher.ts @@ -30,8 +30,10 @@ import {Connection} from './Connection'; import {Browser} from './Browser'; import {helper, assert, debugError} from './helper'; import {TimeoutError} from './Errors'; +import type {ConnectionTransport} from './ConnectionTransport'; import {WebSocketTransport} from './WebSocketTransport'; import {PipeTransport} from './PipeTransport'; +import type {Viewport} from './Viewport'; const mkdtempAsync = helper.promisify(fs.mkdtemp); const removeFolderAsync = helper.promisify(removeFolder); @@ -67,7 +69,7 @@ export interface LaunchOptions { export interface BrowserOptions { ignoreHTTPSErrors?: boolean; - defaultViewport?: Puppeteer.Viewport; + defaultViewport?: Viewport; slowMo?: number; } @@ -344,7 +346,7 @@ class ChromeLauncher implements ProductLauncher { async connect(options: BrowserOptions & { browserWSEndpoint?: string; browserURL?: string; - transport?: Puppeteer.ConnectionTransport; + transport?: ConnectionTransport; }): Promise { const { browserWSEndpoint, @@ -451,7 +453,7 @@ class FirefoxLauncher implements ProductLauncher { async connect(options: BrowserOptions & { browserWSEndpoint?: string; browserURL?: string; - transport?: Puppeteer.ConnectionTransport; + transport?: ConnectionTransport; }): Promise { const { browserWSEndpoint, diff --git a/src/Page.ts b/src/Page.ts index 96e0d8f7d1376..ae4956c6d1a5f 100644 --- a/src/Page.ts +++ b/src/Page.ts @@ -30,6 +30,7 @@ import {Worker as PuppeteerWorker} from './Worker'; import {Browser, BrowserContext} from './Browser'; import {Target} from './Target'; import {createJSHandle, JSHandle, ElementHandle} from './JSHandle'; +import type {Viewport} from './Viewport'; import {Request as PuppeteerRequest, Response as PuppeteerResponse, Credentials} from './NetworkManager'; import {Accessibility} from './Accessibility'; import {TimeoutSettings} from './TimeoutSettings'; @@ -124,7 +125,7 @@ const paperFormats: Record = { } as const; export class Page extends EventEmitter { - static async create(client: CDPSession, target: Target, ignoreHTTPSErrors: boolean, defaultViewport: Puppeteer.Viewport | null, screenshotTaskQueue: TaskQueue): Promise { + static async create(client: CDPSession, target: Target, ignoreHTTPSErrors: boolean, defaultViewport: Viewport | null, screenshotTaskQueue: TaskQueue): Promise { const page = new Page(client, target, ignoreHTTPSErrors, screenshotTaskQueue); await page._initialize(); if (defaultViewport) @@ -146,7 +147,7 @@ export class Page extends EventEmitter { _pageBindings = new Map(); _coverage: Coverage; _javascriptEnabled = true; - _viewport: Puppeteer.Viewport | null; + _viewport: Viewport | null; _screenshotTaskQueue: TaskQueue; _workers = new Map(); // TODO: improve this typedef - it's a function that takes a file chooser or something? @@ -666,7 +667,7 @@ export class Page extends EventEmitter { await this._client.send('Page.bringToFront'); } - async emulate(options: {viewport: Puppeteer.Viewport; userAgent: string}): Promise { + async emulate(options: {viewport: Viewport; userAgent: string}): Promise { await Promise.all([ this.setViewport(options.viewport), this.setUserAgent(options.userAgent) @@ -712,14 +713,14 @@ export class Page extends EventEmitter { } } - async setViewport(viewport: Puppeteer.Viewport): Promise { + async setViewport(viewport: Viewport): Promise { const needsReload = await this._emulationManager.emulateViewport(viewport); this._viewport = viewport; if (needsReload) await this.reload(); } - viewport(): Puppeteer.Viewport | null { + viewport(): Viewport | null { return this._viewport; } diff --git a/src/PipeTransport.ts b/src/PipeTransport.ts index 167661c0cbd6d..a3bebb0bc6c4d 100644 --- a/src/PipeTransport.ts +++ b/src/PipeTransport.ts @@ -14,8 +14,9 @@ * limitations under the License. */ import {helper, debugError, PuppeteerEventListener} from './helper'; +import type {ConnectionTransport} from './ConnectionTransport'; -export class PipeTransport implements Puppeteer.ConnectionTransport { +export class PipeTransport implements ConnectionTransport { _pipeWrite: NodeJS.WritableStream; _pendingMessage: string; _eventListeners: PuppeteerEventListener[]; diff --git a/src/Puppeteer.ts b/src/Puppeteer.ts index 1c59b768d8f0b..dbcbd99c0da9b 100644 --- a/src/Puppeteer.ts +++ b/src/Puppeteer.ts @@ -17,6 +17,7 @@ import Launcher from './Launcher'; import type {LaunchOptions, ChromeArgOptions, BrowserOptions, ProductLauncher} from './Launcher'; import {BrowserFetcher, BrowserFetcherOptions} from './BrowserFetcher'; import {puppeteerErrors, PuppeteerErrors} from './Errors'; +import type {ConnectionTransport} from './ConnectionTransport'; import {devicesMap} from './DeviceDescriptors'; import type {DevicesMap} from './/DeviceDescriptors'; @@ -48,7 +49,7 @@ export class Puppeteer { connect(options: BrowserOptions & { browserWSEndpoint?: string; browserURL?: string; - transport?: Puppeteer.ConnectionTransport; + transport?: ConnectionTransport; product?: string; }): Promise { if (options.product) diff --git a/src/Target.ts b/src/Target.ts index 7069c18886f7c..417683ff2a9b8 100644 --- a/src/Target.ts +++ b/src/Target.ts @@ -20,6 +20,7 @@ import {Worker as PuppeteerWorker} from './Worker'; import {CDPSession} from './Connection'; import {TaskQueue} from './TaskQueue'; import {Browser, BrowserContext} from './Browser'; +import type {Viewport} from './Viewport'; export class Target { _targetInfo: Protocol.Target.TargetInfo; @@ -27,7 +28,7 @@ export class Target { _targetId: string; _sessionFactory: () => Promise; _ignoreHTTPSErrors: boolean; - _defaultViewport?: Puppeteer.Viewport; + _defaultViewport?: Viewport; _screenshotTaskQueue: TaskQueue; _pagePromise?: Promise; _workerPromise?: Promise; @@ -37,7 +38,7 @@ export class Target { _closedCallback: () => void; _isInitialized: boolean; - constructor(targetInfo: Protocol.Target.TargetInfo, browserContext: BrowserContext, sessionFactory: () => Promise, ignoreHTTPSErrors: boolean, defaultViewport: Puppeteer.Viewport | null, screenshotTaskQueue: TaskQueue) { + constructor(targetInfo: Protocol.Target.TargetInfo, browserContext: BrowserContext, sessionFactory: () => Promise, ignoreHTTPSErrors: boolean, defaultViewport: Viewport | null, screenshotTaskQueue: TaskQueue) { this._targetInfo = targetInfo; this._browserContext = browserContext; this._targetId = targetInfo.targetId; diff --git a/src/WebSocketTransport.ts b/src/WebSocketTransport.ts index c8a97fd408f5d..5e6e90c0b1481 100644 --- a/src/WebSocketTransport.ts +++ b/src/WebSocketTransport.ts @@ -14,8 +14,9 @@ * limitations under the License. */ import * as NodeWebSocket from 'ws'; +import type {ConnectionTransport} from './ConnectionTransport'; -export class WebSocketTransport implements Puppeteer.ConnectionTransport { +export class WebSocketTransport implements ConnectionTransport { static create(url: string): Promise { return new Promise((resolve, reject) => { const ws = new NodeWebSocket(url, [], { diff --git a/src/externs.d.ts b/src/externs.d.ts deleted file mode 100644 index f7e8531454736..0000000000000 --- a/src/externs.d.ts +++ /dev/null @@ -1,21 +0,0 @@ -import * as child_process from 'child_process'; - -declare global { - module Puppeteer { - export interface ConnectionTransport { - send(string); - close(); - onmessage?: (message: string) => void, - onclose?: () => void, - } - - export type Viewport = { - width: number; - height: number; - deviceScaleFactor?: number; - isMobile?: boolean; - isLandscape?: boolean; - hasTouch?: boolean; - } - } -} diff --git a/src/viewport.ts b/src/viewport.ts new file mode 100644 index 0000000000000..f626ce8d2bcf4 --- /dev/null +++ b/src/viewport.ts @@ -0,0 +1,23 @@ +/** + * Copyright 2020 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export interface Viewport { + width: number; + height: number; + deviceScaleFactor?: number; + isMobile?: boolean; + isLandscape?: boolean; + hasTouch?: boolean; +} From 3734f7c0c43d54926f577ff29fb1891718dee644 Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Tue, 5 May 2020 15:55:55 +0100 Subject: [PATCH 2/4] fix doclint --- utils/doclint/check_public_api/index.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index b80fd185b77e9..a9011a70bdff9 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -367,6 +367,22 @@ function compareDocumentations(actual, expected) { actualName: 'Array', expectedName: 'Array' }], + ['Method Page.emulate() options', { + actualName: 'Object', + expectedName: 'Viewport' + }], + ['Method Page.setViewport() options', { + actualName: 'Object', + expectedName: 'Viewport' + }], + ['Method Page.connect() options.defaultViewport', { + actualName: 'Object', + expectedName: 'Viewport' + }], + ['Method Page.launch() options.defaultViewport', { + actualName: 'Object', + expectedName: 'Viewport' + }], ['Method Page.goBack() options', { actualName: 'Object', expectedName: 'WaitForOptions' From 25e5dacb9de6ba3683029b75202dfc4781167573 Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Wed, 6 May 2020 11:46:17 +0100 Subject: [PATCH 3/4] Clearer naming --- src/Browser.ts | 2 +- src/EmulationManager.ts | 2 +- src/Launcher.ts | 2 +- src/Page.ts | 2 +- src/{viewport.ts => PuppeteerViewport.ts} | 0 src/Target.ts | 2 +- 6 files changed, 5 insertions(+), 5 deletions(-) rename src/{viewport.ts => PuppeteerViewport.ts} (100%) diff --git a/src/Browser.ts b/src/Browser.ts index 4ca05b425b67e..98afacf699536 100644 --- a/src/Browser.ts +++ b/src/Browser.ts @@ -22,7 +22,7 @@ import {Events} from './Events'; import {Connection} from './Connection'; import {Page} from './Page'; import {ChildProcess} from 'child_process'; -import type {Viewport} from './Viewport'; +import type {Viewport} from './PuppeteerViewport'; type BrowserCloseCallback = () => Promise | void; diff --git a/src/EmulationManager.ts b/src/EmulationManager.ts index 58f043a4e4e5c..9bc173e8402b3 100644 --- a/src/EmulationManager.ts +++ b/src/EmulationManager.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import {CDPSession} from './Connection'; -import type {Viewport} from './Viewport'; +import type {Viewport} from './PuppeteerViewport'; export class EmulationManager { _client: CDPSession; diff --git a/src/Launcher.ts b/src/Launcher.ts index 015fe9b7edaaf..4fa8998317eb5 100644 --- a/src/Launcher.ts +++ b/src/Launcher.ts @@ -33,7 +33,7 @@ import {TimeoutError} from './Errors'; import type {ConnectionTransport} from './ConnectionTransport'; import {WebSocketTransport} from './WebSocketTransport'; import {PipeTransport} from './PipeTransport'; -import type {Viewport} from './Viewport'; +import type {Viewport} from './PuppeteerViewport'; const mkdtempAsync = helper.promisify(fs.mkdtemp); const removeFolderAsync = helper.promisify(removeFolder); diff --git a/src/Page.ts b/src/Page.ts index ae4956c6d1a5f..529918a0ca4f9 100644 --- a/src/Page.ts +++ b/src/Page.ts @@ -30,7 +30,7 @@ import {Worker as PuppeteerWorker} from './Worker'; import {Browser, BrowserContext} from './Browser'; import {Target} from './Target'; import {createJSHandle, JSHandle, ElementHandle} from './JSHandle'; -import type {Viewport} from './Viewport'; +import type {Viewport} from './PuppeteerViewport'; import {Request as PuppeteerRequest, Response as PuppeteerResponse, Credentials} from './NetworkManager'; import {Accessibility} from './Accessibility'; import {TimeoutSettings} from './TimeoutSettings'; diff --git a/src/viewport.ts b/src/PuppeteerViewport.ts similarity index 100% rename from src/viewport.ts rename to src/PuppeteerViewport.ts diff --git a/src/Target.ts b/src/Target.ts index 417683ff2a9b8..6475d0fb6d31b 100644 --- a/src/Target.ts +++ b/src/Target.ts @@ -20,7 +20,7 @@ import {Worker as PuppeteerWorker} from './Worker'; import {CDPSession} from './Connection'; import {TaskQueue} from './TaskQueue'; import {Browser, BrowserContext} from './Browser'; -import type {Viewport} from './Viewport'; +import type {Viewport} from './PuppeteerViewport'; export class Target { _targetInfo: Protocol.Target.TargetInfo; From d88932341f15b7fff93c800e87078aeb87f5c55c Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Wed, 6 May 2020 14:08:22 +0100 Subject: [PATCH 4/4] fix doclint failures --- utils/doclint/check_public_api/index.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index a9011a70bdff9..16ad74a3e6e7f 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -367,11 +367,15 @@ function compareDocumentations(actual, expected) { actualName: 'Array', expectedName: 'Array' }], - ['Method Page.emulate() options', { + ['Method Page.emulate() options.viewport', { actualName: 'Object', expectedName: 'Viewport' }], - ['Method Page.setViewport() options', { + ['Method Page.setViewport() options.viewport', { + actualName: 'Object', + expectedName: 'Viewport' + }], + ['Method Page.setViewport() viewport', { actualName: 'Object', expectedName: 'Viewport' }], @@ -379,6 +383,14 @@ function compareDocumentations(actual, expected) { actualName: 'Object', expectedName: 'Viewport' }], + ['Method Puppeteer.connect() options.defaultViewport', { + actualName: 'Object', + expectedName: 'Viewport' + }], + ['Method Puppeteer.launch() options.defaultViewport', { + actualName: 'Object', + expectedName: 'Viewport' + }], ['Method Page.launch() options.defaultViewport', { actualName: 'Object', expectedName: 'Viewport'