chore: simplify conversions around selectOption (#3517)

We do not need to support api types on the server side.
microsoft · Aug 18, 2020 · ecf4cd3 · ecf4cd3
1 parent aeadf50
commit ecf4cd3
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 39 deletions.
diff --git a/src/dom.ts b/src/dom.ts
@@ -397,35 +397,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     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<string[]> {
+  async selectOption(elements: ElementHandle[], values: types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise<string[]> {
     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<string[] | 'error:notconnected'> {
-    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<string[] | 'error:notconnected'> {
+    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
@@ -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<string[]> {
-    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<string[]> {
+    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<void> {

diff --git a/src/rpc/client/elementHandle.ts b/src/rpc/client/elementHandle.ts
@@ -224,15 +224,14 @@ export class ElementHandle<T extends Node = Node> extends JSHandle<T> {
 }
 
 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 ];
   if (!values.length)
     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
@@ -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
@@ -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<Frame, FrameInitializer> 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<void> {