From 40b64a5b60c8a02a9f76c56b134e0df1953257b1 Mon Sep 17 00:00:00 2001 From: Joel Einbinder Date: Thu, 5 Dec 2019 20:48:09 +0000 Subject: [PATCH] dgozman comments --- src/chromium/JSHandle.ts | 4 +-- src/chromium/Page.ts | 6 ++--- src/chromium/Playwright.ts | 2 +- src/dom.ts | 53 +++++++++++++++++++++----------------- src/firefox/JSHandle.ts | 5 ++-- src/frames.ts | 6 ++--- src/injected/injected.ts | 6 +++-- src/input.ts | 10 ++++--- src/javascript.ts | 8 +++--- src/types.ts | 16 +++++++----- src/webkit/JSHandle.ts | 2 +- 11 files changed, 67 insertions(+), 51 deletions(-) diff --git a/src/chromium/JSHandle.ts b/src/chromium/JSHandle.ts index 0c0d2ddf89..afd83f20f1 100644 --- a/src/chromium/JSHandle.ts +++ b/src/chromium/JSHandle.ts @@ -194,11 +194,11 @@ export class DOMWorldDelegate implements dom.DOMWorldDelegate { await handle.evaluate(input.setFileInputFunction, files); } - async adoptElementHandle(handle: dom.ElementHandle, to: dom.DOMWorld): Promise { + async adoptElementHandle(handle: dom.ElementHandle, to: dom.DOMWorld): Promise> { const nodeInfo = await this._client.send('DOM.describeNode', { objectId: toRemoteObject(handle).objectId, }); - return this.adoptBackendNodeId(nodeInfo.node.backendNodeId, to); + return this.adoptBackendNodeId(nodeInfo.node.backendNodeId, to) as Promise>; } async adoptBackendNodeId(backendNodeId: Protocol.DOM.BackendNodeId, to: dom.DOMWorld): Promise { diff --git a/src/chromium/Page.ts b/src/chromium/Page.ts index 90c500eed0..e4e01f430b 100644 --- a/src/chromium/Page.ts +++ b/src/chromium/Page.ts @@ -222,7 +222,7 @@ export class Page extends EventEmitter { this._timeoutSettings.setDefaultTimeout(timeout); } - async $(selector: string | types.Selector): Promise { + async $(selector: string | types.Selector): Promise | null> { return this.mainFrame().$(selector); } @@ -239,11 +239,11 @@ export class Page extends EventEmitter { return this.mainFrame().$$eval(selector, pageFunction, ...args as any); } - async $$(selector: string | types.Selector): Promise { + async $$(selector: string | types.Selector): Promise[]> { return this.mainFrame().$$(selector); } - async $x(expression: string): Promise { + async $x(expression: string): Promise[]> { return this.mainFrame().$x(expression); } diff --git a/src/chromium/Playwright.ts b/src/chromium/Playwright.ts index 195a9de7ab..6d38709837 100644 --- a/src/chromium/Playwright.ts +++ b/src/chromium/Playwright.ts @@ -30,7 +30,7 @@ export class Playwright { this._launcher = new Launcher(projectRoot, preferredRevision); } - launch(options: (LauncherLaunchOptions & LauncherChromeArgOptions & LauncherBrowserOptions) | undefined): Promise { + launch(options?: (LauncherLaunchOptions & LauncherChromeArgOptions & LauncherBrowserOptions) | undefined): Promise { return this._launcher.launch(options); } diff --git a/src/dom.ts b/src/dom.ts index aee6927cd6..f1e28aeaa2 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -23,10 +23,10 @@ export interface DOMWorldDelegate { screenshot(handle: ElementHandle, options?: any): Promise; ensurePointerActionPoint(handle: ElementHandle, relativePoint?: types.Point): Promise; setInputFiles(handle: ElementHandle, files: input.FilePayload[]): Promise; - adoptElementHandle(handle: ElementHandle, to: DOMWorld): Promise; + adoptElementHandle(handle: ElementHandle, to: DOMWorld): Promise>; } -export type ScopedSelector = types.Selector & { scope?: ElementHandle }; +type ScopedSelector = types.Selector & { scope?: ElementHandle }; type ResolvedSelector = { scope?: ElementHandle, selector: string, visible?: boolean, disposeScope?: boolean }; export class DOMWorld { @@ -59,7 +59,7 @@ export class DOMWorld { return this._injectedPromise; } - async adoptElementHandle(handle: ElementHandle): Promise { + async adoptElementHandle(handle: ElementHandle): Promise> { assert(handle.executionContext() !== this.context, 'Should not adopt to the same context'); return this.delegate.adoptElementHandle(handle, this); } @@ -74,11 +74,11 @@ export class DOMWorld { return { scope: selector.scope, selector: normalizeSelector(selector.selector), visible: selector.visible }; } - async $(selector: string | ScopedSelector): Promise { + async $(selector: string | ScopedSelector): Promise | null> { const resolved = await this.resolveSelector(selector); const handle = await this.context.evaluateHandle( - (injected: Injected, selector: string, scope: SelectorRoot | undefined, visible: boolean | undefined) => { - const element = injected.querySelector(selector, scope || document); + (injected: Injected, selector: string, scope?: Node, visible?: boolean) => { + const element = injected.querySelector(selector, scope as SelectorRoot || document); if (visible === undefined || !element) return element; return injected.isVisible(element) === visible ? element : undefined; @@ -92,11 +92,11 @@ export class DOMWorld { return handle.asElement(); } - async $$(selector: string | ScopedSelector): Promise { + async $$(selector: string | ScopedSelector): Promise[]> { const resolved = await this.resolveSelector(selector); const arrayHandle = await this.context.evaluateHandle( - (injected: Injected, selector: string, scope: SelectorRoot | undefined, visible: boolean | undefined) => { - const elements = injected.querySelectorAll(selector, scope || document); + (injected: Injected, selector: string, scope?: Node, visible?: boolean) => { + const elements = injected.querySelectorAll(selector, scope as SelectorRoot || document); if (visible !== undefined) return elements.filter(element => injected.isVisible(element) === visible); return elements; @@ -130,8 +130,8 @@ export class DOMWorld { $$eval: types.$$Eval = async (selector, pageFunction, ...args) => { const resolved = await this.resolveSelector(selector); const arrayHandle = await this.context.evaluateHandle( - (injected: Injected, selector: string, scope: SelectorRoot | undefined, visible: boolean | undefined) => { - const elements = injected.querySelectorAll(selector, scope || document); + (injected: Injected, selector: string, scope?: Node, visible?: boolean) => { + const elements = injected.querySelectorAll(selector, scope as SelectorRoot || document); if (visible !== undefined) return elements.filter(element => injected.isVisible(element) === visible); return elements; @@ -144,7 +144,7 @@ export class DOMWorld { } } -export class ElementHandle extends js.JSHandle { +export class ElementHandle extends js.JSHandle { private readonly _world: DOMWorld; constructor(context: js.ExecutionContext, remoteObject: any) { @@ -153,7 +153,7 @@ export class ElementHandle extends js.JSHandle { this._world = context._domWorld; } - asElement(): ElementHandle | null { + asElement(): ElementHandle | null { return this; } @@ -162,13 +162,15 @@ export class ElementHandle extends js.JSHandle { } async _scrollIntoViewIfNeeded() { - const error = await this.evaluate(async (element, pageJavascriptEnabled) => { - if (!element.isConnected) + const error = await this.evaluate(async (node: Node, pageJavascriptEnabled: boolean) => { + if (!node.isConnected) return 'Node is detached from document'; - if (element.nodeType !== Node.ELEMENT_NODE) + if (node.nodeType !== Node.ELEMENT_NODE) return 'Node is not of type HTMLElement'; + const element = node as Element; // force-scroll if page's javascript is disabled. if (!pageJavascriptEnabled) { + //@ts-ignore because only Chromium still supports 'instant' element.scrollIntoView({block: 'center', inline: 'center', behavior: 'instant'}); return false; } @@ -182,8 +184,10 @@ export class ElementHandle extends js.JSHandle { // there are rafs. requestAnimationFrame(() => {}); }); - if (visibleRatio !== 1.0) + if (visibleRatio !== 1.0) { + //@ts-ignore because only Chromium still supports 'instant' element.scrollIntoView({block: 'center', inline: 'center', behavior: 'instant'}); + } return false; }, this._world.delegate.isJavascriptEnabled()); if (error) @@ -242,13 +246,13 @@ export class ElementHandle extends js.JSHandle { } async setInputFiles(...files: (string|input.FilePayload)[]) { - const multiple = await this.evaluate((element: HTMLInputElement) => !!element.multiple); + const multiple = await this.evaluate((element: Node) => !!(element as HTMLInputElement).multiple); assert(multiple || files.length <= 1, 'Non-multiple file input can only accept single file!'); await this._world.delegate.setInputFiles(this, await input.loadFiles(files)); } async focus() { - await this.evaluate(element => element.focus()); + await this.evaluate((element: Node) => (element as HTMLElement).focus()); } async type(text: string, options: { delay: (number | undefined); } | undefined) { @@ -280,7 +284,7 @@ export class ElementHandle extends js.JSHandle { return this._world.$(this._scopedSelector(selector)); } - $$(selector: string | types.Selector): Promise { + $$(selector: string | types.Selector): Promise[]> { return this._world.$$(this._scopedSelector(selector)); } @@ -292,12 +296,15 @@ export class ElementHandle extends js.JSHandle { return this._world.$$eval(this._scopedSelector(selector), pageFunction, ...args as any); } - $x(expression: string): Promise { + $x(expression: string): Promise[]> { return this._world.$$({ scope: this, selector: 'xpath=' + expression }); } isIntersectingViewport(): Promise { - return this.evaluate(async element => { + return this.evaluate(async (node: Node) => { + if (node.nodeType !== Node.ELEMENT_NODE) + throw new Error('Node is not of type HTMLElement'); + const element = node as Element; const visibleRatio = await new Promise(resolve => { const observer = new IntersectionObserver(entries => { resolve(entries[0].intersectionRatio); @@ -348,7 +355,7 @@ export function waitForSelectorTask(selector: string | ScopedSelector, timeout: return async (domWorld: DOMWorld) => { // TODO: we should not be able to adopt selector scope from a different document - handle this case. const resolved = await domWorld.resolveSelector(selector); - return domWorld.context.evaluateHandle((injected: Injected, selector: string, scope: SelectorRoot | undefined, visible: boolean | undefined, timeout: number) => { + return domWorld.context.evaluateHandle((injected: Injected, selector: string, scope: Node | undefined, visible: boolean | undefined, timeout: number) => { if (visible !== undefined) return injected.pollRaf(predicate, timeout); return injected.pollMutation(predicate, timeout); diff --git a/src/firefox/JSHandle.ts b/src/firefox/JSHandle.ts index 2f54d6564a..a38cbfb28c 100644 --- a/src/firefox/JSHandle.ts +++ b/src/firefox/JSHandle.ts @@ -22,6 +22,7 @@ import * as types from '../types'; import * as frames from '../frames'; import { JugglerSession } from './Connection'; import { FrameManager } from './FrameManager'; +import { Protocol } from './protocol'; export class DOMWorldDelegate implements dom.DOMWorldDelegate { readonly keyboard: input.Keyboard; @@ -138,13 +139,13 @@ export class DOMWorldDelegate implements dom.DOMWorldDelegate { await handle.evaluate(input.setFileInputFunction, files); } - async adoptElementHandle(handle: dom.ElementHandle, to: dom.DOMWorld): Promise { + async adoptElementHandle(handle: dom.ElementHandle, to: dom.DOMWorld): Promise> { assert(false, 'Multiple isolated worlds are not implemented'); return handle; } } -function toRemoteObject(handle: dom.ElementHandle): any { +function toRemoteObject(handle: dom.ElementHandle): Protocol.RemoteObject { return handle._remoteObject; } diff --git a/src/frames.ts b/src/frames.ts index fefe473a21..36ed432d70 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -122,12 +122,12 @@ export class Frame { return context.evaluate(pageFunction, ...args as any); } - async $(selector: string | types.Selector): Promise { + async $(selector: string | types.Selector): Promise | null> { const domWorld = await this._mainDOMWorld(); return domWorld.$(types.clearSelector(selector)); } - async $x(expression: string): Promise { + async $x(expression: string): Promise[]> { const domWorld = await this._mainDOMWorld(); return domWorld.$$('xpath=' + expression); } @@ -142,7 +142,7 @@ export class Frame { return domWorld.$$eval(selector, pageFunction, ...args as any); } - async $$(selector: string | types.Selector): Promise { + async $$(selector: string | types.Selector): Promise[]> { const domWorld = await this._mainDOMWorld(); return domWorld.$$(types.clearSelector(selector)); } diff --git a/src/injected/injected.ts b/src/injected/injected.ts index 7f46facc3b..545558be5f 100644 --- a/src/injected/injected.ts +++ b/src/injected/injected.ts @@ -17,9 +17,11 @@ class Injected { this.engines.set(engine.name, engine); } - querySelector(selector: string, root: SelectorRoot): Element | undefined { + querySelector(selector: string, root: Node): Element | undefined { const parsed = this._parseSelector(selector); - let element = root; + if (!root["querySelector"]) + throw new Error('Node is not queryable.'); + let element = root as SelectorRoot; for (const { engine, selector } of parsed) { const next = engine.query((element as Element).shadowRoot || element, selector); if (!next) diff --git a/src/input.ts b/src/input.ts index 5e4e2885e3..829886ded1 100644 --- a/src/input.ts +++ b/src/input.ts @@ -287,9 +287,10 @@ export class Mouse { } } -export const selectFunction = (element: HTMLSelectElement, ...optionsToSelect: (Node | SelectOption)[]) => { - if (element.nodeName.toLowerCase() !== 'select') +export const selectFunction = (node: Node, ...optionsToSelect: (Node | SelectOption)[]) => { + if (node.nodeName.toLowerCase() !== 'select') throw new Error('Element is not a