From 98a51f9ebf33affd5393f292f39fd8af309ad771 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Wed, 4 Aug 2021 15:36:29 +0200 Subject: [PATCH 1/8] refactor: convert puppeteer to TS --- .npmrc | 1 + package.json | 4 +- src/abstract-classes/browser-controller.ts | 4 +- src/index.js | 3 +- src/launch-context.ts | 2 + ...-controller.js => puppeteer-controller.ts} | 38 +++++----- src/puppeteer/puppeteer-plugin.js | 71 ------------------- src/puppeteer/puppeteer-plugin.ts | 66 +++++++++++++++++ src/utils.ts | 3 + test/browser-plugins/plugins.test.js | 5 +- test/browser-pool.test.js | 3 +- test/index.test.js | 3 +- 12 files changed, 102 insertions(+), 101 deletions(-) create mode 100644 .npmrc rename src/puppeteer/{puppeteer-controller.js => puppeteer-controller.ts} (60%) delete mode 100644 src/puppeteer/puppeteer-plugin.js create mode 100644 src/puppeteer/puppeteer-plugin.ts diff --git a/.npmrc b/.npmrc new file mode 100644 index 0000000..521a9f7 --- /dev/null +++ b/.npmrc @@ -0,0 +1 @@ +legacy-peer-deps=true diff --git a/package.json b/package.json index 6160332..9cdf523 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "ow": "^0.23.0", "proxy-chain": "^1.0.2", "tiny-typed-emitter": "^2.1.0", - "tslib": "^2.2.0" + "tslib": "^2.3.0" }, "devDependencies": { "@apify/eslint-config-ts": "^0.1.1", @@ -69,6 +69,6 @@ "puppeteer": "^9.0.0", "ts-jest": "^26.5.5", "ts-node": "^9.1.1", - "typescript": "^4.2.4" + "typescript": "^4.3.5" } } diff --git a/src/abstract-classes/browser-controller.ts b/src/abstract-classes/browser-controller.ts index ce3d7f4..3de4cc4 100644 --- a/src/abstract-classes/browser-controller.ts +++ b/src/abstract-classes/browser-controller.ts @@ -103,12 +103,12 @@ export abstract class BrowserController< /** * Browser representation of the underlying automation library. */ - browser?: LaunchResult = undefined; + browser: LaunchResult = undefined!; /** * The configuration the browser was launched with. */ - launchContext?: LaunchContext = undefined; + launchContext: LaunchContext = undefined!; isActive = false; diff --git a/src/index.js b/src/index.js index fccd713..e7dbaeb 100644 --- a/src/index.js +++ b/src/index.js @@ -1,5 +1,6 @@ const BrowserPool = require('./browser-pool'); -const PuppeteerPlugin = require('./puppeteer/puppeteer-plugin'); +// eslint-disable-next-line import/extensions +const { PuppeteerPlugin } = require('./puppeteer/puppeteer-plugin'); const PlaywrightPlugin = require('./playwright/playwright-plugin'); /** diff --git a/src/launch-context.ts b/src/launch-context.ts index cdb479c..c05e9fa 100644 --- a/src/launch-context.ts +++ b/src/launch-context.ts @@ -67,6 +67,8 @@ export class LaunchContext< private readonly _reservedFieldNames = [...Reflect.ownKeys(this), 'extend']; + [K: PropertyKey]: unknown; + constructor(options: LaunchContextOptions) { const { id, diff --git a/src/puppeteer/puppeteer-controller.js b/src/puppeteer/puppeteer-controller.ts similarity index 60% rename from src/puppeteer/puppeteer-controller.js rename to src/puppeteer/puppeteer-controller.ts index 241af88..d551bc8 100644 --- a/src/puppeteer/puppeteer-controller.js +++ b/src/puppeteer/puppeteer-controller.ts @@ -1,17 +1,15 @@ -const _ = require('lodash'); - -const { BrowserController } = require('../abstract-classes/browser-controller'); // eslint-disable-line import/extensions +import * as Puppeteer from 'puppeteer'; +import { BrowserController, Cookie } from '../abstract-classes/browser-controller'; +import { log } from '../logger'; +import { noop } from '../utils'; const PROCESS_KILL_TIMEOUT_MILLIS = 5000; -/** - * puppeteer - */ -class PuppeteerController extends BrowserController { - async _newPage() { - const { useIncognitoPages } = this.launchContext; - let page; - let context; +export class PuppeteerController extends BrowserController { + protected async _newPage(): Promise { + const { useIncognitoPages } = this.launchContext!; + let page: Puppeteer.Page; + let context: Puppeteer.BrowserContext; if (useIncognitoPages) { context = await this.browser.createIncognitoBrowserContext(); @@ -24,23 +22,23 @@ class PuppeteerController extends BrowserController { this.activePages--; if (useIncognitoPages) { - context.close().catch(_.noop); + context.close().catch(noop); } }); page.once('error', (error) => { - this.log.exception(error, 'Page crashed.'); - page.close().catch(_.noop); + log.exception(error, 'Page crashed.'); + page.close().catch(noop); }); return page; } - async _close() { + protected async _close(): Promise { await this.browser.close(); } - async _kill() { + protected async _kill(): Promise { const browserProcess = this.browser.process(); if (!browserProcess) { @@ -52,7 +50,7 @@ class PuppeteerController extends BrowserController { // This is here because users reported that it happened // that error `TypeError: Cannot read property 'kill' of null` was thrown. // Likely Chrome process wasn't started due to some error ... - if (browserProcess) browserProcess.kill('SIGKILL'); + browserProcess?.kill('SIGKILL'); }, PROCESS_KILL_TIMEOUT_MILLIS); try { @@ -63,13 +61,11 @@ class PuppeteerController extends BrowserController { } } - async _getCookies(page) { + protected _getCookies(page: Puppeteer.Page): Promise { return page.cookies(); } - async _setCookies(page, cookies) { + protected _setCookies(page: Puppeteer.Page, cookies: Cookie[]): Promise { return page.setCookie(...cookies); } } - -module.exports = PuppeteerController; diff --git a/src/puppeteer/puppeteer-plugin.js b/src/puppeteer/puppeteer-plugin.js deleted file mode 100644 index 6ee62d2..0000000 --- a/src/puppeteer/puppeteer-plugin.js +++ /dev/null @@ -1,71 +0,0 @@ -const { BrowserPlugin } = require('../abstract-classes/browser-plugin'); // eslint-disable-line import/extensions -const PuppeteerController = require('./puppeteer-controller'); - -const PROXY_SERVER_ARG = '--proxy-server='; - -/** - * puppeteer - */ -class PuppeteerPlugin extends BrowserPlugin { - /** - * @param {LaunchContext} launchContext - * @return {Promise} - * @private - */ - async _launch(launchContext) { - const { - launchOptions, - anonymizedProxyUrl, - userDataDir, - } = launchContext; - - const finalLaunchOptions = { - ...launchOptions, - userDataDir: launchOptions.userDataDir || userDataDir, - }; - - const browser = await this.library.launch(finalLaunchOptions); - - if (anonymizedProxyUrl) { - browser.once('disconnected', () => { - this._closeAnonymizedProxy(anonymizedProxyUrl); - }); - } - - return browser; - } - - /** - * @return {PuppeteerController} - * @private - */ - _createController() { - return new PuppeteerController(this); - } - - /** - * - * @param launchContext {object} - * @return {Promise} - * @private - */ - async _addProxyToLaunchOptions(launchContext) { - const { launchOptions, proxyUrl } = launchContext; - let finalProxyUrl = proxyUrl; - - if (this._shouldAnonymizeProxy(proxyUrl)) { - finalProxyUrl = await this._getAnonymizedProxyUrl(proxyUrl); - launchContext.anonymizedProxyUrl = finalProxyUrl; - } - - const proxyArg = `${PROXY_SERVER_ARG}${finalProxyUrl}`; - - if (Array.isArray(launchOptions.args)) { - launchOptions.args.push(proxyArg); - } else { - launchOptions.args = [proxyArg]; - } - } -} - -module.exports = PuppeteerPlugin; diff --git a/src/puppeteer/puppeteer-plugin.ts b/src/puppeteer/puppeteer-plugin.ts new file mode 100644 index 0000000..2ad5313 --- /dev/null +++ b/src/puppeteer/puppeteer-plugin.ts @@ -0,0 +1,66 @@ +import * as Puppeteer from 'puppeteer'; +import { BrowserController } from '../abstract-classes/browser-controller'; +import { BrowserPlugin } from '../abstract-classes/browser-plugin'; +import { LaunchContext } from '../launch-context'; +import { PuppeteerController } from './puppeteer-controller'; + +const PROXY_SERVER_ARG = '--proxy-server='; + +// Shortcut since Puppeteer doesn't export this as one type +export type PuppeteerLaunchOptions = Parameters[0] + +/** + * Puppeteer + */ +export class PuppeteerPlugin extends BrowserPlugin { + protected async _launch( + launchContext: LaunchContext, + ): Promise { + const { + launchOptions, + anonymizedProxyUrl, + userDataDir, + } = launchContext; + + const finalLaunchOptions = { + ...launchOptions, + userDataDir: launchOptions?.userDataDir ?? userDataDir, + }; + + const browser = await this.library.launch(finalLaunchOptions); + + if (anonymizedProxyUrl) { + browser.once('disconnected', () => { + this._closeAnonymizedProxy(anonymizedProxyUrl as string); + }); + } + + return browser; + } + + protected _createController(): BrowserController { + return new PuppeteerController(this); + } + + protected async _addProxyToLaunchOptions( + launchContext: LaunchContext, + ): Promise { + launchContext.launchOptions ??= {}; + + const { launchOptions, proxyUrl } = launchContext; + let finalProxyUrl = proxyUrl; + + if (proxyUrl && this._shouldAnonymizeProxy(proxyUrl)) { + finalProxyUrl = await this._getAnonymizedProxyUrl(proxyUrl); + launchContext.anonymizedProxyUrl = finalProxyUrl; + } + + const proxyArg = `${PROXY_SERVER_ARG}${finalProxyUrl}`; + + if (Array.isArray(launchOptions.args)) { + launchOptions.args.push(proxyArg); + } else { + launchOptions.args = [proxyArg]; + } + } +} diff --git a/src/utils.ts b/src/utils.ts index 8a53665..d29b03f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -13,3 +13,6 @@ export function addTimeoutToPromise(promise: Promise, timeoutMillis: numbe }; export type UnwrapPromise = T extends PromiseLike ? UnwrapPromise : T; + +// eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars +export function noop(..._args: unknown[]): void {} diff --git a/test/browser-plugins/plugins.test.js b/test/browser-plugins/plugins.test.js index 597afbd..4c56bd0 100644 --- a/test/browser-plugins/plugins.test.js +++ b/test/browser-plugins/plugins.test.js @@ -1,9 +1,10 @@ +/* eslint-disable import/extensions */ const puppeteer = require('puppeteer'); const playwright = require('playwright'); const fs = require('fs'); -const PuppeteerPlugin = require('../../src/puppeteer/puppeteer-plugin'); -const PuppeteerController = require('../../src/puppeteer/puppeteer-controller'); +const { PuppeteerPlugin } = require('../../src/puppeteer/puppeteer-plugin'); +const { PuppeteerController } = require('../../src/puppeteer/puppeteer-controller'); const PlaywrightPlugin = require('../../src/playwright/playwright-plugin'); const PlaywrightController = require('../../src/playwright/playwright-controller'); diff --git a/test/browser-pool.test.js b/test/browser-pool.test.js index 02b61f8..8901d03 100644 --- a/test/browser-pool.test.js +++ b/test/browser-pool.test.js @@ -1,7 +1,8 @@ +/* eslint-disable import/extensions */ const puppeteer = require('puppeteer'); const playwright = require('playwright'); const BrowserPool = require('../src/browser-pool'); -const PuppeteerPlugin = require('../src/puppeteer/puppeteer-plugin'); +const { PuppeteerPlugin } = require('../src/puppeteer/puppeteer-plugin'); const PlaywrightPlugin = require('../src/playwright/playwright-plugin'); const { BROWSER_POOL_EVENTS: { diff --git a/test/index.test.js b/test/index.test.js index 1271766..cee830c 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -1,5 +1,6 @@ +/* eslint-disable import/extensions */ const BrowserPool = require('../src/browser-pool'); -const PuppeteerPlugin = require('../src/puppeteer/puppeteer-plugin'); +const { PuppeteerPlugin } = require('../src/puppeteer/puppeteer-plugin'); const PlaywrightPlugin = require('../src/playwright/playwright-plugin'); const modules = require('../src/index'); From 5298b41e7a644998c8882b3edaa5ce3fc0d9a3ea Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Wed, 4 Aug 2021 15:43:16 +0200 Subject: [PATCH 2/8] fix: correct type for newPage --- src/abstract-classes/browser-controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/abstract-classes/browser-controller.ts b/src/abstract-classes/browser-controller.ts index 3de4cc4..7e5e0fd 100644 --- a/src/abstract-classes/browser-controller.ts +++ b/src/abstract-classes/browser-controller.ts @@ -204,7 +204,7 @@ export abstract class BrowserController< * Opens new browser page. * @ignore */ - async newPage(pageOptions: NewPageOptions): Promise { + async newPage(pageOptions?: NewPageOptions): Promise { this.activePages++; this.totalPages++; await this.isActivePromise; From 5cfe987ac1d7202e2f9998454bbf5c55495415d6 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Wed, 4 Aug 2021 15:56:26 +0200 Subject: [PATCH 3/8] chore: fix CI for now --- src/launch-context.ts | 3 ++- src/puppeteer/puppeteer-controller.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/launch-context.ts b/src/launch-context.ts index c05e9fa..f811650 100644 --- a/src/launch-context.ts +++ b/src/launch-context.ts @@ -67,7 +67,8 @@ export class LaunchContext< private readonly _reservedFieldNames = [...Reflect.ownKeys(this), 'extend']; - [K: PropertyKey]: unknown; + // TODO: change this to PropertyKey + [K: string]: unknown; constructor(options: LaunchContextOptions) { const { diff --git a/src/puppeteer/puppeteer-controller.ts b/src/puppeteer/puppeteer-controller.ts index d551bc8..f8e5f40 100644 --- a/src/puppeteer/puppeteer-controller.ts +++ b/src/puppeteer/puppeteer-controller.ts @@ -7,7 +7,7 @@ const PROCESS_KILL_TIMEOUT_MILLIS = 5000; export class PuppeteerController extends BrowserController { protected async _newPage(): Promise { - const { useIncognitoPages } = this.launchContext!; + const { useIncognitoPages } = this.launchContext; let page: Puppeteer.Page; let context: Puppeteer.BrowserContext; From 3b0c91a9b780974c912d6144c4da0541d3bbe3f3 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Wed, 4 Aug 2021 16:16:21 +0200 Subject: [PATCH 4/8] chore: CI pls From 8b4a9a04fcc85d1cfb32234dbafd45c46972cb67 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Wed, 4 Aug 2021 22:34:28 +0200 Subject: [PATCH 5/8] chore: cleanup generics used, let TS infer --- src/puppeteer/puppeteer-plugin.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/puppeteer/puppeteer-plugin.ts b/src/puppeteer/puppeteer-plugin.ts index 2ad5313..7056c27 100644 --- a/src/puppeteer/puppeteer-plugin.ts +++ b/src/puppeteer/puppeteer-plugin.ts @@ -6,16 +6,11 @@ import { PuppeteerController } from './puppeteer-controller'; const PROXY_SERVER_ARG = '--proxy-server='; -// Shortcut since Puppeteer doesn't export this as one type -export type PuppeteerLaunchOptions = Parameters[0] - /** * Puppeteer */ export class PuppeteerPlugin extends BrowserPlugin { - protected async _launch( - launchContext: LaunchContext, - ): Promise { + protected async _launch(launchContext: LaunchContext): Promise { const { launchOptions, anonymizedProxyUrl, @@ -38,12 +33,12 @@ export class PuppeteerPlugin extends BrowserPlugin { return browser; } - protected _createController(): BrowserController { + protected _createController(): BrowserController { return new PuppeteerController(this); } protected async _addProxyToLaunchOptions( - launchContext: LaunchContext, + launchContext: LaunchContext, ): Promise { launchContext.launchOptions ??= {}; From d09d66c5ca783f6d50ccfa661c718357265165fe Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Thu, 5 Aug 2021 09:11:59 +0200 Subject: [PATCH 6/8] docs: bring back randomly deleted comment --- src/puppeteer/puppeteer-controller.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/puppeteer/puppeteer-controller.ts b/src/puppeteer/puppeteer-controller.ts index f8e5f40..37e5c8f 100644 --- a/src/puppeteer/puppeteer-controller.ts +++ b/src/puppeteer/puppeteer-controller.ts @@ -5,6 +5,9 @@ import { noop } from '../utils'; const PROCESS_KILL_TIMEOUT_MILLIS = 5000; +/** + * Puppeteer + */ export class PuppeteerController extends BrowserController { protected async _newPage(): Promise { const { useIncognitoPages } = this.launchContext; From 444b3effa4c2ec88cbe08a4a40544f21d636304c Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 9 Aug 2021 10:39:59 +0200 Subject: [PATCH 7/8] chore: suggested changes Co-authored-by: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> --- src/launch-context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/launch-context.ts b/src/launch-context.ts index f811650..f931651 100644 --- a/src/launch-context.ts +++ b/src/launch-context.ts @@ -67,7 +67,7 @@ export class LaunchContext< private readonly _reservedFieldNames = [...Reflect.ownKeys(this), 'extend']; - // TODO: change this to PropertyKey + // TODO: change this to PropertyKey when TypeScript 4.4 releases [K: string]: unknown; constructor(options: LaunchContextOptions) { From 37baa78f3ec5bd354ca7b30d2b89fc5037ea5515 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 9 Aug 2021 11:29:16 +0200 Subject: [PATCH 8/8] chore: rerun CI since button doesn't work