From 38f99e0219075410a2ba461b144ed2615e03eee9 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 18 Aug 2020 15:59:15 -0700 Subject: [PATCH] chore: simplify conversions around selectOption We do not need to support api types on the server side. --- src/dom.ts | 28 ++++------------------ src/frames.ts | 4 ++-- src/rpc/client/elementHandle.ts | 3 +-- src/rpc/server/elementHandlerDispatcher.ts | 11 ++------- src/rpc/server/frameDispatcher.ts | 5 ++-- 5 files changed, 12 insertions(+), 39 deletions(-) diff --git a/src/dom.ts b/src/dom.ts index 284dc4b5aca76..8a15fe3e6d425 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -397,35 +397,15 @@ export class ElementHandle extends js.JSHandle { return this._retryPointerAction(progress, 'dblclick', true /* waitForEnabled */, point => this._page.mouse.dblclick(point.x, point.y, options), options); } - async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[] | null, options: types.NavigatingActionWaitOptions = {}): Promise { + async selectOption(elements: ElementHandle[], values: types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise { return this._page._runAbortableTask(async progress => { - const result = await this._selectOption(progress, values, options); + const result = await this._selectOption(progress, elements, values, options); return throwRetargetableDOMError(result); }, this._page._timeoutSettings.timeout(options)); } - async _selectOption(progress: Progress, values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[] | null, options: types.NavigatingActionWaitOptions): Promise { - let vals: string[] | ElementHandle[] | types.SelectOption[]; - if (values === null) - vals = []; - else if (!Array.isArray(values)) - vals = [ values ] as (string[] | ElementHandle[] | types.SelectOption[]); - else - vals = values; - const selectOptions = (vals as any).map((value: any) => helper.isString(value) ? { value } : value); - for (let i = 0; i < selectOptions.length; i++) { - const option = selectOptions[i]; - assert(option !== null, `options[${i}]: expected object, got null`); - assert(typeof option === 'object', `options[${i}]: expected object, got ${typeof option}`); - if (option instanceof ElementHandle) - continue; - if (option.value !== undefined) - assert(helper.isString(option.value), `options[${i}].value: expected string, got ${typeof option.value}`); - if (option.label !== undefined) - assert(helper.isString(option.label), `options[${i}].label: expected string, got ${typeof option.label}`); - if (option.index !== undefined) - assert(helper.isNumber(option.index), `options[${i}].index: expected number, got ${typeof option.index}`); - } + async _selectOption(progress: Progress, elements: ElementHandle[], values: types.SelectOption[], options: types.NavigatingActionWaitOptions): Promise { + const selectOptions = [...elements, ...values]; return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { progress.throwIfAborted(); // Avoid action that has side-effects. return throwFatalDOMError(await this._evaluateInUtility(([injected, node, selectOptions]) => injected.selectOptions(node, selectOptions), selectOptions)); diff --git a/src/frames.ts b/src/frames.ts index be842a5ef9507..9464fc166b24e 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -863,8 +863,8 @@ export class Frame { await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._hover(progress, options)); } - async selectOption(selector: string, values: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[] | null, options: types.NavigatingActionWaitOptions = {}): Promise { - return this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._selectOption(progress, values, options)); + async selectOption(selector: string, elements: dom.ElementHandle[], values: types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise { + return this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._selectOption(progress, elements, values, options)); } async setInputFiles(selector: string, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}): Promise { diff --git a/src/rpc/client/elementHandle.ts b/src/rpc/client/elementHandle.ts index 88954278a66d8..3c9011c5a09d6 100644 --- a/src/rpc/client/elementHandle.ts +++ b/src/rpc/client/elementHandle.ts @@ -224,7 +224,7 @@ export class ElementHandle extends JSHandle { } export function convertSelectOptionValues(values: string | ElementHandle | SelectOption | string[] | ElementHandle[] | SelectOption[] | null): { elements?: ElementHandleChannel[], options?: SelectOption[] } { - if (!values) + if (values === null) return {}; if (!Array.isArray(values)) values = [ values as any ]; @@ -232,7 +232,6 @@ export function convertSelectOptionValues(values: string | ElementHandle | Selec return {}; for (let i = 0; i < values.length; i++) assert(values[i] !== null, `options[${i}]: expected object, got null`); - if (values[0] instanceof ElementHandle) return { elements: (values as ElementHandle[]).map((v: ElementHandle) => v._elementChannel) }; if (helper.isString(values[0])) diff --git a/src/rpc/server/elementHandlerDispatcher.ts b/src/rpc/server/elementHandlerDispatcher.ts index 032906b6e893d..0d692d3db5597 100644 --- a/src/rpc/server/elementHandlerDispatcher.ts +++ b/src/rpc/server/elementHandlerDispatcher.ts @@ -87,7 +87,8 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements Eleme } async selectOption(params: { elements?: ElementHandleChannel[], options?: types.SelectOption[] } & types.NavigatingActionWaitOptions): Promise<{ values: string[] }> { - return { values: await this._elementHandle.selectOption(convertSelectOptionValues(params.elements, params.options), params) }; + const elements = (params.elements || []).map(e => (e as ElementHandleDispatcher)._elementHandle); + return { values: await this._elementHandle.selectOption(elements, params.options || [], params) }; } async fill(params: { value: string } & types.NavigatingActionWaitOptions) { @@ -158,14 +159,6 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements Eleme } } -export function convertSelectOptionValues(elements?: ElementHandleChannel[], options?: types.SelectOption[]): string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[] | null { - if (elements) - return elements.map(v => (v as ElementHandleDispatcher)._elementHandle); - if (options) - return options; - return null; -} - export function convertInputFiles(files: { name: string, mimeType: string, buffer: string }[]): types.FilePayload[] { return files.map(f => ({ name: f.name, mimeType: f.mimeType, buffer: Buffer.from(f.buffer, 'base64') })); } diff --git a/src/rpc/server/frameDispatcher.ts b/src/rpc/server/frameDispatcher.ts index d4bdc3fadd8e3..e14998777bb26 100644 --- a/src/rpc/server/frameDispatcher.ts +++ b/src/rpc/server/frameDispatcher.ts @@ -18,7 +18,7 @@ import { Frame, kAddLifecycleEvent, kRemoveLifecycleEvent, kNavigationEvent, Nav import * as types from '../../types'; import { ElementHandleChannel, FrameChannel, FrameInitializer, JSHandleChannel, ResponseChannel, SerializedArgument, FrameWaitForFunctionParams, SerializedValue } from '../channels'; import { Dispatcher, DispatcherScope, lookupNullableDispatcher, existingDispatcher } from './dispatcher'; -import { convertSelectOptionValues, ElementHandleDispatcher, createHandle, convertInputFiles } from './elementHandlerDispatcher'; +import { ElementHandleDispatcher, createHandle, convertInputFiles } from './elementHandlerDispatcher'; import { parseArgument, serializeResult } from './jsHandleDispatcher'; import { ResponseDispatcher, RequestDispatcher } from './networkDispatchers'; @@ -152,7 +152,8 @@ export class FrameDispatcher extends Dispatcher impleme } async selectOption(params: { selector: string, elements?: ElementHandleChannel[], options?: types.SelectOption[] } & types.NavigatingActionWaitOptions): Promise<{ values: string[] }> { - return { values: await this._frame.selectOption(params.selector, convertSelectOptionValues(params.elements, params.options), params) }; + const elements = (params.elements || []).map(e => (e as ElementHandleDispatcher)._elementHandle); + return { values: await this._frame.selectOption(params.selector, elements, params.options || [], params) }; } async setInputFiles(params: { selector: string, files: { name: string, mimeType: string, buffer: string }[] } & types.NavigatingActionWaitOptions): Promise {