chore: simplify conversions around setInputFiles (#3516)

We do not need api types on the server side anymore.
microsoft · Aug 19, 2020 · 3cf48f9 · 3cf48f9
1 parent ecf4cd3
commit 3cf48f9
Show file tree

Hide file tree

Showing 11 changed files with 45 additions and 67 deletions.
diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts
@@ -270,7 +270,7 @@ export class CRPage implements PageDelegate {
 
   async setInputFiles(handle: dom.ElementHandle<HTMLInputElement>, files: types.FilePayload[]): Promise<void> {
     await handle._evaluateInUtility(([injected, node, files]) =>
-      injected.setInputFiles(node, files), dom.toFileTransferPayload(files));
+      injected.setInputFiles(node, files), files);
   }
 
   async adoptElementHandle<T extends Node>(handle: dom.ElementHandle<T>, to: dom.FrameExecutionContext): Promise<dom.ElementHandle<T>> {

diff --git a/src/converters.ts b/src/converters.ts
@@ -14,34 +14,8 @@
  * limitations under the License.
  */
 
-import * as fs from 'fs';
-import * as mime from 'mime';
-import * as path from 'path';
-import * as util from 'util';
 import * as types from './types';
 
-export async function normalizeFilePayloads(files: string | types.FilePayload | string[] | types.FilePayload[]): Promise<types.FilePayload[]> {
-  let ff: string[] | types.FilePayload[];
-  if (!Array.isArray(files))
-    ff = [ files ] as string[] | types.FilePayload[];
-  else
-    ff = files;
-  const filePayloads: types.FilePayload[] = [];
-  for (const item of ff) {
-    if (typeof item === 'string') {
-      const file: types.FilePayload = {
-        name: path.basename(item),
-        mimeType: mime.getType(item) || 'application/octet-stream',
-        buffer: await util.promisify(fs.readFile)(item)
-      };
-      filePayloads.push(file);
-    } else {
-      filePayloads.push(item);
-    }
-  }
-  return filePayloads;
-}
-
 export function headersObjectToArray(headers: { [key: string]: string }): types.HeadersArray {
   const result: types.HeadersArray = [];
   for (const name in headers) {

diff --git a/src/dom.ts b/src/dom.ts
@@ -26,7 +26,6 @@ import * as types from './types';
 import { Progress } from './progress';
 import DebugScript from './debug/injected/debugScript';
 import { FatalDOMError, RetargetableDOMError } from './common/domErrors';
-import { normalizeFilePayloads } from './converters';
 
 export class FrameExecutionContext extends js.ExecutionContext {
   readonly frame: frames.Frame;
@@ -458,14 +457,14 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     }, this._page._timeoutSettings.timeout(options));
   }
 
-  async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}) {
+  async setInputFiles(files: types.FilePayload[], options: types.NavigatingActionWaitOptions = {}) {
     return this._page._runAbortableTask(async progress => {
       const result = await this._setInputFiles(progress, files, options);
       return assertDone(throwRetargetableDOMError(result));
     }, this._page._timeoutSettings.timeout(options));
   }
 
-  async _setInputFiles(progress: Progress, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
+  async _setInputFiles(progress: Progress, files: types.FilePayload[], options: types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
     const multiple = throwFatalDOMError(await this._evaluateInUtility(([injected, node]): 'error:notinput' | 'error:notconnected' | boolean => {
       if (node.nodeType !== Node.ELEMENT_NODE || (node as Node as Element).tagName !== 'INPUT')
         return 'error:notinput';
@@ -476,11 +475,10 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     }, {}));
     if (typeof multiple === 'string')
       return multiple;
-    const filePayloads = await normalizeFilePayloads(files);
-    assert(multiple || filePayloads.length <= 1, 'Non-multiple file input can only accept single file!');
+    assert(multiple || files.length <= 1, 'Non-multiple file input can only accept single file!');
     await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
       progress.throwIfAborted();  // Avoid action that has side-effects.
-      await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
+      await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, files);
     });
     return 'done';
   }
@@ -762,14 +760,6 @@ export class InjectedScriptPollHandler<T> {
   }
 }
 
-export function toFileTransferPayload(files: types.FilePayload[]): types.FileTransferPayload[] {
-  return files.map(file => ({
-    name: file.name,
-    type: file.mimeType,
-    data: file.buffer.toString('base64')
-  }));
-}
-
 export function throwFatalDOMError<T>(result: T | FatalDOMError): T {
   if (result === 'error:notelement')
     throw new Error('Node is not an element');

diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts
@@ -458,7 +458,7 @@ export class FFPage implements PageDelegate {
 
   async setInputFiles(handle: dom.ElementHandle<HTMLInputElement>, files: types.FilePayload[]): Promise<void> {
     await handle._evaluateInUtility(([injected, node, files]) =>
-      injected.setInputFiles(node, files), dom.toFileTransferPayload(files));
+      injected.setInputFiles(node, files), files);
   }
 
   async adoptElementHandle<T extends Node>(handle: dom.ElementHandle<T>, to: dom.FrameExecutionContext): Promise<dom.ElementHandle<T>> {

diff --git a/src/frames.ts b/src/frames.ts
@@ -429,7 +429,7 @@ export class Frame {
       const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
       progress.log(`navigating to "${url}", waiting until "${waitUntil}"`);
       const headers = this._page._state.extraHTTPHeaders || [];
-      const refererHeader = headers.find(h => h.name === 'referer' || h.name === 'Referer');
+      const refererHeader = headers.find(h => h.name.toLowerCase() === 'referer');
       let referer = refererHeader ? refererHeader.value : undefined;
       if (options.referer !== undefined) {
         if (referer !== undefined && referer !== options.referer)
@@ -867,7 +867,7 @@ export class Frame {
     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> {
+  async setInputFiles(selector: string, files: types.FilePayload[], options: types.NavigatingActionWaitOptions = {}): Promise<void> {
     await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._setInputFiles(progress, files, options));
   }
 

diff --git a/src/injected/injectedScript.ts b/src/injected/injectedScript.ts
@@ -425,7 +425,7 @@ export default class InjectedScript {
     throw new Error('Not a checkbox');
   }
 
-  async setInputFiles(node: Node, payloads: types.FileTransferPayload[]) {
+  async setInputFiles(node: Node, payloads: types.FilePayload[]) {
     if (node.nodeType !== Node.ELEMENT_NODE)
       return 'Node is not of type HTMLElement';
     const element: Element | undefined = node as Element;
@@ -437,8 +437,8 @@ export default class InjectedScript {
       return 'Not an input[type=file] element';
 
     const files = await Promise.all(payloads.map(async file => {
-      const result = await fetch(`data:${file.type};base64,${file.data}`);
-      return new File([await result.blob()], file.name, {type: file.type});
+      const result = await fetch(`data:${file.mimeType};base64,${file.buffer}`);
+      return new File([await result.blob()], file.name, {type: file.mimeType});
     }));
     const dt = new DataTransfer();
     for (const file of files)

diff --git a/src/rpc/client/elementHandle.ts b/src/rpc/client/elementHandle.ts
@@ -14,13 +14,16 @@
  * limitations under the License.
  */
 
-import { ElementHandleChannel, JSHandleInitializer, ElementHandleScrollIntoViewIfNeededOptions, ElementHandleHoverOptions, ElementHandleClickOptions, ElementHandleDblclickOptions, ElementHandleFillOptions, ElementHandleSetInputFilesOptions, ElementHandlePressOptions, ElementHandleCheckOptions, ElementHandleUncheckOptions, ElementHandleScreenshotOptions, ElementHandleTypeOptions, ElementHandleSelectTextOptions, ElementHandleWaitForSelectorOptions, ElementHandleWaitForElementStateOptions } from '../channels';
+import { ElementHandleChannel, JSHandleInitializer, ElementHandleScrollIntoViewIfNeededOptions, ElementHandleHoverOptions, ElementHandleClickOptions, ElementHandleDblclickOptions, ElementHandleFillOptions, ElementHandleSetInputFilesOptions, ElementHandlePressOptions, ElementHandleCheckOptions, ElementHandleUncheckOptions, ElementHandleScreenshotOptions, ElementHandleTypeOptions, ElementHandleSelectTextOptions, ElementHandleWaitForSelectorOptions, ElementHandleWaitForElementStateOptions, ElementHandleSetInputFilesParams } from '../channels';
 import { Frame } from './frame';
 import { FuncOn, JSHandle, serializeArgument, parseResult } from './jsHandle';
 import { ChannelOwner } from './channelOwner';
 import { helper, assert } from '../../helper';
-import { normalizeFilePayloads } from '../../converters';
 import { SelectOption, FilePayload, Rect, SelectOptionOptions } from './types';
+import * as fs from 'fs';
+import * as mime from 'mime';
+import * as path from 'path';
+import * as util from 'util';
 
 export class ElementHandle<T extends Node = Node> extends JSHandle<T> {
   readonly _elementChannel: ElementHandleChannel;
@@ -239,7 +242,23 @@ export function convertSelectOptionValues(values: string | ElementHandle | Selec
   return { options: values as SelectOption[] };
 }
 
-export async function convertInputFiles(files: string | FilePayload | string[] | FilePayload[]): Promise<{ name: string, mimeType: string, buffer: string }[]> {
-  const filePayloads = await normalizeFilePayloads(files);
-  return filePayloads.map(f => ({ name: f.name, mimeType: f.mimeType, buffer: f.buffer.toString('base64') }));
+type SetInputFilesFiles = ElementHandleSetInputFilesParams['files'];
+export async function convertInputFiles(files: string | FilePayload | string[] | FilePayload[]): Promise<SetInputFilesFiles> {
+  const items: (string | FilePayload)[] = Array.isArray(files) ? files : [ files ];
+  const filePayloads: SetInputFilesFiles = await Promise.all(items.map(async item => {
+    if (typeof item === 'string') {
+      return {
+        name: path.basename(item),
+        mimeType: mime.getType(item) || 'application/octet-stream',
+        buffer: (await util.promisify(fs.readFile)(item)).toString('base64')
+      };
+    } else {
+      return {
+        name: item.name,
+        mimeType: item.mimeType,
+        buffer: item.buffer.toString('base64'),
+      };
+    }
+  }));
+  return filePayloads;
 }
diff --git a/src/rpc/server/elementHandlerDispatcher.ts b/src/rpc/server/elementHandlerDispatcher.ts
@@ -100,7 +100,7 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements Eleme
   }
 
   async setInputFiles(params: { files: { name: string, mimeType: string, buffer: string }[] } & types.NavigatingActionWaitOptions) {
-    await this._elementHandle.setInputFiles(convertInputFiles(params.files), params);
+    await this._elementHandle.setInputFiles(params.files, params);
   }
 
   async focus() {
@@ -158,7 +158,3 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements Eleme
     return { element: ElementHandleDispatcher.createNullable(this._scope, await this._elementHandle.waitForSelector(params.selector, params)) };
   }
 }
-
-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 { ElementHandleDispatcher, createHandle, convertInputFiles } from './elementHandlerDispatcher';
+import { ElementHandleDispatcher, createHandle } from './elementHandlerDispatcher';
 import { parseArgument, serializeResult } from './jsHandleDispatcher';
 import { ResponseDispatcher, RequestDispatcher } from './networkDispatchers';
 
@@ -157,7 +157,7 @@ export class FrameDispatcher extends Dispatcher<Frame, FrameInitializer> impleme
   }
 
   async setInputFiles(params: { selector: string, files: { name: string, mimeType: string, buffer: string }[] } & types.NavigatingActionWaitOptions): Promise<void> {
-    await this._frame.setInputFiles(params.selector, convertInputFiles(params.files), params);
+    await this._frame.setInputFiles(params.selector, params.files, params);
   }
 
   async type(params: { selector: string, text: string } & { delay?: number | undefined } & types.TimeoutOptions & { noWaitAfter?: boolean }): Promise<void> {

diff --git a/src/types.ts b/src/types.ts
@@ -90,13 +90,7 @@ export type SelectOption = {
 export type FilePayload = {
   name: string,
   mimeType: string,
-  buffer: Buffer,
-};
-
-export type FileTransferPayload = {
-  name: string,
-  type: string,
-  data: string,
+  buffer: string,
 };
 
 export type MediaType = 'screen' | 'print';

diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts
@@ -804,7 +804,12 @@ export class WKPage implements PageDelegate {
 
   async setInputFiles(handle: dom.ElementHandle<HTMLInputElement>, files: types.FilePayload[]): Promise<void> {
     const objectId = handle._objectId;
-    await this._session.send('DOM.setInputFiles', { objectId, files: dom.toFileTransferPayload(files) });
+    const protocolFiles = files.map(file => ({
+      name: file.name,
+      type: file.mimeType,
+      data: file.buffer,
+    }));
+    await this._session.send('DOM.setInputFiles', { objectId, files: protocolFiles });
   }
 
   async adoptElementHandle<T extends Node>(handle: dom.ElementHandle<T>, to: dom.FrameExecutionContext): Promise<dom.ElementHandle<T>> {