From e7b91a7f41c3a0f0b9e99b0f83a792e4d1f08459 Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Fri, 19 Jun 2020 15:39:03 +0100 Subject: [PATCH] chore: enforce a max line length on comments (#6055) --- .eslintrc.js | 16 +++++++++++ .../puppeteer.webworker.evaluatehandle.md | 2 +- new-docs/puppeteer.webworker.md | 2 +- src/api-docs-entry.ts | 12 +++++---- src/api.ts | 6 +++-- src/common/Accessibility.ts | 15 ++++++----- src/common/Debug.ts | 5 ++-- src/common/Dialog.ts | 7 +++-- src/common/HTTPRequest.ts | 19 ++++++++----- src/common/JSHandle.ts | 10 ++++--- src/common/NetworkManager.ts | 3 ++- src/common/Page.ts | 14 +++++++--- src/common/WebWorker.ts | 27 +++++++++++++------ src/initialize.ts | 6 ++--- src/node/BrowserRunner.ts | 10 ++++--- test/coverage-utils.js | 5 ++-- test/launcher.spec.js | 3 ++- 17 files changed, 109 insertions(+), 53 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 6f6672c6089..aa57972cbf4 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -37,6 +37,22 @@ module.exports = { "func-call-spacing": 2, "prefer-const": 2, + "max-len": [2, { + /* this setting doesn't impact things as we use Prettier to format + * our code and hence dictate the line length. + * Prettier aims for 80 but sometimes makes the decision to go just + * over 80 chars as it decides that's better than wrapping. ESLint's + * rule defaults to 80 but therefore conflicts with Prettier. So we + * set it to something far higher than Prettier would allow to avoid + * it causing issues and conflicting with Prettier. + */ + "code": 200, + "comments": 90, + "ignoreTemplateLiterals": true, + "ignoreUrls": true, + "ignoreStrings": true, + "ignoreRegExpLiterals": true + }], // anti-patterns "no-var": 2, "no-with": 2, diff --git a/new-docs/puppeteer.webworker.evaluatehandle.md b/new-docs/puppeteer.webworker.evaluatehandle.md index 8482842f8ca..f187a930aa8 100644 --- a/new-docs/puppeteer.webworker.evaluatehandle.md +++ b/new-docs/puppeteer.webworker.evaluatehandle.md @@ -4,7 +4,7 @@ ## WebWorker.evaluateHandle() method -The only difference between `worker.evaluate` and `worker.evaluateHandle` is that `worker.evaluateHandle` returns in-page object (JSHandle). If the function passed to the `worker.evaluateHandle` returns a \[Promise\], then `worker.evaluateHandle` would wait for the promise to resolve and return its value. Shortcut for \[(await worker.executionContext()).evaluateHandle(pageFunction, ...args)\](\#executioncontextevaluatehandlepagefunction-args). +The only difference between `worker.evaluate` and `worker.evaluateHandle` is that `worker.evaluateHandle` returns in-page object (JSHandle). If the function passed to the `worker.evaluateHandle` returns a \[Promise\], then `worker.evaluateHandle` would wait for the promise to resolve and return its value. Shortcut for `await worker.executionContext()).evaluateHandle(pageFunction, ...args)` Signature: diff --git a/new-docs/puppeteer.webworker.md b/new-docs/puppeteer.webworker.md index e4044296109..771e1d3bea5 100644 --- a/new-docs/puppeteer.webworker.md +++ b/new-docs/puppeteer.webworker.md @@ -46,7 +46,7 @@ for (const worker of page.workers()) { | Method | Modifiers | Description | | --- | --- | --- | | [evaluate(pageFunction, args)](./puppeteer.webworker.evaluate.md) | | If the function passed to the worker.evaluate returns a Promise, then worker.evaluate would wait for the promise to resolve and return its value. If the function passed to the worker.evaluate returns a non-serializable value, then worker.evaluate resolves to undefined. DevTools Protocol also supports transferring some additional values that are not serializable by JSON: -0, NaN, Infinity, -Infinity, and bigint literals. Shortcut for await worker.executionContext()).evaluate(pageFunction, ...args). | -| [evaluateHandle(pageFunction, args)](./puppeteer.webworker.evaluatehandle.md) | | The only difference between worker.evaluate and worker.evaluateHandle is that worker.evaluateHandle returns in-page object (JSHandle). If the function passed to the worker.evaluateHandle returns a \[Promise\], then worker.evaluateHandle would wait for the promise to resolve and return its value. Shortcut for \[(await worker.executionContext()).evaluateHandle(pageFunction, ...args)\](\#executioncontextevaluatehandlepagefunction-args). | +| [evaluateHandle(pageFunction, args)](./puppeteer.webworker.evaluatehandle.md) | | The only difference between worker.evaluate and worker.evaluateHandle is that worker.evaluateHandle returns in-page object (JSHandle). If the function passed to the worker.evaluateHandle returns a \[Promise\], then worker.evaluateHandle would wait for the promise to resolve and return its value. Shortcut for await worker.executionContext()).evaluateHandle(pageFunction, ...args) | | [executionContext()](./puppeteer.webworker.executioncontext.md) | | Returns the ExecutionContext the WebWorker runs in | | [url()](./puppeteer.webworker.url.md) | | | diff --git a/src/api-docs-entry.ts b/src/api-docs-entry.ts index f5ee597ee9d..cc57f602e96 100644 --- a/src/api-docs-entry.ts +++ b/src/api-docs-entry.ts @@ -15,13 +15,15 @@ */ /* - * This file re-exports any APIs that we want to have documentation generated for. - * It is used by API Extractor to determine what parts of the system to document. + * This file re-exports any APIs that we want to have documentation generated + * for. It is used by API Extractor to determine what parts of the system to + * document. * - * We also have src/api.ts. This is used in `index.js` and by the legacy DocLint system. - * src/api-docs-entry.ts is ONLY used by API Extractor. + * We also have src/api.ts. This is used in `index.js` and by the legacy DocLint + * system. src/api-docs-entry.ts is ONLY used by API Extractor. * - * Once we have migrated to API Extractor and removed DocLint we can remove the duplication and use this file. + * Once we have migrated to API Extractor and removed DocLint we can remove the + * duplication and use this file. */ export * from './common/Accessibility'; export * from './common/Browser'; diff --git a/src/api.ts b/src/api.ts index ac5b5228dc4..988eab0b556 100644 --- a/src/api.ts +++ b/src/api.ts @@ -15,8 +15,10 @@ */ /* This file is used in two places: - * 1) the coverage-utils use it to gain a list of all methods we check for test coverage on - * 2) index.js uses it to iterate through all methods and call helper.installAsyncStackHooks on + * 1) the coverage-utils use it to gain a list of all methods we check for test + * coverage on + * 2) index.js uses it to iterate through all methods and call + * helper.installAsyncStackHooks on */ module.exports = { Accessibility: require('./common/Accessibility').Accessibility, diff --git a/src/common/Accessibility.ts b/src/common/Accessibility.ts index 512632c95e8..d9a4b5fc258 100644 --- a/src/common/Accessibility.ts +++ b/src/common/Accessibility.ts @@ -64,7 +64,8 @@ export interface SerializedAXNode { required?: boolean; selected?: boolean; /** - * Whether the checkbox is checked, or in a {@link https://www.w3.org/TR/wai-aria-practices/examples/checkbox/checkbox-2/checkbox-2.html | mixed state}. + * Whether the checkbox is checked, or in a + * {@link https://www.w3.org/TR/wai-aria-practices/examples/checkbox/checkbox-2/checkbox-2.html | mixed state}. */ checked?: boolean | 'mixed'; /** @@ -144,9 +145,10 @@ export class Accessibility { * * @remarks * - * **NOTE** The Chromium accessibility tree contains nodes that go unused on most platforms and by - * most screen readers. Puppeteer will discard them as well for an easier to process tree, - * unless `interestingOnly` is set to `false`. + * **NOTE** The Chromium accessibility tree contains nodes that go unused on + * most platforms and by most screen readers. Puppeteer will discard them as + * well for an easier to process tree, unless `interestingOnly` is set to + * `false`. * * @example * An example of dumping the entire accessibility tree: @@ -436,8 +438,9 @@ class AXNode { properties.get(key) as boolean; for (const booleanProperty of booleanProperties) { - // WebArea's treat focus differently than other nodes. They report whether their frame has focus, - // not whether focus is specifically on the root node. + // WebArea's treat focus differently than other nodes. They report whether + // their frame has focus, not whether focus is specifically on the root + // node. if (booleanProperty === 'focused' && this._role === 'WebArea') continue; const value = getBooleanPropertyValue(booleanProperty); if (!value) continue; diff --git a/src/common/Debug.ts b/src/common/Debug.ts index ad4ad8ab56c..24f0434bf86 100644 --- a/src/common/Debug.ts +++ b/src/common/Debug.ts @@ -19,8 +19,9 @@ const isNodeEnv = typeof document === 'undefined'; /** * A debug function that can be used in any environment. * - * If used in Node, it falls back to the {@link https://www.npmjs.com/package/debug | debug module}. - * In the browser it uses `console.log`. + * If used in Node, it falls back to the + * {@link https://www.npmjs.com/package/debug | debug module}. In the browser it + * uses `console.log`. * * @param prefix - this will be prefixed to each log. * @returns a function that can be called to log to that debug channel. diff --git a/src/common/Dialog.ts b/src/common/Dialog.ts index 7675ad4d4b0..87974ebdf9c 100644 --- a/src/common/Dialog.ts +++ b/src/common/Dialog.ts @@ -76,14 +76,17 @@ export class Dialog { } /** - * @returns The default value of the prompt, or an empty string if the dialog is not a `prompt`. + * @returns The default value of the prompt, or an empty string if the dialog + * is not a `prompt`. */ defaultValue(): string { return this._defaultValue; } /** - * @param promptText - optional text that will be entered in the dialog prompt. Has no effect if the dialog's type is not `prompt`. + * @param promptText - optional text that will be entered in the dialog + * prompt. Has no effect if the dialog's type is not `prompt`. + * * @returns A promise that resolves when the dialog has been accepted. */ async accept(promptText?: string): Promise { diff --git a/src/common/HTTPRequest.ts b/src/common/HTTPRequest.ts index a11250f34b0..04ff725f8a3 100644 --- a/src/common/HTTPRequest.ts +++ b/src/common/HTTPRequest.ts @@ -135,8 +135,9 @@ export class HTTPRequest { headers: headers ? headersArray(headers) : undefined, }) .catch((error) => { - // In certain cases, protocol will return error if the request was already canceled - // or the page was closed. We should tolerate these errors. + // In certain cases, protocol will return error if the request was + // already canceled or the page was closed. We should tolerate these + // errors. debugError(error); }); } @@ -179,8 +180,9 @@ export class HTTPRequest { body: responseBody ? responseBody.toString('base64') : undefined, }) .catch((error) => { - // In certain cases, protocol will return error if the request was already canceled - // or the page was closed. We should tolerate these errors. + // In certain cases, protocol will return error if the request was + // already canceled or the page was closed. We should tolerate these + // errors. debugError(error); }); } @@ -199,8 +201,9 @@ export class HTTPRequest { errorReason, }) .catch((error) => { - // In certain cases, protocol will return error if the request was already canceled - // or the page was closed. We should tolerate these errors. + // In certain cases, protocol will return error if the request was + // already canceled or the page was closed. We should tolerate these + // errors. debugError(error); }); } @@ -250,7 +253,9 @@ function headersArray( return result; } -// List taken from https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml with extra 306 and 418 codes. +// List taken from +// https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml +// with extra 306 and 418 codes. const STATUS_TEXTS = { '100': 'Continue', '101': 'Switching Protocols', diff --git a/src/common/JSHandle.ts b/src/common/JSHandle.ts index 78aef768087..7d71a87d2d1 100644 --- a/src/common/JSHandle.ts +++ b/src/common/JSHandle.ts @@ -194,8 +194,9 @@ export class ElementHandle extends JSHandle { element.scrollIntoView({ block: 'center', inline: 'center', - // Chrome still supports behavior: instant but it's not in the spec so TS shouts - // We don't want to make this breaking change in Puppeteer yet so we'll ignore the line. + // Chrome still supports behavior: instant but it's not in the spec + // so TS shouts We don't want to make this breaking change in + // Puppeteer yet so we'll ignore the line. // @ts-ignore behavior: 'instant', }); @@ -212,8 +213,9 @@ export class ElementHandle extends JSHandle { element.scrollIntoView({ block: 'center', inline: 'center', - // Chrome still supports behavior: instant but it's not in the spec so TS shouts - // We don't want to make this breaking change in Puppeteer yet so we'll ignore the line. + // Chrome still supports behavior: instant but it's not in the spec + // so TS shouts We don't want to make this breaking change in + // Puppeteer yet so we'll ignore the line. // @ts-ignore behavior: 'instant', }); diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 2ed5c7a9297..2f52284cf79 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -242,7 +242,8 @@ export class NetworkManager extends EventEmitter { let redirectChain = []; if (event.redirectResponse) { const request = this._requestIdToRequest.get(event.requestId); - // If we connect late to the target, we could have missed the requestWillBeSent event. + // If we connect late to the target, we could have missed the + // requestWillBeSent event. if (request) { this._handleRequestRedirect(request, event.redirectResponse); redirectChain = request._redirectChain; diff --git a/src/common/Page.ts b/src/common/Page.ts index 2e93eb01720..963fd515ca2 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -143,7 +143,9 @@ enum VisionDeficiency { */ export const enum PageEmittedEvents { /** - * Emitted when a dedicated {@link https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API | WebWorker} is spawned by the page. + * Emitted when a dedicated + * {@link https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API | WebWorker} + * is spawned by the page. * @eventProperty */ WorkerCreated = 'workercreated', @@ -166,7 +168,9 @@ class ScreenshotTaskQueue { } /** - * Page provides methods to interact with a single tab or [extension background page](https://developer.chrome.com/extensions/background_pages) in Chromium. One [Browser] instance might have multiple [Page] instances. + * Page provides methods to interact with a single tab or [extension background + * page](https://developer.chrome.com/extensions/background_pages) in Chromium. + * One [Browser] instance might have multiple [Page] instances. * * @remarks * @@ -184,7 +188,8 @@ class ScreenshotTaskQueue { * })(); * ``` * - * The Page class extends from Puppeteer's {@link EventEmitter } class and will emit various events which are documented in the {@link PageEmittedEvents} enum. + * The Page class extends from Puppeteer's {@link EventEmitter } class and will + * emit various events which are documented in the {@link PageEmittedEvents} enum. * * @example * This example logs a message for a single page `load` event: @@ -1052,7 +1057,8 @@ export class Page extends EventEmitter { ): Promise { let screenshotType = null; // options.type takes precedence over inferring the type from options.path - // because it may be a 0-length file with no extension created beforehand (i.e. as a temp file). + // because it may be a 0-length file with no extension created beforehand + // (i.e. as a temp file). if (options.type) { assert( options.type === 'png' || options.type === 'jpeg', diff --git a/src/common/WebWorker.ts b/src/common/WebWorker.ts index 4dccdbe4301..4b9e4ab3ff7 100644 --- a/src/common/WebWorker.ts +++ b/src/common/WebWorker.ts @@ -31,10 +31,12 @@ type ExceptionThrownCallback = ( type JSHandleFactory = (obj: Protocol.Runtime.RemoteObject) => JSHandle; /** - * The WebWorker class represents a {@link https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API | WebWorker}. + * The WebWorker class represents a + * {@link https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API | WebWorker}. * * @remarks - * The events `workercreated` and `workerdestroyed` are emitted on the page object to signal the worker lifecycle. + * The events `workercreated` and `workerdestroyed` are emitted on the page + * object to signal the worker lifecycle. * * @example * ```js @@ -115,9 +117,14 @@ export class WebWorker extends EventEmitter { } /** - * If the function passed to the `worker.evaluate` returns a Promise, then `worker.evaluate` would wait for the promise to resolve and return its value. - * If the function passed to the `worker.evaluate` returns a non-serializable value, then `worker.evaluate` resolves to `undefined`. DevTools Protocol also supports transferring some additional values that are not serializable by `JSON`: `-0`, `NaN`, `Infinity`, `-Infinity`, and bigint literals. - * Shortcut for `await worker.executionContext()).evaluate(pageFunction, ...args)`. + * If the function passed to the `worker.evaluate` returns a Promise, then + * `worker.evaluate` would wait for the promise to resolve and return its + * value. If the function passed to the `worker.evaluate` returns a + * non-serializable value, then `worker.evaluate` resolves to `undefined`. + * DevTools Protocol also supports transferring some additional values that + * are not serializable by `JSON`: `-0`, `NaN`, `Infinity`, `-Infinity`, and + * bigint literals. Shortcut for `await + * worker.executionContext()).evaluate(pageFunction, ...args)`. * * @param pageFunction - Function to be evaluated in the worker context. * @param args - Arguments to pass to `pageFunction`. @@ -134,9 +141,13 @@ export class WebWorker extends EventEmitter { } /** - * The only difference between `worker.evaluate` and `worker.evaluateHandle` is that `worker.evaluateHandle` returns in-page object (JSHandle). - * If the function passed to the `worker.evaluateHandle` returns a [Promise], then `worker.evaluateHandle` would wait for the promise to resolve and return its value. - * Shortcut for [(await worker.executionContext()).evaluateHandle(pageFunction, ...args)](#executioncontextevaluatehandlepagefunction-args). + * The only difference between `worker.evaluate` and `worker.evaluateHandle` + * is that `worker.evaluateHandle` returns in-page object (JSHandle). If the + * function passed to the `worker.evaluateHandle` returns a [Promise], then + * `worker.evaluateHandle` would wait for the promise to resolve and return + * its value. Shortcut for + * `await worker.executionContext()).evaluateHandle(pageFunction, ...args)` + * * @param pageFunction - Function to be evaluated in the page context. * @param args - Arguments to pass to `pageFunction`. * @returns Promise which resolves to the return value of `pageFunction`. diff --git a/src/initialize.ts b/src/initialize.ts index a083eacbcc7..3399d8782e4 100644 --- a/src/initialize.ts +++ b/src/initialize.ts @@ -64,9 +64,9 @@ export const initializePuppeteer = (options: InitOptions): Puppeteer => { product ); - // The introspection in `Helper.installAsyncStackHooks` references `Puppeteer._launcher` - // before the Puppeteer ctor is called, such that an invalid Launcher is selected at import, - // so we reset it. + // The introspection in `Helper.installAsyncStackHooks` references + // `Puppeteer._launcher` before the Puppeteer ctor is called, such that an + // invalid Launcher is selected at import, so we reset it. puppeteer._lazyLauncher = undefined; return puppeteer; }; diff --git a/src/node/BrowserRunner.ts b/src/node/BrowserRunner.ts index 6a2e921107f..c3ae7f47e5f 100644 --- a/src/node/BrowserRunner.ts +++ b/src/node/BrowserRunner.ts @@ -78,9 +78,10 @@ export class BrowserRunner { this._executablePath, this._processArguments, { - // On non-windows platforms, `detached: true` makes child process a leader of a new - // process group, making it possible to kill child process tree with `.kill(-pid)` command. - // @see https://nodejs.org/api/child_process.html#child_process_options_detached + // On non-windows platforms, `detached: true` makes child process a + // leader of a new process group, making it possible to kill child + // process tree with `.kill(-pid)` command. @see + // https://nodejs.org/api/child_process.html#child_process_options_detached detached: process.platform !== 'win32', env, stdio, @@ -180,7 +181,8 @@ export class BrowserRunner { const transport = await WebSocketTransport.create(browserWSEndpoint); this.connection = new Connection(browserWSEndpoint, transport, slowMo); } else { - // stdio was assigned during start(), and the 'pipe' option there adds the 4th and 5th items to stdio array + // stdio was assigned during start(), and the 'pipe' option there adds the + // 4th and 5th items to stdio array const { 3: pipeWrite, 4: pipeRead } = this.proc.stdio; const transport = new PipeTransport( pipeWrite as NodeJS.WritableStream, diff --git a/test/coverage-utils.js b/test/coverage-utils.js index cd490e9f5bc..aea76803fc3 100644 --- a/test/coverage-utils.js +++ b/test/coverage-utils.js @@ -15,8 +15,9 @@ */ /* We want to ensure that all of Puppeteer's public API is tested via our unit - * tests but we can't use a tool like Istanbul because the way it instruments code - * unfortunately breaks in Puppeteer where some of that code is then being executed in a browser context. + * tests but we can't use a tool like Istanbul because the way it instruments + * code unfortunately breaks in Puppeteer where some of that code is then being + * executed in a browser context. * * So instead we maintain this coverage code which does the following: * * takes every public method that we expect to be tested diff --git a/test/launcher.spec.js b/test/launcher.spec.js index b20e0f52b97..7b470562092 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -260,7 +260,8 @@ describe('Launcher specs', function () { // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 await rmAsync(userDataDir).catch((error) => {}); }); - // This mysteriously fails on Windows on AppVeyor. See https://github.com/puppeteer/puppeteer/issues/4111 + // This mysteriously fails on Windows on AppVeyor. See + // https://github.com/puppeteer/puppeteer/issues/4111 xit('userDataDir option should restore cookies', async () => { const { server, puppeteer } = getTestState();