fix(page): teach waitForSelector to return null (#3846)

`page.waitForSelector` should return `null` if waiting for `hidden:
true` and there's no matching node in DOM.

Before this patch, `page.waitForSelector` would return some JSHandle
pointing to boolean value.

docs/api.md

@@ -1926,7 +1926,7 @@ return finalResponse.ok();
   - `visible` <[boolean]> wait for element to be present in DOM and to be visible, i.e. to not have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `hidden` <[boolean]> wait for element to not be found in the DOM or to be hidden, i.e. have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `timeout` <[number]> maximum time to wait for in milliseconds. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
-- returns: <[Promise]<[ElementHandle]>> Promise which resolves when element specified by selector string is added to DOM.
+- returns: <[Promise]<?[ElementHandle]>> Promise which resolves when element specified by selector string is added to DOM. Resolves to `null` if waiting for `hidden: true` and selector is not found in DOM.
 
 Wait for the `selector` to appear in page. If at the moment of calling
 the method the `selector` already exists, the method will return
@@ -1955,7 +1955,7 @@ Shortcut for [page.mainFrame().waitForSelector(selector[, options])](#framewaitf
   - `visible` <[boolean]> wait for element to be present in DOM and to be visible, i.e. to not have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `hidden` <[boolean]> wait for element to not be found in the DOM or to be hidden, i.e. have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `timeout` <[number]> maximum time to wait for in milliseconds. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
-- returns: <[Promise]<[ElementHandle]>> Promise which resolves when element specified by xpath string is added to DOM.
+- returns: <[Promise]<?[ElementHandle]>> Promise which resolves when element specified by xpath string is added to DOM. Resolves to `null` if waiting for `hidden: true` and xpath is not found in DOM.
 
 Wait for the `xpath` to appear in page. If at the moment of calling
 the method the `xpath` already exists, the method will return
@@ -2725,7 +2725,7 @@ const [response] = await Promise.all([
   - `visible` <[boolean]> wait for element to be present in DOM and to be visible, i.e. to not have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `hidden` <[boolean]> wait for element to not be found in the DOM or to be hidden, i.e. have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `timeout` <[number]> maximum time to wait for in milliseconds. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
-- returns: <[Promise]<[ElementHandle]>> Promise which resolves when element specified by selector string is added to DOM.
+- returns: <[Promise]<?[ElementHandle]>> Promise which resolves when element specified by selector string is added to DOM. Resolves to `null` if waiting for `hidden: true` and selector is not found in DOM.
 
 Wait for the `selector` to appear in page. If at the moment of calling
 the method the `selector` already exists, the method will return
@@ -2753,7 +2753,7 @@ puppeteer.launch().then(async browser => {
   - `visible` <[boolean]> wait for element to be present in DOM and to be visible, i.e. to not have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `hidden` <[boolean]> wait for element to not be found in the DOM or to be hidden, i.e. have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
   - `timeout` <[number]> maximum time to wait for in milliseconds. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
-- returns: <[Promise]<[ElementHandle]>> Promise which resolves when element specified by xpath string is added to DOM.
+- returns: <[Promise]<?[ElementHandle]>> Promise which resolves when element specified by xpath string is added to DOM. Resolves to `null` if waiting for `hidden: true` and xpath is not found in DOM.
 
 Wait for the `xpath` to appear in page. If at the moment of calling
 the method the `xpath` already exists, the method will return

lib/DOMWorld.js

@@ -429,7 +429,7 @@ class DOMWorld {
   /**
    * @param {string} selector
    * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options
-   * @return {!Promise<!Puppeteer.ElementHandle>}
+   * @return {!Promise<?Puppeteer.ElementHandle>}
    */
   waitForSelector(selector, options) {
     return this._waitForSelectorOrXPath(selector, false, options);
@@ -438,7 +438,7 @@ class DOMWorld {
   /**
    * @param {string} xpath
    * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options
-   * @return {!Promise<!Puppeteer.ElementHandle>}
+   * @return {!Promise<?Puppeteer.ElementHandle>}
    */
   waitForXPath(xpath, options) {
     return this._waitForSelectorOrXPath(xpath, true, options);
@@ -468,9 +468,9 @@ class DOMWorld {
    * @param {string} selectorOrXPath
    * @param {boolean} isXPath
    * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options
-   * @return {!Promise<!Puppeteer.ElementHandle>}
+   * @return {!Promise<?Puppeteer.ElementHandle>}
    */
-  _waitForSelectorOrXPath(selectorOrXPath, isXPath, options = {}) {
+  async _waitForSelectorOrXPath(selectorOrXPath, isXPath, options = {}) {
     const {
       visible: waitForVisible = false,
       hidden: waitForHidden = false,
@@ -478,7 +478,13 @@ class DOMWorld {
     } = options;
     const polling = waitForVisible || waitForHidden ? 'raf' : 'mutation';
     const title = `${isXPath ? 'XPath' : 'selector'} "${selectorOrXPath}"${waitForHidden ? ' to be hidden' : ''}`;
-    return new WaitTask(this, predicate, title, polling, timeout, selectorOrXPath, isXPath, waitForVisible, waitForHidden).promise;
+    const waitTask = new WaitTask(this, predicate, title, polling, timeout, selectorOrXPath, isXPath, waitForVisible, waitForHidden);
+    const handle = await waitTask.promise;
+    if (!handle.asElement()) {
+      await handle.dispose();
+      return null;
+    }
+    return handle.asElement();
 
     /**
      * @param {string} selectorOrXPath

lib/FrameManager.js

@@ -585,7 +585,7 @@ class Frame {
    * @param {(string|number|Function)} selectorOrFunctionOrTimeout
    * @param {!Object=} options
    * @param {!Array<*>} args
-   * @return {!Promise<!Puppeteer.JSHandle>}
+   * @return {!Promise<?Puppeteer.JSHandle>}
    */
   waitFor(selectorOrFunctionOrTimeout, options = {}, ...args) {
     const xPathPattern = '//';
@@ -606,7 +606,7 @@ class Frame {
   /**
    * @param {string} selector
    * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options
-   * @return {!Promise<!Puppeteer.ElementHandle>}
+   * @return {!Promise<?Puppeteer.ElementHandle>}
    */
   waitForSelector(selector, options) {
     return this._mainWorld.waitForSelector(selector, options);
@@ -615,7 +615,7 @@ class Frame {
   /**
    * @param {string} xpath
    * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options
-   * @return {!Promise<!Puppeteer.ElementHandle>}
+   * @return {!Promise<?Puppeteer.ElementHandle>}
    */
   waitForXPath(xpath, options) {
     return this._mainWorld.waitForXPath(xpath, options);

lib/Page.js

@@ -1049,7 +1049,7 @@ class Page extends EventEmitter {
   /**
    * @param {string} selector
    * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options
-   * @return {!Promise<!Puppeteer.ElementHandle>}
+   * @return {!Promise<?Puppeteer.ElementHandle>}
    */
   waitForSelector(selector, options = {}) {
     return this.mainFrame().waitForSelector(selector, options);
@@ -1058,7 +1058,7 @@ class Page extends EventEmitter {
   /**
    * @param {string} xpath
    * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options
-   * @return {!Promise<!Puppeteer.ElementHandle>}
+   * @return {!Promise<?Puppeteer.ElementHandle>}
    */
   waitForXPath(xpath, options = {}) {
     return this.mainFrame().waitForXPath(xpath, options);

test/waittask.spec.js

@@ -17,6 +17,13 @@
 const utils = require('./utils');
 const {TimeoutError} = utils.requireRoot('Errors');
 
+let asyncawait = true;
+try {
+  new Function('async function foo() {await 1}');
+} catch (e) {
+  asyncawait = false;
+}
+
 module.exports.addTests = function({testRunner, expect, product}) {
   const {describe, xdescribe, fdescribe} = testRunner;
   const {it, fit, xit} = testRunner;
@@ -322,6 +329,10 @@ module.exports.addTests = function({testRunner, expect, product}) {
       expect(await waitForSelector).toBe(true);
       expect(divRemoved).toBe(true);
     });
+    it('should return null if waiting to hide non-existing element', async({page, server}) => {
+      const handle = await page.waitForSelector('non-existing', { hidden: true });
+      expect(handle).toBe(null);
+    });
     it('should respect timeout', async({page, server}) => {
       let error = null;
       await page.waitForSelector('div', {timeout: 10}).catch(e => error = e);
@@ -350,7 +361,7 @@ module.exports.addTests = function({testRunner, expect, product}) {
       await page.setContent(`<div class='zombo'>anything</div>`);
       expect(await page.evaluate(x => x.textContent, await waitForSelector)).toBe('anything');
     });
-    it('should have correct stack trace for timeout', async({page, server}) => {
+    (asyncawait ? it : xit)('should have correct stack trace for timeout', async({page, server}) => {
       let error;
       await page.waitForSelector('.zombo', {timeout: 10}).catch(e => error = e);
       expect(error.stack).toContain('waittask.spec.js');