From 465a7c405f01fcef99380ffa69d86042a1f5618f Mon Sep 17 00:00:00 2001 From: jrandolf <101637635+jrandolf@users.noreply.github.com> Date: Thu, 9 Jun 2022 19:00:50 +0200 Subject: [PATCH] feat: export puppeteer methods (#8493) --- .eslintignore | 1 - .gitignore | 1 - CONTRIBUTING.md | 2 -- cjs-entry-core.js | 29 ------------------- cjs-entry.js | 29 ------------------- package.json | 17 ++++------- scripts/test-install.sh | 18 ++++++++++++ src/.eslintrc.js | 19 ------------ src/common/LifecycleWatcher.ts | 8 ++--- src/common/Puppeteer.ts | 8 +++++ src/initialize-web.ts | 24 --------------- ...tialize-node.ts => initializePuppeteer.ts} | 20 ++++++------- src/node/Puppeteer.ts | 6 ++++ src/node/install.ts | 2 +- ...de-puppeteer-core.ts => puppeteer-core.ts} | 20 +++++++++---- src/{node.ts => puppeteer.ts} | 20 +++++++++---- src/web.ts | 25 ---------------- test-browser/connection.spec.js | 2 +- test/assert-coverage-test.js | 15 +++++++++- test/launcher.spec.ts | 9 ++++-- test/mocha-utils.ts | 2 +- utils/prepare_puppeteer_core.js | 6 ++-- 22 files changed, 105 insertions(+), 178 deletions(-) delete mode 100644 cjs-entry-core.js delete mode 100644 cjs-entry.js delete mode 100644 src/.eslintrc.js delete mode 100644 src/initialize-web.ts rename src/{initialize-node.ts => initializePuppeteer.ts} (79%) rename src/{node-puppeteer-core.ts => puppeteer-core.ts} (65%) rename src/{node.ts => puppeteer.ts} (65%) delete mode 100644 src/web.ts diff --git a/.eslintignore b/.eslintignore index a6257f0e56673..da45113a01b69 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,6 +1,5 @@ test/assets/modernizr.js third_party/* -utils/browser/puppeteer-web.js utils/doclint/check_public_api/test/ node6/* node6-test/* diff --git a/.gitignore b/.gitignore index ae3dfc539b1e6..0142fe48e125b 100644 --- a/.gitignore +++ b/.gitignore @@ -14,7 +14,6 @@ test-ts-types/**/dist/ package-lock.json yarn.lock /node6 -/utils/browser/puppeteer-web.js /lib test/coverage.json temp/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 162872110e741..cc09fb55a932d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -121,8 +121,6 @@ lib - vendor <== the output of compiling `vendor/tsconfig.esm.json` ``` -The main entry point for the Node module Puppeteer is `cjs-entry.js`. This imports `lib/cjs/puppeteer/index.js` and exposes it to Node users. - ### tsconfig for the tests We also maintain `test/tsconfig.test.json`. This is **only used to compile the unit test `*.spec.ts` files**. When the tests are run, we first compile Puppeteer as normal before running the unit tests **against the compiled output**. Doing this lets the test run against the compiled code we ship to users so it gives us more confidence in our compiled output being correct. diff --git a/cjs-entry-core.js b/cjs-entry-core.js deleted file mode 100644 index ad2e49c5f51b4..0000000000000 --- a/cjs-entry-core.js +++ /dev/null @@ -1,29 +0,0 @@ -/** - * 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. - */ - -/** - * We use `export default puppeteer` in `src/index.ts` to expose the library But - * TypeScript in CJS mode compiles that to `exports.default = `. This means that - * our CJS Node users would have to use `require('puppeteer').default` which - * isn't very nice. - * - * So instead we expose this file as our entry point. This requires the compiled - * Puppeteer output and re-exports the `default` export via `module.exports.` - * This means that we can publish to CJS and ESM whilst maintaining the expected - * import behaviour for CJS and ESM users. - */ -const puppeteerExport = require('./lib/cjs/puppeteer/node-puppeteer-core.js'); -module.exports = puppeteerExport.default; diff --git a/cjs-entry.js b/cjs-entry.js deleted file mode 100644 index 70c262a9b78d8..0000000000000 --- a/cjs-entry.js +++ /dev/null @@ -1,29 +0,0 @@ -/** - * 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. - */ - -/** - * We use `export default puppeteer` in `src/index.ts` to expose the library But - * TypeScript in CJS mode compiles that to `exports.default = `. This means that - * our CJS Node users would have to use `require('puppeteer').default` which - * isn't very nice. - * - * So instead we expose this file as our entry point. This requires the compiled - * Puppeteer output and re-exports the `default` export via `module.exports.` - * This means that we can publish to CJS and ESM whilst maintaining the expected - * import behaviour for CJS and ESM users. - */ -const puppeteerExport = require('./lib/cjs/puppeteer/node.js'); -module.exports = puppeteerExport.default; diff --git a/package.json b/package.json index 29ec871dd034a..c2f360854415b 100644 --- a/package.json +++ b/package.json @@ -9,12 +9,12 @@ "automation" ], "type": "commonjs", - "main": "./cjs-entry.js", + "main": "./lib/cjs/puppeteer/puppeteer.js", "exports": { ".": { "types": "./lib/types.d.ts", - "import": "./lib/esm/puppeteer/node.js", - "require": "./cjs-entry.js" + "import": "./lib/esm/puppeteer/puppeteer.js", + "require": "./lib/cjs/puppeteer/puppeteer.js" }, "./*": { "import": "./*", @@ -67,16 +67,9 @@ "build-docs-production": "cd website && npm install && npm run build" }, "files": [ - "lib/types.d.ts", - "lib/**/*.d.ts", - "lib/**/*.d.ts.map", - "lib/**/*.js", - "lib/**/*.js.map", - "lib/**/package.json", + "lib", "install.js", - "typescript-if-required.js", - "cjs-entry.js", - "cjs-entry-core.js" + "typescript-if-required.js" ], "author": "The Chromium Authors", "license": "Apache-2.0", diff --git a/scripts/test-install.sh b/scripts/test-install.sh index 44ab93a930e18..15d1a552dee6d 100755 --- a/scripts/test-install.sh +++ b/scripts/test-install.sh @@ -40,6 +40,24 @@ import puppeteer from 'puppeteer'; })(); " +echo "Testing... Chrome ES Modules Destructuring" +TMPDIR="$(mktemp -d)" +cd $TMPDIR +echo '{"type":"module"}' >>$TMPDIR/package.json +npm install --loglevel silent "${tarball}" +node --input-type="module" --eval="import puppeteer from 'puppeteer'" +node --input-type="module" --eval="import 'puppeteer/lib/esm/puppeteer/revisions.js';" +node --input-type="module" --eval=" +import { launch } from 'puppeteer'; +(async () => { + const browser = await launch(); + const page = await browser.newPage(); + await page.goto('http://example.com'); + await page.screenshot({ path: 'example.png' }); + await browser.close(); +})(); +" + echo "Testing... Chrome Webpack ES Modules" TMPDIR="$(mktemp -d)" cd $TMPDIR diff --git a/src/.eslintrc.js b/src/.eslintrc.js deleted file mode 100644 index 05bd5e99c707a..0000000000000 --- a/src/.eslintrc.js +++ /dev/null @@ -1,19 +0,0 @@ -module.exports = { - extends: '../.eslintrc.js', - /** - * ESLint rules - * - * All available rules: http://eslint.org/docs/rules/ - * - * Rules take the following form: - * "rule-name", [severity, \{ opts \}] - * Severity: 2 == error, 1 == warning, 0 == off. - */ - rules: { - 'no-console': [ - 2, - { allow: ['warn', 'error', 'assert', 'timeStamp', 'time', 'timeEnd'] }, - ], - 'no-debugger': 0, - }, -}; diff --git a/src/common/LifecycleWatcher.ts b/src/common/LifecycleWatcher.ts index c7898c91e940f..e62b8d03326b1 100644 --- a/src/common/LifecycleWatcher.ts +++ b/src/common/LifecycleWatcher.ts @@ -117,10 +117,10 @@ export class LifecycleWatcher { helper.addEventListener( frameManager._client, CDPSessionEmittedEvents.Disconnected, - () => - this._terminate( - new Error('Navigation failed because browser has disconnected!') - ) + this._terminate.bind( + this, + new Error('Navigation failed because browser has disconnected!') + ) ), helper.addEventListener( this._frameManager, diff --git a/src/common/Puppeteer.ts b/src/common/Puppeteer.ts index 6522dcc46825e..327ac10264902 100644 --- a/src/common/Puppeteer.ts +++ b/src/common/Puppeteer.ts @@ -66,6 +66,14 @@ export class Puppeteer { */ constructor(settings: CommonPuppeteerSettings) { this._isPuppeteerCore = settings.isPuppeteerCore; + + this.connect = this.connect.bind(this); + this.registerCustomQueryHandler = + this.registerCustomQueryHandler.bind(this); + this.unregisterCustomQueryHandler = + this.unregisterCustomQueryHandler.bind(this); + this.customQueryHandlerNames = this.customQueryHandlerNames.bind(this); + this.clearCustomQueryHandlers = this.clearCustomQueryHandlers.bind(this); } /** diff --git a/src/initialize-web.ts b/src/initialize-web.ts deleted file mode 100644 index 4ec42ce3fec5b..0000000000000 --- a/src/initialize-web.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * 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. - */ - -import { Puppeteer } from './common/Puppeteer.js'; - -export const initializePuppeteerWeb = (packageName: string): Puppeteer => { - const isPuppeteerCore = packageName === 'puppeteer-core'; - return new Puppeteer({ - isPuppeteerCore, - }); -}; diff --git a/src/initialize-node.ts b/src/initializePuppeteer.ts similarity index 79% rename from src/initialize-node.ts rename to src/initializePuppeteer.ts index b298a5f14d317..4846ba0dbf483 100644 --- a/src/initialize-node.ts +++ b/src/initializePuppeteer.ts @@ -14,22 +14,22 @@ * limitations under the License. */ -import { PuppeteerNode } from './node/Puppeteer.js'; -import { PUPPETEER_REVISIONS } from './revisions.js'; import { sync } from 'pkg-dir'; import { Product } from './common/Product.js'; import { rootDirname } from './constants.js'; +import { PuppeteerNode } from './node/Puppeteer.js'; +import { PUPPETEER_REVISIONS } from './revisions.js'; -export const initializePuppeteerNode = (packageName: string): PuppeteerNode => { +export const initializePuppeteer = (packageName: string): PuppeteerNode => { + const isPuppeteerCore = packageName === 'puppeteer-core'; const puppeteerRootDirectory = sync(rootDirname); let preferredRevision = PUPPETEER_REVISIONS.chromium; - const isPuppeteerCore = packageName === 'puppeteer-core'; // puppeteer-core ignores environment variables - const productName = isPuppeteerCore - ? undefined - : process.env['PUPPETEER_PRODUCT'] || - process.env['npm_config_puppeteer_product'] || - process.env['npm_package_config_puppeteer_product']; + const productName = !isPuppeteerCore + ? ((process.env['PUPPETEER_PRODUCT'] || + process.env['npm_config_puppeteer_product'] || + process.env['npm_package_config_puppeteer_product']) as Product) + : undefined; if (!isPuppeteerCore && productName === 'firefox') preferredRevision = PUPPETEER_REVISIONS.firefox; @@ -38,6 +38,6 @@ export const initializePuppeteerNode = (packageName: string): PuppeteerNode => { projectRoot: puppeteerRootDirectory, preferredRevision, isPuppeteerCore, - productName: productName as Product, + productName, }); }; diff --git a/src/node/Puppeteer.ts b/src/node/Puppeteer.ts index 58f86ce3600a6..58f04065df954 100644 --- a/src/node/Puppeteer.ts +++ b/src/node/Puppeteer.ts @@ -101,6 +101,12 @@ export class PuppeteerNode extends Puppeteer { this._projectRoot = projectRoot; this.__productName = productName; this._preferredRevision = preferredRevision; + + this.connect = this.connect.bind(this); + this.launch = this.launch.bind(this); + this.executablePath = this.executablePath.bind(this); + this.defaultArgs = this.defaultArgs.bind(this); + this.createBrowserFetcher = this.createBrowserFetcher.bind(this); } /** diff --git a/src/node/install.ts b/src/node/install.ts index 84c3bbf4d4cbb..6b841e5c13b4a 100644 --- a/src/node/install.ts +++ b/src/node/install.ts @@ -17,7 +17,7 @@ import https, { RequestOptions } from 'https'; import ProgressBar from 'progress'; import URL from 'url'; -import puppeteer from '../node.js'; +import puppeteer from '../puppeteer.js'; import { PUPPETEER_REVISIONS } from '../revisions.js'; import { PuppeteerNode } from './Puppeteer.js'; import createHttpsProxyAgent, { diff --git a/src/node-puppeteer-core.ts b/src/puppeteer-core.ts similarity index 65% rename from src/node-puppeteer-core.ts rename to src/puppeteer-core.ts index 23269bbc31814..e91c40521826f 100644 --- a/src/node-puppeteer-core.ts +++ b/src/puppeteer-core.ts @@ -14,12 +14,20 @@ * limitations under the License. */ -import { isNode } from './environment.js'; -import { initializePuppeteerNode } from './initialize-node.js'; +import { initializePuppeteer } from './initializePuppeteer.js'; -if (!isNode) { - throw new Error('Cannot run puppeteer-core outside of Node.js'); -} +const puppeteer = initializePuppeteer('puppeteer-core'); + +export const { + clearCustomQueryHandlers, + connect, + createBrowserFetcher, + customQueryHandlerNames, + defaultArgs, + executablePath, + launch, + registerCustomQueryHandler, + unregisterCustomQueryHandler, +} = puppeteer; -const puppeteer = initializePuppeteerNode('puppeteer-core'); export default puppeteer; diff --git a/src/node.ts b/src/puppeteer.ts similarity index 65% rename from src/node.ts rename to src/puppeteer.ts index b996436b9d54f..913e2fe3ed118 100644 --- a/src/node.ts +++ b/src/puppeteer.ts @@ -14,12 +14,20 @@ * limitations under the License. */ -import { isNode } from './environment.js'; -import { initializePuppeteerNode } from './initialize-node.js'; +import { initializePuppeteer } from './initializePuppeteer.js'; -if (!isNode) { - throw new Error('Trying to run Puppeteer-Node in a web environment.'); -} +const puppeteer = initializePuppeteer('puppeteer'); + +export const { + clearCustomQueryHandlers, + connect, + createBrowserFetcher, + customQueryHandlerNames, + defaultArgs, + executablePath, + launch, + registerCustomQueryHandler, + unregisterCustomQueryHandler, +} = puppeteer; -const puppeteer = initializePuppeteerNode('puppeteer'); export default puppeteer; diff --git a/src/web.ts b/src/web.ts deleted file mode 100644 index 777f9ae0c3a41..0000000000000 --- a/src/web.ts +++ /dev/null @@ -1,25 +0,0 @@ -/** - * 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. - */ - -import { isNode } from './environment.js'; -import { initializePuppeteerWeb } from './initialize-web.js'; - -if (isNode) { - throw new Error('Trying to run Puppeteer-Web in a Node environment'); -} - -const puppeteer = initializePuppeteerWeb('puppeteer'); -export default puppeteer; diff --git a/test-browser/connection.spec.js b/test-browser/connection.spec.js index 69d248e881958..cb5b0cb2c4fb4 100644 --- a/test-browser/connection.spec.js +++ b/test-browser/connection.spec.js @@ -15,7 +15,7 @@ */ import { Connection } from '../lib/esm/puppeteer/common/Connection.js'; import { BrowserWebSocketTransport } from '../lib/esm/puppeteer/common/BrowserWebSocketTransport.js'; -import puppeteer from '../lib/esm/puppeteer/web.js'; +import puppeteer from '../lib/esm/puppeteer/puppeteer.js'; import expect from '../node_modules/expect/build-es5/index.js'; import { getWebSocketEndpoint } from './helper.js'; diff --git a/test/assert-coverage-test.js b/test/assert-coverage-test.js index 28b25dab948e1..a63175e6780fc 100644 --- a/test/assert-coverage-test.js +++ b/test/assert-coverage-test.js @@ -2,6 +2,18 @@ const { describe, it } = require('mocha'); const { getCoverageResults } = require('./coverage-utils.js'); const expect = require('expect'); +const EXCLUDED_METHODS = new Set([ + 'Puppeteer.registerCustomQueryHandler', + 'Puppeteer.unregisterCustomQueryHandler', + 'Puppeteer.customQueryHandlerNames', + 'Puppeteer.clearCustomQueryHandlers', + 'PuppeteerNode.connect', + 'PuppeteerNode.launch', + 'PuppeteerNode.executablePath', + 'PuppeteerNode.defaultArgs', + 'PuppeteerNode.createBrowserFetcher', +]); + describe('API coverage test', () => { it('calls every method', () => { if (!process.env.COVERAGE) return; @@ -9,7 +21,8 @@ describe('API coverage test', () => { const coverageMap = getCoverageResults(); const missingMethods = []; for (const method of coverageMap.keys()) { - if (!coverageMap.get(method)) missingMethods.push(method); + if (!coverageMap.get(method) && !EXCLUDED_METHODS.has(method)) + missingMethods.push(method); } if (missingMethods.length) { console.error( diff --git a/test/launcher.spec.ts b/test/launcher.spec.ts index 33cb1ff71fbd4..2889c054b0ebb 100644 --- a/test/launcher.spec.ts +++ b/test/launcher.spec.ts @@ -148,9 +148,12 @@ describe('Launcher specs', function () { await server.waitForRequest('/one-style.css'); remote.disconnect(); const error = await navigationPromise; - expect(error.message).toBe( - 'Navigation failed because browser has disconnected!' - ); + expect( + [ + 'Navigation failed because browser has disconnected!', + 'Protocol error (Page.navigate): Target closed.', + ].includes(error.message) + ).toBeTruthy(); await browser.close(); }); it('should reject waitForSelector when browser closes', async () => { diff --git a/test/mocha-utils.ts b/test/mocha-utils.ts index 9511efc5c74a2..37190b940f689 100644 --- a/test/mocha-utils.ts +++ b/test/mocha-utils.ts @@ -19,7 +19,7 @@ import * as path from 'path'; import * as fs from 'fs'; import * as os from 'os'; import sinon from 'sinon'; -import puppeteer from '../lib/cjs/puppeteer/node.js'; +import puppeteer from '../lib/cjs/puppeteer/puppeteer.js'; import { Browser, BrowserContext, diff --git a/utils/prepare_puppeteer_core.js b/utils/prepare_puppeteer_core.js index e476b39fe4a03..fe7a0dfc734f8 100755 --- a/utils/prepare_puppeteer_core.js +++ b/utils/prepare_puppeteer_core.js @@ -24,8 +24,8 @@ const json = require(packagePath); delete json.scripts.install; json.name = 'puppeteer-core'; -json.main = './cjs-entry-core.js'; -json.exports['.'].import = './lib/esm/puppeteer/node-puppeteer-core.js'; -json.exports['.'].require = './cjs-entry-core.js'; +json.main = './lib/cjs/puppeteer/puppeteer-core.js'; +json.exports['.'].import = './lib/esm/puppeteer/puppeteer-core.js'; +json.exports['.'].require = './lib/cjs/puppeteer/puppeteer-core.js'; fs.writeFileSync(packagePath, JSON.stringify(json, null, ' '));