feat(select): use ElementHandle instead of 'id=foo' when choosing options (#54)

docs/api.md

@@ -1593,10 +1593,9 @@ Page is guaranteed to have a main frame which persists during navigations.
 
 #### page.select(selector, ...values)
 - `selector` <[string]> A [selector] to query page for.
-- `...values` <...[string]|[Object]> Options to select. If the `<select>` 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.
+- `...values` <...[string]|[ElementHandle]|[Object]> Options to select. If the `<select>` 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`.
-  - `id` <[string]> Matches by `option.id`.
   - `index` <[number]> Matches by the index.
 - returns: <[Promise]<[Array]<[string]>>> An array of option values that have been successfully selected.
 
@@ -2813,10 +2812,9 @@ If the name is empty, returns the id attribute instead.
 
 #### frame.select(selector, ...values)
 - `selector` <[string]> A [selector] to query frame for.
-- `...values` <...[string]|[Object]> Options to select. If the `<select>` 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.
+- `...values` <...[string]|[ElementHandle]|[Object]> Options to select. If the `<select>` 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`.
-  - `id` <[string]> Matches by `option.id`.
   - `index` <[number]> Matches by the index.
 - returns: <[Promise]<[Array]<[string]>>> An array of option values that have been successfully selected.
 
@@ -3539,10 +3537,9 @@ This method scrolls element into view if needed, and then uses [page.screenshot]
 If the element is detached from DOM, the method throws an error.
 
 #### elementHandle.select(...values)
-- `...values` <...[string]|[Object]> Options to select. If the `<select>` 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.
+- `...values` <...[string]|[ElementHandle]|[Object]> Options to select. If the `<select>` 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`.
-  - `id` <[string]> Matches by `option.id`.
   - `index` <[number]> Matches by the index.
 - returns: <[Promise]<[Array]<[string]>>> An array of option values that have been successfully selected.
 

src/chromium/DOMWorld.ts

@@ -322,7 +322,7 @@ export class DOMWorld {
     await handle.dispose();
   }
 
-  async select(selector: string, ...values: (string | SelectOption)[]): Promise<string[]> {
+  async select(selector: string, ...values: (string | ElementHandle | SelectOption)[]): Promise<string[]> {
     const handle = await this.$(selector);
     assert(handle, 'No node found for selector: ' + selector);
     const result = await handle.select(...values);

src/chromium/Frame.ts

@@ -164,8 +164,10 @@ export class Frame {
     return this._secondaryWorld.hover(selector, options);
   }
 
-  select(selector: string, ...values: (string | SelectOption)[]): Promise<string[]>{
+  async select(selector: string, ...values: (string | ElementHandle | SelectOption)[]): Promise<string[]>{
-    return this._secondaryWorld.select(selector, ...values);
+    const secondaryExecutionContext = await this._secondaryWorld.executionContext();
+    const adoptedValues = values.map(async value => value instanceof ElementHandle ? secondaryExecutionContext._adoptElementHandle(value) : value);
+    return this._secondaryWorld.select(selector, ...(await Promise.all(adoptedValues)));
   }
 
   async type(selector: string, text: string, options: { delay: (number | undefined); } | undefined) {

src/chromium/JSHandle.ts

@@ -49,7 +49,6 @@ export type MultiClickOptions = PointerActionOptions & {
 
 export type SelectOption = {
   value?: string;
-  id?: string;
   label?: string;
   index?: number;
 };
@@ -324,19 +323,19 @@ export class ElementHandle extends JSHandle {
     return this._performPointerAction(point => this._page.mouse.tripleclick(point.x, point.y, options), options);
   }
 
-  async select(...values: (string | SelectOption)[]): Promise<string[]> {
+  async select(...values: (string | ElementHandle | SelectOption)[]): Promise<string[]> {
     const options = values.map(value => typeof value === 'object' ? value : { value });
     for (const option of options) {
+      if (option instanceof ElementHandle)
+        continue;
       if (option.value !== undefined)
         assert(helper.isString(option.value), 'Values must be strings. Found value "' + option.value + '" of type "' + (typeof option.value) + '"');
       if (option.label !== undefined)
         assert(helper.isString(option.label), 'Labels must be strings. Found label "' + option.label + '" of type "' + (typeof option.label) + '"');
-      if (option.id !== undefined)
-        assert(helper.isString(option.id), 'Ids must be strings. Found id "' + option.id + '" of type "' + (typeof option.id) + '"');
       if (option.index !== undefined)
         assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"');
     }
-    return this.evaluate((element: HTMLSelectElement, optionsToSelect: SelectOption[]) => {
+    return this.evaluate((element: HTMLSelectElement, ...optionsToSelect: (Node | SelectOption)[]) => {
       if (element.nodeName.toLowerCase() !== 'select')
         throw new Error('Element is not a <select> element.');
 
@@ -345,13 +344,13 @@ export class ElementHandle extends JSHandle {
       for (let index = 0; index < options.length; index++) {
         const option = options[index];
         option.selected = optionsToSelect.some(optionToSelect => {
+          if (optionToSelect instanceof Node)
+            return option === optionToSelect;
           let matches = true;
           if (optionToSelect.value !== undefined)
             matches = matches && optionToSelect.value === option.value;
           if (optionToSelect.label !== undefined)
             matches = matches && optionToSelect.label === option.label;
-          if (optionToSelect.id !== undefined)
-            matches = matches && optionToSelect.id === option.id;
           if (optionToSelect.index !== undefined)
             matches = matches && optionToSelect.index === index;
           return matches;
@@ -362,7 +361,7 @@ export class ElementHandle extends JSHandle {
       element.dispatchEvent(new Event('input', { 'bubbles': true }));
       element.dispatchEvent(new Event('change', { 'bubbles': true }));
       return options.filter(option => option.selected).map(option => option.value);
-    }, options);
+    }, ...options);
   }
 
   async fill(value: string): Promise<void> {

src/chromium/Page.ts

@@ -704,7 +704,7 @@ export class Page extends EventEmitter {
     return this.mainFrame().hover(selector, options);
   }
 
-  select(selector: string, ...values: (string | SelectOption)[]): Promise<string[]> {
+  select(selector: string, ...values: (string | ElementHandle | SelectOption)[]): Promise<string[]> {
     return this.mainFrame().select(selector, ...values);
   }
 

test/page.spec.js

@@ -902,9 +902,9 @@ module.exports.addTests = function({testRunner, expect, headless, playwright, FF
       expect(await page.evaluate(() => result.onInput)).toEqual(['indigo']);
       expect(await page.evaluate(() => result.onChange)).toEqual(['indigo']);
     });
-    it('should select single option by id', async({page, server}) => {
+    it('should select single option by handle', async({page, server}) => {
       await page.goto(server.PREFIX + '/input/select.html');
-      await page.select('select', { id: 'whiteOption' });
+      await page.select('select', await page.$('[id=whiteOption]'));
       expect(await page.evaluate(() => result.onInput)).toEqual(['white']);
       expect(await page.evaluate(() => result.onChange)).toEqual(['white']);
     });
@@ -1028,14 +1028,6 @@ module.exports.addTests = function({testRunner, expect, headless, playwright, FF
       }
       expect(error.message).toContain('Labels must be strings');
 
-      error = null;
-      try {
-        await page.select('select', { id: 12 });
-      } catch (e) {
-        error = e;
-      }
-      expect(error.message).toContain('Ids must be strings');
-
       error = null;
       try {
         await page.select('select', { index: '12' });