From cdfe73fee3145f284c5f4e08457c72653ff71268 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 5 Aug 2020 22:25:56 -0700 Subject: [PATCH] api(console): make ConsoleMessageLocation properties required (#3290) Everywhere in our api, possibly missing properties are nullable. However, to make things easier for everyone, we just default to an empty url instead, so that users do not have to null-check it. --- docs/api.md | 35 ++++++++++++++++---------------- src/chromium/crPage.ts | 10 +++++++-- src/chromium/crProtocolHelper.ts | 5 +++-- src/console.ts | 2 +- src/rpc/channels.ts | 6 +++--- src/rpc/protocol.yml | 6 +++--- src/types.ts | 6 +++--- src/webkit/wkPage.ts | 2 +- src/webkit/wkWorkers.ts | 8 +++++++- 9 files changed, 47 insertions(+), 33 deletions(-) diff --git a/docs/api.md b/docs/api.md index c20954d662..289f181085 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1149,8 +1149,8 @@ await page.dispatchEvent('#source', 'dragstart', { dataTransfer }); #### page.emulateMedia(options) - `options` <[Object]> - - `media` Changes the CSS media type of the page. The only allowed values are `'screen'`, `'print'` and `null`. Passing `null` disables CSS media emulation. Omitting `media` or passing `undefined` does not change the emulated value. - - `colorScheme` Emulates `'prefers-colors-scheme'` media feature, supported values are `'light'`, `'dark'`, `'no-preference'`. Passing `null` disables color scheme emulation. Omitting `colorScheme` or passing `undefined` does not change the emulated value. + - `media` <[null]|"screen"|"print"> Changes the CSS media type of the page. The only allowed values are `'screen'`, `'print'` and `null`. Passing `null` disables CSS media emulation. Omitting `media` or passing `undefined` does not change the emulated value. + - `colorScheme` <[null]|"light"|"dark"|"no-preference"> Emulates `'prefers-colors-scheme'` media feature, supported values are `'light'`, `'dark'`, `'no-preference'`. Passing `null` disables color scheme emulation. Omitting `colorScheme` or passing `undefined` does not change the emulated value. - returns: <[Promise]> ```js @@ -1391,7 +1391,7 @@ Returns frame matching the specified criteria. Either `name` or `url` must be sp - `name` <[string]> Attribute name to get the value for. - `options` <[Object]> - `timeout` <[number]> Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. -- returns: <[Promise]> +- returns: <[Promise]<[null]|[string]>> Returns element attribute value. @@ -1663,7 +1663,7 @@ Page routes take precedence over browser context routes (set up with [browserCon #### page.selectOption(selector, values[, options]) - `selector` <[string]> A selector to query page for. See [working with selectors](#working-with-selectors) for more details. -- `values` |[Object]|[Array]<[ElementHandle]>|[Array]<[Object]>> Options to select. If the `` has the `multiple` attribute, all matching options are selected, otherwise only the first option matching one of the passed options is selected. String values are equivalent to `{value:'string'}`. Option is considered matching if all specified properties match. - `value` <[string]> Matches by `option.value`. - `label` <[string]> Matches by `option.label`. - `index` <[number]> Matches by the index. @@ -1767,7 +1767,7 @@ await page.goto('https://example.com'); - `selector` <[string]> A selector to search for an element. If there are multiple elements satisfying the selector, the first will be picked. See [working with selectors](#working-with-selectors) for more details. - `options` <[Object]> - `timeout` <[number]> Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. -- returns: <[Promise]> +- returns: <[Promise]<[null]|[string]>> Resolves to the `element.textContent`. @@ -2338,7 +2338,7 @@ console.log(frame === contentFrame); // -> true - `name` <[string]> Attribute name to get the value for. - `options` <[Object]> - `timeout` <[number]> Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. -- returns: <[Promise]> +- returns: <[Promise]<[null]|[string]>> Returns element attribute value. @@ -2438,7 +2438,7 @@ Shortcuts such as `key: "Control+o"` or `key: "Control+Shift+T"` are supported a #### frame.selectOption(selector, values[, options]) - `selector` <[string]> A selector to query frame for. See [working with selectors](#working-with-selectors) for more details. -- `values` |[Object]|[Array]<[ElementHandle]>|[Array]<[Object]>> Options to select. If the `` has the `multiple` attribute, all matching options are selected, otherwise only the first option matching one of the passed options is selected. String values are equivalent to `{value:'string'}`. Option is considered matching if all specified properties match. - `value` <[string]> Matches by `option.value`. - `label` <[string]> Matches by `option.label`. - `index` <[number]> Matches by the index. @@ -2490,7 +2490,7 @@ Sets the value of the file input to these file paths or files. If some of the `f - `selector` <[string]> A selector to search for an element. If there are multiple elements satisfying the selector, the first will be picked. See [working with selectors](#working-with-selectors) for more details. - `options` <[Object]> - `timeout` <[number]> Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. -- returns: <[Promise]> +- returns: <[Promise]<[null]|[string]>> Resolves to the `element.textContent`. @@ -2859,7 +2859,7 @@ Calls [focus](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus #### elementHandle.getAttribute(name) - `name` <[string]> Attribute name to get the value for. -- returns: <[Promise]> +- returns: <[Promise]<[null]|[string]>> Returns element attribute value. @@ -2929,7 +2929,7 @@ This method waits for [actionability](./actionability.md) checks, then tries to Throws when ```elementHandle``` does not point to an element [connected](https://developer.mozilla.org/en-US/docs/Web/API/Node/isConnected) to a Document or a ShadowRoot. #### elementHandle.selectOption(values[, options]) -- `values` |[Object]|[Array]<[ElementHandle]>|[Array]<[Object]>> Options to select. If the `` has the `multiple` attribute, all matching options are selected, otherwise only the first option matching one of the passed options is selected. String values are equivalent to `{value:'string'}`. Option is considered matching if all specified properties match. - `value` <[string]> Matches by `option.value`. - `label` <[string]> Matches by `option.label`. - `index` <[number]> Matches by the index. @@ -2977,7 +2977,7 @@ This method expects `elementHandle` to point to an [input element](https://devel Sets the value of the file input to these file paths or files. If some of the `filePaths` are relative paths, then they are resolved relative to the [current working directory](https://nodejs.org/api/process.html#process_process_cwd). For empty array, clears the selected files. #### elementHandle.textContent() -- returns: <[Promise]> Resolves to the `node.textContent`. +- returns: <[Promise]<[null]|[string]>> Resolves to the `node.textContent`. #### elementHandle.toString() - returns: <[string]> @@ -3120,9 +3120,9 @@ function, it **will not be called**. #### consoleMessage.location() - returns: <[Object]> - - `url` <[string]> Optional URL of the resource if available. - - `lineNumber` <[number]> Optional 0-based line number in the resource if available. - - `columnNumber` <[number]> Optional 0-based column number in the resource if available. + - `url` <[string]> URL of the resource if available, otherwise empty string. + - `lineNumber` <[number]> 0-based line number in the resource. + - `columnNumber` <[number]> 0-based column number in the resource. #### consoleMessage.text() - returns: <[string]> @@ -3209,7 +3209,7 @@ const path = await download.path(); #### download.createReadStream() -- returns: <[Promise]> +- returns: <[Promise]<[null]|[Readable]>> Returns readable stream for current download or `null` if download failed. @@ -3219,12 +3219,12 @@ Returns readable stream for current download or `null` if download failed. Deletes the downloaded file. #### download.failure() -- returns: <[Promise]> +- returns: <[Promise]<[null]|[string]>> Returns download error if any. #### download.path() -- returns: <[Promise]> +- returns: <[Promise]<[null]|[string]>> Returns path to the downloaded file in case of successful download. @@ -4628,6 +4628,7 @@ const { chromium } = require('playwright'); [boolean]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type "Boolean" [function]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function "Function" [iterator]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols "Iterator" +[null]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/null [number]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type "Number" [origin]: https://developer.mozilla.org/en-US/docs/Glossary/Origin "Origin" [selector]: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors "selector" diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index a840610b12..3774258f3c 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -678,8 +678,14 @@ class FrameSession { const {level, text, args, source, url, lineNumber} = event.entry; if (args) args.map(arg => releaseObject(this._client, arg.objectId!)); - if (source !== 'worker') - this._page.emit(Events.Page.Console, new ConsoleMessage(level, text, [], {url, lineNumber})); + if (source !== 'worker') { + const location: types.ConsoleMessageLocation = { + url: url || '', + lineNumber: lineNumber || 0, + columnNumber: 0, + }; + this._page.emit(Events.Page.Console, new ConsoleMessage(level, text, [], location)); + } } async _onFileChooserOpened(event: Protocol.Page.fileChooserOpenedPayload) { diff --git a/src/chromium/crProtocolHelper.ts b/src/chromium/crProtocolHelper.ts index 8f566bf9b9..5bc95c5fe5 100644 --- a/src/chromium/crProtocolHelper.ts +++ b/src/chromium/crProtocolHelper.ts @@ -19,6 +19,7 @@ import { CRSession } from './crConnection'; import { Protocol } from './protocol'; import * as fs from 'fs'; import * as util from 'util'; +import * as types from '../types'; export function getExceptionMessage(exceptionDetails: Protocol.Runtime.ExceptionDetails): string { if (exceptionDetails.exception) @@ -58,12 +59,12 @@ export async function readProtocolStream(client: CRSession, handle: string, path return Buffer.concat(bufs); } -export function toConsoleMessageLocation(stackTrace: Protocol.Runtime.StackTrace | undefined) { +export function toConsoleMessageLocation(stackTrace: Protocol.Runtime.StackTrace | undefined): types.ConsoleMessageLocation { return stackTrace && stackTrace.callFrames.length ? { url: stackTrace.callFrames[0].url, lineNumber: stackTrace.callFrames[0].lineNumber, columnNumber: stackTrace.callFrames[0].columnNumber, - } : {}; + } : { url: '', lineNumber: 0, columnNumber: 0 }; } export function exceptionToError(exceptionDetails: Protocol.Runtime.ExceptionDetails): Error { diff --git a/src/console.ts b/src/console.ts index 5bf189d873..d60a2908e4 100644 --- a/src/console.ts +++ b/src/console.ts @@ -28,7 +28,7 @@ export class ConsoleMessage { this._type = type; this._text = text; this._args = args; - this._location = location || {}; + this._location = location || { url: '', lineNumber: 0, columnNumber: 0 }; } type(): string { diff --git a/src/rpc/channels.ts b/src/rpc/channels.ts index 42182b6db5..2045ee3c92 100644 --- a/src/rpc/channels.ts +++ b/src/rpc/channels.ts @@ -2112,9 +2112,9 @@ export type ConsoleMessageInitializer = { text: string, args: JSHandleChannel[], location: { - url?: string, - lineNumber?: number, - columnNumber?: number, + url: string, + lineNumber: number, + columnNumber: number, }, }; export interface ConsoleMessageChannel extends Channel { diff --git a/src/rpc/protocol.yml b/src/rpc/protocol.yml index 32b5b29427..d837e088ba 100644 --- a/src/rpc/protocol.yml +++ b/src/rpc/protocol.yml @@ -1751,9 +1751,9 @@ ConsoleMessage: location: type: object properties: - url: string? - lineNumber: number? - columnNumber: number? + url: string + lineNumber: number + columnNumber: number diff --git a/src/types.ts b/src/types.ts index 3b532e0a1c..e973520884 100644 --- a/src/types.ts +++ b/src/types.ts @@ -343,9 +343,9 @@ export type SerializedAXNode = { }; export type ConsoleMessageLocation = { - url?: string, - lineNumber?: number, - columnNumber?: number, + url: string, + lineNumber: number, + columnNumber: number, }; export type Error = { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 588c439c96..7cecbe8d09 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -497,7 +497,7 @@ export class WKPage implements PageDelegate { handles, count: 0, location: { - url, + url: url || '', lineNumber: (lineNumber || 1) - 1, columnNumber: (columnNumber || 1) - 1, } diff --git a/src/webkit/wkWorkers.ts b/src/webkit/wkWorkers.ts index 3bd6fe4e56..f1041abcee 100644 --- a/src/webkit/wkWorkers.ts +++ b/src/webkit/wkWorkers.ts @@ -19,6 +19,7 @@ import { Page, Worker } from '../page'; import { Protocol } from './protocol'; import { WKSession } from './wkConnection'; import { WKExecutionContext } from './wkExecutionContext'; +import * as types from '../types'; export class WKWorkers { private _sessionListeners: RegisteredListener[] = []; @@ -93,6 +94,11 @@ export class WKWorkers { const handles = (parameters || []).map(p => { return worker._existingExecutionContext!.createHandle(p); }); - this._page._addConsoleMessage(derivedType, handles, { url, lineNumber: (lineNumber || 1) - 1, columnNumber: (columnNumber || 1) - 1 }, handles.length ? undefined : text); + const location: types.ConsoleMessageLocation = { + url: url || '', + lineNumber: (lineNumber || 1) - 1, + columnNumber: (columnNumber || 1) - 1 + }; + this._page._addConsoleMessage(derivedType, handles, location, handles.length ? undefined : text); } }