Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: migrate src/Puppeteer to TypeScript #5789

Merged
merged 2 commits into from May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion index.js
Expand Up @@ -25,7 +25,7 @@ for (const className in api) {
// Expose alias for deprecated method.
Page.prototype.emulateMedia = Page.prototype.emulateMediaType;

const Puppeteer = require('./lib/Puppeteer');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed lib/Puppeteer to export as a named export which is the pattern the vast majority of files use in the codebase. It also plays nicer with TypeScript and outputting as CommonJS.

This does mean that if someone has const Puppeteer = require('puppeteer/lib/puppeteer') it would break, but I don't think we should consider that a breaking change (src/ should be considered internal + we could change its structure/files at any point).

@mathiasbynens WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found 3 matches on GitHub for this pattern: https://github.com/search?q=%22require%28%27puppeteer%2Flib%2FPuppeteer%27%29%22&type=Code but they don't look like active codebases.

Plus the change only requires adding braces around the require. We can call that out in the release notes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed we should be able to change this.

const {Puppeteer} = require('./lib/Puppeteer');
const packageJson = require('./package.json');
let preferredRevision = packageJson.puppeteer.chromium_revision;
const isPuppeteerCore = packageJson.name === 'puppeteer-core';
Expand Down
9 changes: 5 additions & 4 deletions src/Browser.ts
Expand Up @@ -20,18 +20,19 @@ import * as EventEmitter from 'events';
import {TaskQueue} from './TaskQueue';
import {Events} from './Events';
import {Connection} from './Connection';
import {ChildProcess} from 'child_process';

type BrowserCloseCallback = () => Promise<void> | void;

export class Browser extends EventEmitter {
static async create(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: Puppeteer.ChildProcess, closeCallback?: BrowserCloseCallback): Promise<Browser> {
static async create(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: ChildProcess, closeCallback?: BrowserCloseCallback): Promise<Browser> {
const browser = new Browser(connection, contextIds, ignoreHTTPSErrors, defaultViewport, process, closeCallback);
await connection.send('Target.setDiscoverTargets', {discover: true});
return browser;
}
_ignoreHTTPSErrors: boolean;
_defaultViewport?: Puppeteer.Viewport;
_process?: Puppeteer.ChildProcess;
_process?: ChildProcess;
_screenshotTaskQueue = new TaskQueue();
_connection: Connection;
_closeCallback: BrowserCloseCallback;
Expand All @@ -40,7 +41,7 @@ export class Browser extends EventEmitter {
// TODO: once Target is in TypeScript we can type this properly.
_targets: Map<string, Target>;

constructor(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: Puppeteer.ChildProcess, closeCallback?: BrowserCloseCallback) {
constructor(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: ChildProcess, closeCallback?: BrowserCloseCallback) {
super();
this._ignoreHTTPSErrors = ignoreHTTPSErrors;
this._defaultViewport = defaultViewport;
Expand All @@ -61,7 +62,7 @@ export class Browser extends EventEmitter {
this._connection.on('Target.targetInfoChanged', this._targetInfoChanged.bind(this));
}

process(): Puppeteer.ChildProcess | null {
process(): ChildProcess | null {
return this._process;
}

Expand Down
4 changes: 0 additions & 4 deletions src/BrowserFetcher.ts
Expand Up @@ -99,10 +99,6 @@ function existsAsync(filePath: string): Promise<boolean> {
});
}

/**
* @typedef {Object} BrowserFetcher.Options
*/

export interface BrowserFetcherOptions {
platform?: Platform;
product?: string;
Expand Down
5 changes: 2 additions & 3 deletions src/DeviceDescriptors.ts
Expand Up @@ -882,7 +882,7 @@ const devices: Device[] = [
}
];

type DevicesMap = {
export type DevicesMap = {
[name: string]: Device;
};

Expand All @@ -891,5 +891,4 @@ const devicesMap: DevicesMap = {};
for (const device of devices)
devicesMap[device.name] = device;


export = devicesMap;
export {devicesMap};
6 changes: 6 additions & 0 deletions src/Errors.ts
Expand Up @@ -23,3 +23,9 @@ class CustomError extends Error {
}

export class TimeoutError extends CustomError {}

export type PuppeteerErrors = Record<string, typeof CustomError>;

export const puppeteerErrors: PuppeteerErrors = {
TimeoutError,
};
5 changes: 1 addition & 4 deletions src/Launcher.ts
Expand Up @@ -375,9 +375,6 @@ class ChromeLauncher implements ProductLauncher {

}

/**
* @implements {!Puppeteer.ProductLauncher}
*/
class FirefoxLauncher implements ProductLauncher {
_projectRoot: string;
_preferredRevision: string;
Expand Down Expand Up @@ -738,7 +735,7 @@ class FirefoxLauncher implements ProductLauncher {
}


function waitForWSEndpoint(browserProcess: Puppeteer.ChildProcess, timeout: number, preferredRevision: string): Promise<string> {
function waitForWSEndpoint(browserProcess: childProcess.ChildProcess, timeout: number, preferredRevision: string): Promise<string> {
return new Promise((resolve, reject) => {
const rl = readline.createInterface({input: browserProcess.stderr});
let stderr = '';
Expand Down
174 changes: 0 additions & 174 deletions src/Puppeteer.js

This file was deleted.