From 2061dd47181b024ee90e37349736f154652bc74b Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Mon, 28 Jan 2019 14:24:53 -0500 Subject: [PATCH] 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 | 8 ++++---- lib/DOMWorld.js | 16 +++++++++++----- lib/FrameManager.js | 6 +++--- lib/Page.js | 4 ++-- test/waittask.spec.js | 13 ++++++++++++- 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/docs/api.md b/docs/api.md index 0aca429fb4cab..f1cffee59fd42 100644 --- a/docs/api.md +++ b/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]> 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]> 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]> 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]> 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 diff --git a/lib/DOMWorld.js b/lib/DOMWorld.js index f05949e57cec9..f91410d34f437 100644 --- a/lib/DOMWorld.js +++ b/lib/DOMWorld.js @@ -429,7 +429,7 @@ class DOMWorld { /** * @param {string} selector * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options - * @return {!Promise} + * @return {!Promise} */ 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} + * @return {!Promise} */ 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} + * @return {!Promise} */ - _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 diff --git a/lib/FrameManager.js b/lib/FrameManager.js index 513dbf9ee171b..126fdb1d39ded 100644 --- a/lib/FrameManager.js +++ b/lib/FrameManager.js @@ -585,7 +585,7 @@ class Frame { * @param {(string|number|Function)} selectorOrFunctionOrTimeout * @param {!Object=} options * @param {!Array<*>} args - * @return {!Promise} + * @return {!Promise} */ 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} + * @return {!Promise} */ 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} + * @return {!Promise} */ waitForXPath(xpath, options) { return this._mainWorld.waitForXPath(xpath, options); diff --git a/lib/Page.js b/lib/Page.js index 2e588efc04bf6..cefe261a2728b 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -1049,7 +1049,7 @@ class Page extends EventEmitter { /** * @param {string} selector * @param {!{visible?: boolean, hidden?: boolean, timeout?: number}=} options - * @return {!Promise} + * @return {!Promise} */ 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} + * @return {!Promise} */ waitForXPath(xpath, options = {}) { return this.mainFrame().waitForXPath(xpath, options); diff --git a/test/waittask.spec.js b/test/waittask.spec.js index c23dd4fc6d176..2b77eec9739b9 100644 --- a/test/waittask.spec.js +++ b/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(`
anything
`); 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');