Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(waitFor): update various waitFor options to be a single boolean #1066

Merged
merged 1 commit into from
Feb 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 26 additions & 38 deletions docs/api.md

Large diffs are not rendered by default.

26 changes: 13 additions & 13 deletions src/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import { Page } from './page';
import * as platform from './platform';
import { Selectors } from './selectors';

export type WaitForInteractableOptions = types.TimeoutOptions & { waitForInteractable?: boolean };

export class FrameExecutionContext extends js.ExecutionContext {
readonly frame: frames.Frame;

Expand Down Expand Up @@ -232,14 +230,16 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return point;
}

async _performPointerAction(action: (point: types.Point) => Promise<void>, options?: input.PointerActionOptions & WaitForInteractableOptions): Promise<void> {
const { waitForInteractable = true } = (options || {});
if (waitForInteractable)
async _performPointerAction(action: (point: types.Point) => Promise<void>, options?: input.PointerActionOptions & types.WaitForOptions): Promise<void> {
const { waitFor = true } = (options || {});
if (!helper.isBoolean(waitFor))
throw new Error('waitFor option should be a boolean, got "' + (typeof waitFor) + '"');
if (waitFor)
await this._waitForStablePosition(options);
const relativePoint = options ? options.relativePoint : undefined;
await this._scrollRectIntoViewIfNeeded(relativePoint ? { x: relativePoint.x, y: relativePoint.y, width: 0, height: 0 } : undefined);
const point = relativePoint ? await this._relativePoint(relativePoint) : await this._clickablePoint();
if (waitForInteractable)
if (waitFor)
await this._waitForHitTargetAt(point, options);
let restoreModifiers: input.Modifier[] | undefined;
if (options && options.modifiers)
Expand All @@ -249,19 +249,19 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
await this._page.keyboard._ensureModifiers(restoreModifiers);
}

hover(options?: input.PointerActionOptions & WaitForInteractableOptions): Promise<void> {
hover(options?: input.PointerActionOptions & types.WaitForOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.move(point.x, point.y), options);
}

click(options?: input.ClickOptions & WaitForInteractableOptions): Promise<void> {
click(options?: input.ClickOptions & types.WaitForOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.click(point.x, point.y, options), options);
}

dblclick(options?: input.MultiClickOptions & WaitForInteractableOptions): Promise<void> {
dblclick(options?: input.MultiClickOptions & types.WaitForOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.dblclick(point.x, point.y, options), options);
}

tripleclick(options?: input.MultiClickOptions & WaitForInteractableOptions): Promise<void> {
tripleclick(options?: input.MultiClickOptions & types.WaitForOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.tripleclick(point.x, point.y, options), options);
}

Expand Down Expand Up @@ -414,15 +414,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
await this._page.keyboard.press(key, options);
}

async check(options?: WaitForInteractableOptions) {
async check(options?: types.WaitForOptions) {
await this._setChecked(true, options);
}

async uncheck(options?: WaitForInteractableOptions) {
async uncheck(options?: types.WaitForOptions) {
await this._setChecked(false, options);
}

private async _setChecked(state: boolean, options: WaitForInteractableOptions = {}) {
private async _setChecked(state: boolean, options?: types.WaitForOptions) {
const isCheckboxChecked = async (): Promise<boolean> => {
return this._evaluateInUtility((node: Node) => {
if (node.nodeType !== Node.ELEMENT_NODE)
Expand Down
37 changes: 19 additions & 18 deletions src/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import * as types from './types';
import * as js from './javascript';
import * as dom from './dom';
import * as network from './network';
import * as input from './input';
import { helper, assert, RegisteredListener } from './helper';
import { ClickOptions, MultiClickOptions, PointerActionOptions } from './input';
import { TimeoutError } from './errors';
import { Events } from './events';
import { Page } from './page';
Expand Down Expand Up @@ -52,7 +52,6 @@ export type GotoResult = {
export type LifecycleEvent = 'load' | 'domcontentloaded' | 'networkidle0' | 'networkidle2';
const kLifecycleEvents: Set<LifecycleEvent> = new Set(['load', 'domcontentloaded', 'networkidle0', 'networkidle2']);

export type WaitForOptions = types.TimeoutOptions & { waitFor?: types.Visibility | 'nowait' };
type ConsoleTagHandler = () => void;

export class FrameManager {
Expand Down Expand Up @@ -781,63 +780,63 @@ export class Frame {
return result!;
}

async click(selector: string, options?: WaitForOptions & ClickOptions & dom.WaitForInteractableOptions) {
async click(selector: string, options?: input.ClickOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.click(options);
await handle.dispose();
}

async dblclick(selector: string, options?: WaitForOptions & MultiClickOptions & dom.WaitForInteractableOptions) {
async dblclick(selector: string, options?: input.MultiClickOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.dblclick(options);
await handle.dispose();
}

async tripleclick(selector: string, options?: WaitForOptions & MultiClickOptions & dom.WaitForInteractableOptions) {
async tripleclick(selector: string, options?: input.MultiClickOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.tripleclick(options);
await handle.dispose();
}

async fill(selector: string, value: string, options?: WaitForOptions) {
async fill(selector: string, value: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.fill(value);
await handle.dispose();
}

async focus(selector: string, options?: WaitForOptions) {
async focus(selector: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.focus();
await handle.dispose();
}

async hover(selector: string, options?: WaitForOptions & PointerActionOptions & dom.WaitForInteractableOptions) {
async hover(selector: string, options?: input.PointerActionOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.hover(options);
await handle.dispose();
}

async select(selector: string, value: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[] | undefined, options?: WaitForOptions): Promise<string[]> {
async select(selector: string, value: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[] | undefined, options?: types.WaitForOptions): Promise<string[]> {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
const values = value === undefined ? [] : Array.isArray(value) ? value : [value];
const result = await handle.select(...values);
await handle.dispose();
return result;
}

async type(selector: string, text: string, options?: WaitForOptions & { delay?: number }) {
async type(selector: string, text: string, options?: { delay?: number } & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.type(text, options);
await handle.dispose();
}

async check(selector: string, options?: WaitForOptions & dom.WaitForInteractableOptions) {
async check(selector: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.check(options);
await handle.dispose();
}

async uncheck(selector: string, options?: WaitForOptions & dom.WaitForInteractableOptions) {
async uncheck(selector: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.uncheck(options);
await handle.dispose();
Expand All @@ -853,13 +852,15 @@ export class Frame {
return Promise.reject(new Error('Unsupported target type: ' + (typeof selectorOrFunctionOrTimeout)));
}

private async _optionallyWaitForSelectorInUtilityContext(selector: string, options: WaitForOptions | undefined): Promise<dom.ElementHandle<Element>> {
const { timeout = this._page._timeoutSettings.timeout(), waitFor = 'visible' } = (options || {});
private async _optionallyWaitForSelectorInUtilityContext(selector: string, options: types.WaitForOptions | undefined): Promise<dom.ElementHandle<Element>> {
const { timeout = this._page._timeoutSettings.timeout(), waitFor = true } = (options || {});
if (!helper.isBoolean(waitFor))
throw new Error('waitFor option should be a boolean, got "' + (typeof waitFor) + '"');
let handle: dom.ElementHandle<Element>;
if (waitFor !== 'nowait') {
const maybeHandle = await this._waitForSelectorInUtilityContext(selector, waitFor, timeout);
if (waitFor) {
const maybeHandle = await this._waitForSelectorInUtilityContext(selector, 'any', timeout);
if (!maybeHandle)
throw new Error('No node found for selector: ' + selectorToString(selector, waitFor));
throw new Error('No node found for selector: ' + selectorToString(selector, 'any'));
handle = maybeHandle;
} else {
const context = await this._context('utility');
Expand All @@ -875,7 +876,7 @@ export class Frame {
if (waitFor === 'visible' || waitFor === 'hidden' || waitFor === 'any')
visibility = waitFor;
else
throw new Error(`Unsupported waitFor option "${waitFor}"`);
throw new Error(`Unsupported visibility option "${waitFor}"`);
const task = dom.waitForSelectorTask(selector, visibility, timeout);
const result = await this._scheduleRerunnableTask(task, 'utility', timeout, `selector "${selectorToString(selector, visibility)}"`);
if (!result.asElement()) {
Expand Down
8 changes: 8 additions & 0 deletions src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ class Helper {
return typeof obj === 'number' || obj instanceof Number;
}

static isObject(obj: any): obj is NonNullable<object> {
return typeof obj === 'object' && obj !== null;
}

static isBoolean(obj: any): obj is boolean {
return typeof obj === 'boolean' || obj instanceof Boolean;
}

static async waitForEvent(
emitter: platform.EventEmitterType,
eventName: (string | symbol),
Expand Down
20 changes: 10 additions & 10 deletions src/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,43 +485,43 @@ export class Page extends platform.EventEmitter {
return this._closed;
}

async click(selector: string, options?: frames.WaitForOptions & input.ClickOptions & dom.WaitForInteractableOptions) {
async click(selector: string, options?: input.ClickOptions & types.WaitForOptions) {
return this.mainFrame().click(selector, options);
}

async dblclick(selector: string, options?: frames.WaitForOptions & input.MultiClickOptions & dom.WaitForInteractableOptions) {
async dblclick(selector: string, options?: input.MultiClickOptions & types.WaitForOptions) {
return this.mainFrame().dblclick(selector, options);
}

async tripleclick(selector: string, options?: frames.WaitForOptions & input.MultiClickOptions & dom.WaitForInteractableOptions) {
async tripleclick(selector: string, options?: input.MultiClickOptions & types.WaitForOptions) {
return this.mainFrame().tripleclick(selector, options);
}

async fill(selector: string, value: string, options?: frames.WaitForOptions) {
async fill(selector: string, value: string, options?: types.WaitForOptions) {
return this.mainFrame().fill(selector, value, options);
}

async focus(selector: string, options?: frames.WaitForOptions) {
async focus(selector: string, options?: types.WaitForOptions) {
return this.mainFrame().focus(selector, options);
}

async hover(selector: string, options?: frames.WaitForOptions & input.PointerActionOptions & dom.WaitForInteractableOptions) {
async hover(selector: string, options?: input.PointerActionOptions & types.WaitForOptions) {
return this.mainFrame().hover(selector, options);
}

async select(selector: string, value: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[] | undefined, options?: frames.WaitForOptions): Promise<string[]> {
async select(selector: string, value: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[] | undefined, options?: types.WaitForOptions): Promise<string[]> {
return this.mainFrame().select(selector, value, options);
}

async type(selector: string, text: string, options?: frames.WaitForOptions & { delay?: number }) {
async type(selector: string, text: string, options?: { delay?: number } & types.WaitForOptions) {
return this.mainFrame().type(selector, text, options);
}

async check(selector: string, options?: frames.WaitForOptions & dom.WaitForInteractableOptions) {
async check(selector: string, options?: types.WaitForOptions) {
return this.mainFrame().check(selector, options);
}

async uncheck(selector: string, options?: frames.WaitForOptions & dom.WaitForInteractableOptions) {
async uncheck(selector: string, options?: types.WaitForOptions) {
return this.mainFrame().uncheck(selector, options);
}

Expand Down
2 changes: 2 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export type Rect = Size & Point;
export type Quad = [ Point, Point, Point, Point ];

export type TimeoutOptions = { timeout?: number };
export type WaitForOptions = TimeoutOptions & { waitFor?: boolean };

export type Visibility = 'visible' | 'hidden' | 'any';

export type Polling = 'raf' | 'mutation' | number;
Expand Down
23 changes: 8 additions & 15 deletions test/click.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,25 +142,18 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
await page.click('button');
expect(await page.evaluate(() => result)).toBe('Clicked');
});
it('should waitFor hidden when already hidden', async({page, server}) => {
it('should not wait with false waitFor', async({page, server}) => {
let error = null;
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', b => b.style.display = 'none');
await page.click('button', { waitFor: 'hidden' }).catch(e => error = e);
await page.click('button', { waitFor: false }).catch(e => error = e);
expect(error.message).toBe('Node is either not visible or not an HTMLElement');
expect(await page.evaluate(() => result)).toBe('Was not clicked');
});
it('should waitFor hidden', async({page, server}) => {
let error = null;
it('should throw for non-boolean waitFor', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
const clicked = page.click('button', { waitFor: 'hidden' }).catch(e => error = e);
for (let i = 0; i < 5; i++)
await page.evaluate('1'); // Do a round trip.
expect(error).toBe(null);
await page.$eval('button', b => b.style.display = 'none');
await clicked;
expect(error.message).toBe('Node is either not visible or not an HTMLElement');
expect(await page.evaluate(() => result)).toBe('Was not clicked');
const error = await page.click('button', { waitFor: 'visible' }).catch(e => e);
expect(error.message).toBe('waitFor option should be a boolean, got "string"');
});
it('should waitFor visible', async({page, server}) => {
let done = false;
Expand Down Expand Up @@ -218,7 +211,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
it('should fail to click a missing button', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
let error = null;
await page.click('button.does-not-exist', { waitFor: 'nowait' }).catch(e => error = e);
await page.click('button.does-not-exist', { waitFor: false }).catch(e => error = e);
expect(error.message).toBe('No node found for selector: button.does-not-exist');
});
// @see https://github.com/GoogleChrome/puppeteer/issues/161
Expand Down Expand Up @@ -451,7 +444,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
const error = await button.click({ timeout: 100 }).catch(e => e);
expect(error.message).toContain('timeout 100ms exceeded');
});
it('should fail when obscured and not waiting for interactable', async({page, server}) => {
it('should fail when obscured and not waiting for hit target', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
const button = await page.$('button');
await page.evaluate(() => {
Expand All @@ -464,7 +457,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
blocker.style.top = '0';
document.body.appendChild(blocker);
});
await button.click({ waitForInteractable: false });
await button.click({ waitFor: false });
expect(await page.evaluate(() => window.result)).toBe('Was not clicked');
});

Expand Down
4 changes: 2 additions & 2 deletions test/page.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1031,12 +1031,12 @@ module.exports.describe = function({testRunner, expect, headless, playwright, FF
it('should throw on hidden and invisible elements', async({page, server}) => {
await page.goto(server.PREFIX + '/input/textarea.html');
await page.$eval('input', i => i.style.display = 'none');
const invisibleError = await page.fill('input', 'some value', { waitFor: 'nowait' }).catch(e => e);
const invisibleError = await page.fill('input', 'some value', { waitFor: false }).catch(e => e);
expect(invisibleError.message).toBe('Element is not visible');

await page.goto(server.PREFIX + '/input/textarea.html');
await page.$eval('input', i => i.style.visibility = 'hidden');
const hiddenError = await page.fill('input', 'some value', { waitFor: 'nowait' }).catch(e => e);
const hiddenError = await page.fill('input', 'some value', { waitFor: false }).catch(e => e);
expect(hiddenError.message).toBe('Element is hidden');
});
it('should be able to fill the body', async({page}) => {
Expand Down
8 changes: 4 additions & 4 deletions test/waittask.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,22 +375,22 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO
it('should throw for unknown waitFor option', async({page, server}) => {
await page.setContent('<section>test</section>');
const error = await page.waitForSelector('section', { visibility: 'foo' }).catch(e => e);
expect(error.message).toContain('Unsupported waitFor option');
expect(error.message).toContain('Unsupported visibility option');
});
it('should throw for numeric waitFor option', async({page, server}) => {
await page.setContent('<section>test</section>');
const error = await page.waitForSelector('section', { visibility: 123 }).catch(e => e);
expect(error.message).toContain('Unsupported waitFor option');
expect(error.message).toContain('Unsupported visibility option');
});
it('should throw for true waitFor option', async({page, server}) => {
await page.setContent('<section>test</section>');
const error = await page.waitForSelector('section', { visibility: true }).catch(e => e);
expect(error.message).toContain('Unsupported waitFor option');
expect(error.message).toContain('Unsupported visibility option');
});
it('should throw for false waitFor option', async({page, server}) => {
await page.setContent('<section>test</section>');
const error = await page.waitForSelector('section', { visibility: false }).catch(e => e);
expect(error.message).toContain('Unsupported waitFor option');
expect(error.message).toContain('Unsupported visibility option');
});
it('should support >> selector syntax', async({page, server}) => {
await page.goto(server.EMPTY_PAGE);
Expand Down