From 8e6375f532ecc8b1cc7add6bdcabc28612f924d9 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 3 Jun 2020 13:22:05 -0700 Subject: [PATCH] chore: reduce the number of evaluate methods, improve types (#2454) Types can now handle non-trivial tuples with handles inside. --- src/chromium/crPage.ts | 2 +- src/dom.ts | 66 +++++++++++++++++------------ src/firefox/ffPage.ts | 2 +- src/javascript.ts | 20 +-------- src/page.ts | 6 ++- src/server/electron.ts | 2 +- src/types.ts | 3 ++ utils/generate_types/overrides.d.ts | 4 ++ utils/generate_types/test/test.ts | 16 +++++++ 9 files changed, 70 insertions(+), 51 deletions(-) diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 729680474b..10d26583ef 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -264,7 +264,7 @@ export class CRPage implements PageDelegate { } async setInputFiles(handle: dom.ElementHandle, files: types.FilePayload[]): Promise { - await handle._evaluateInUtility(({ injected, node }, files) => + await handle._evaluateInUtility(([injected, node, files]) => injected.setInputFiles(node, files), dom.toFileTransferPayload(files)); } diff --git a/src/dom.ts b/src/dom.ts index c68a6b03d7..492140a8ff 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -60,9 +60,19 @@ export class FrameExecutionContext extends js.ExecutionContext { return null; } - async doEvaluateInternal(returnByValue: boolean, waitForNavigations: boolean, pageFunction: string | Function, ...args: any[]): Promise { - return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, !waitForNavigations, async () => { - return this._delegate.evaluate(this, returnByValue, pageFunction, ...args); + async evaluateInternal(pageFunction: types.Func0): Promise; + async evaluateInternal(pageFunction: types.Func1, arg: Arg): Promise; + async evaluateInternal(pageFunction: never, ...args: never[]): Promise { + return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { + return this._delegate.evaluate(this, true /* returnByValue */, pageFunction, ...args); + }); + } + + async evaluateHandleInternal(pageFunction: types.Func0): Promise>; + async evaluateHandleInternal(pageFunction: types.Func1, arg: Arg): Promise>; + async evaluateHandleInternal(pageFunction: never, ...args: never[]): Promise { + return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { + return this._delegate.evaluate(this, false /* returnByValue */, pageFunction, ...args); }); } @@ -104,19 +114,19 @@ export class ElementHandle extends js.JSHandle { return this; } - async _evaluateInMain(pageFunction: types.FuncOn<{ injected: InjectedScript, node: T }, Arg, R>, arg: Arg): Promise { + async _evaluateInMain(pageFunction: types.Func1<[js.JSHandle, ElementHandle, Arg], R>, arg: Arg): Promise { const main = await this._context.frame._mainContext(); - return main.doEvaluateInternal(true /* returnByValue */, true /* waitForNavigations */, pageFunction, { injected: await main.injectedScript(), node: this }, arg); + return main.evaluateInternal(pageFunction, [await main.injectedScript(), this, arg]); } - async _evaluateInUtility(pageFunction: types.FuncOn<{ injected: InjectedScript, node: T }, Arg, R>, arg: Arg): Promise { + async _evaluateInUtility(pageFunction: types.Func1<[js.JSHandle, ElementHandle, Arg], R>, arg: Arg): Promise { const utility = await this._context.frame._utilityContext(); - return utility.doEvaluateInternal(true /* returnByValue */, true /* waitForNavigations */, pageFunction, { injected: await utility.injectedScript(), node: this }, arg); + return utility.evaluateInternal(pageFunction, [await utility.injectedScript(), this, arg]); } - async _evaluateHandleInUtility(pageFunction: types.FuncOn<{ injected: InjectedScript, node: T }, Arg, R>, arg: Arg): Promise> { + async _evaluateHandleInUtility(pageFunction: types.Func1<[js.JSHandle, ElementHandle, Arg], R>, arg: Arg): Promise> { const utility = await this._context.frame._utilityContext(); - return utility.doEvaluateInternal(false /* returnByValue */, true /* waitForNavigations */, pageFunction, { injected: await utility.injectedScript(), node: this }, arg); + return utility.evaluateHandleInternal(pageFunction, [await utility.injectedScript(), this, arg]); } async ownerFrame(): Promise { @@ -135,14 +145,14 @@ export class ElementHandle extends js.JSHandle { } async contentFrame(): Promise { - const isFrameElement = await this._evaluateInUtility(({node}) => node && (node.nodeName === 'IFRAME' || node.nodeName === 'FRAME'), {}); + const isFrameElement = await this._evaluateInUtility(([injected, node]) => node && (node.nodeName === 'IFRAME' || node.nodeName === 'FRAME'), {}); if (!isFrameElement) return null; return this._page._delegate.getContentFrame(this); } async getAttribute(name: string): Promise { - return this._evaluateInUtility(({node}, name: string) => { + return this._evaluateInUtility(([injeced, node, name]) => { if (node.nodeType !== Node.ELEMENT_NODE) throw new Error('Not an element'); const element = node as unknown as Element; @@ -151,11 +161,11 @@ export class ElementHandle extends js.JSHandle { } async textContent(): Promise { - return this._evaluateInUtility(({node}) => node.textContent, {}); + return this._evaluateInUtility(([injected, node]) => node.textContent, {}); } async innerText(): Promise { - return this._evaluateInUtility(({node}) => { + return this._evaluateInUtility(([injected, node]) => { if (node.nodeType !== Node.ELEMENT_NODE) throw new Error('Not an element'); if (node.namespaceURI !== 'http://www.w3.org/1999/xhtml') @@ -166,7 +176,7 @@ export class ElementHandle extends js.JSHandle { } async innerHTML(): Promise { - return this._evaluateInUtility(({node}) => { + return this._evaluateInUtility(([injected, node]) => { if (node.nodeType !== Node.ELEMENT_NODE) throw new Error('Not an element'); const element = node as unknown as Element; @@ -175,7 +185,7 @@ export class ElementHandle extends js.JSHandle { } async dispatchEvent(type: string, eventInit: Object = {}) { - await this._evaluateInMain(({ injected, node }, { type, eventInit }) => + await this._evaluateInMain(([injected, node, { type, eventInit }]) => injected.dispatchEvent(node, type, eventInit), { type, eventInit }); } @@ -229,7 +239,7 @@ export class ElementHandle extends js.JSHandle { private async _offsetPoint(offset: types.Point): Promise { const [box, border] = await Promise.all([ this.boundingBox(), - this._evaluateInUtility(({ injected, node }) => injected.getElementBorderWidth(node), {}).catch(logError(this._context._logger)), + this._evaluateInUtility(([injected, node]) => injected.getElementBorderWidth(node), {}).catch(logError(this._context._logger)), ]); if (!box || !border) return 'invisible'; @@ -353,7 +363,7 @@ export class ElementHandle extends js.JSHandle { assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"'); } return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { - const injectedResult = await this._evaluateInUtility(({ injected, node }, selectOptions) => injected.selectOptions(node, selectOptions), selectOptions); + const injectedResult = await this._evaluateInUtility(([injected, node, selectOptions]) => injected.selectOptions(node, selectOptions), selectOptions); return handleInjectedResult(injectedResult); }); } @@ -366,9 +376,9 @@ export class ElementHandle extends js.JSHandle { progress.log(inputLog, `elementHandle.fill("${value}")`); assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"'); await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { - const poll = await this._evaluateHandleInUtility(({ injected, node }, { value }) => { + const poll = await this._evaluateHandleInUtility(([injected, node, value]) => { return injected.waitForEnabledAndFill(node, value); - }, { value }); + }, value); new InjectedScriptPollHandler(progress, poll); const injectedResult = await poll.evaluate(poll => poll.result); const needsInput = handleInjectedResult(injectedResult); @@ -383,7 +393,7 @@ export class ElementHandle extends js.JSHandle { async selectText(): Promise { this._page._log(inputLog, `elementHandle.selectText()`); - const injectedResult = await this._evaluateInUtility(({ injected, node }) => injected.selectText(node), {}); + const injectedResult = await this._evaluateInUtility(([injected, node]) => injected.selectText(node), {}); handleInjectedResult(injectedResult); } @@ -393,7 +403,7 @@ export class ElementHandle extends js.JSHandle { async _setInputFiles(progress: Progress, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions) { progress.log(inputLog, progress.apiName); - const injectedResult = await this._evaluateInUtility(({ node }): types.InjectedScriptResult => { + const injectedResult = await this._evaluateInUtility(([injected, node]): types.InjectedScriptResult => { if (node.nodeType !== Node.ELEMENT_NODE || (node as Node as Element).tagName !== 'INPUT') return { status: 'error', error: 'Node is not an HTMLInputElement' }; if (!node.isConnected) @@ -428,7 +438,7 @@ export class ElementHandle extends js.JSHandle { async focus() { this._page._log(inputLog, `elementHandle.focus()`); - const injectedResult = await this._evaluateInUtility(({ injected, node }) => injected.focusNode(node), {}); + const injectedResult = await this._evaluateInUtility(([injected, node]) => injected.focusNode(node), {}); handleInjectedResult(injectedResult); } @@ -471,10 +481,10 @@ export class ElementHandle extends js.JSHandle { } async _setChecked(progress: Progress, state: boolean, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions) { - if (await this._evaluateInUtility(({ injected, node }) => injected.isCheckboxChecked(node), {}) === state) + if (await this._evaluateInUtility(([injected, node]) => injected.isCheckboxChecked(node), {}) === state) return; await this._click(progress, options); - if (await this._evaluateInUtility(({ injected, node }) => injected.isCheckboxChecked(node), {}) !== state) + if (await this._evaluateInUtility(([injected, node]) => injected.isCheckboxChecked(node), {}) !== state) throw new Error('Unable to click checkbox'); } @@ -517,9 +527,9 @@ export class ElementHandle extends js.JSHandle { async _waitForDisplayedAtStablePositionAndEnabled(progress: Progress): Promise { progress.log(inputLog, 'waiting for element to be displayed, enabled and not moving...'); const rafCount = this._page._delegate.rafCountForStablePosition(); - const poll = await this._evaluateHandleInUtility(({ injected, node }, { rafCount }) => { + const poll = await this._evaluateHandleInUtility(([injected, node, rafCount]) => { return injected.waitForDisplayedAtStablePositionAndEnabled(node, rafCount); - }, { rafCount }); + }, rafCount); new InjectedScriptPollHandler(progress, poll); const injectedResult = await poll.evaluate(poll => poll.result); handleInjectedResult(injectedResult); @@ -536,9 +546,9 @@ export class ElementHandle extends js.JSHandle { // Translate from viewport coordinates to frame coordinates. point = { x: point.x - box.x, y: point.y - box.y }; } - const injectedResult = await this._evaluateInUtility(({ injected, node }, { point }) => { + const injectedResult = await this._evaluateInUtility(([injected, node, point]) => { return injected.checkHitTargetAt(node, point); - }, { point }); + }, point); return handleInjectedResult(injectedResult); } } diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 406333abf8..157e7fa994 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -443,7 +443,7 @@ export class FFPage implements PageDelegate { } async setInputFiles(handle: dom.ElementHandle, files: types.FilePayload[]): Promise { - await handle._evaluateInUtility(({ injected, node }, files) => + await handle._evaluateInUtility(([injected, node, files]) => injected.setInputFiles(node, files), dom.toFileTransferPayload(files)); } diff --git a/src/javascript.ts b/src/javascript.ts index a00131260f..d07ed913b3 100644 --- a/src/javascript.ts +++ b/src/javascript.ts @@ -45,26 +45,10 @@ export class ExecutionContext { this._logger = logger; } - doEvaluateInternal(returnByValue: boolean, waitForNavigations: boolean, pageFunction: string | Function, ...args: any[]): Promise { - return this._delegate.evaluate(this, returnByValue, pageFunction, ...args); - } - adoptIfNeeded(handle: JSHandle): Promise | null { return null; } - async evaluateInternal(pageFunction: types.Func0): Promise; - async evaluateInternal(pageFunction: types.Func1, arg: Arg): Promise; - async evaluateInternal(pageFunction: never, ...args: never[]): Promise { - return this.doEvaluateInternal(true /* returnByValue */, true /* waitForNavigations */, pageFunction, ...args); - } - - async evaluateHandleInternal(pageFunction: types.Func0): Promise>; - async evaluateHandleInternal(pageFunction: types.Func1, arg: Arg): Promise>; - async evaluateHandleInternal(pageFunction: never, ...args: never[]): Promise { - return this.doEvaluateInternal(false /* returnByValue */, true /* waitForNavigations */, pageFunction, ...args); - } - utilityScript(): Promise { if (!this._utilityScriptPromise) { const source = `new (${utilityScriptSource.source})()`; @@ -95,13 +79,13 @@ export class JSHandle { async evaluate(pageFunction: types.FuncOn, arg: Arg): Promise; async evaluate(pageFunction: types.FuncOn, arg?: any): Promise; async evaluate(pageFunction: types.FuncOn, arg: Arg): Promise { - return this._context.doEvaluateInternal(true /* returnByValue */, true /* waitForNavigations */, pageFunction, this, arg); + return this._context._delegate.evaluate(this._context, true /* returnByValue */, pageFunction, this, arg); } async evaluateHandle(pageFunction: types.FuncOn, arg: Arg): Promise>; async evaluateHandle(pageFunction: types.FuncOn, arg?: any): Promise>; async evaluateHandle(pageFunction: types.FuncOn, arg: Arg): Promise> { - return this._context.doEvaluateInternal(false /* returnByValue */, true /* waitForNavigations */, pageFunction, this, arg); + return this._context._delegate.evaluate(this._context, false /* returnByValue */, pageFunction, this, arg); } async getProperty(propertyName: string): Promise { diff --git a/src/page.ts b/src/page.ts index c09664307a..fed842a2e6 100644 --- a/src/page.ts +++ b/src/page.ts @@ -586,14 +586,16 @@ export class Worker extends EventEmitter { async evaluate(pageFunction: types.Func1, arg?: any): Promise; async evaluate(pageFunction: types.Func1, arg: Arg): Promise { assertMaxArguments(arguments.length, 2); - return (await this._executionContextPromise).evaluateInternal(pageFunction, arg); + const context = await this._executionContextPromise; + return context._delegate.evaluate(context, true /* returnByValue */, pageFunction, arg); } async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise>; async evaluateHandle(pageFunction: types.Func1, arg?: any): Promise>; async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise> { assertMaxArguments(arguments.length, 2); - return (await this._executionContextPromise).evaluateHandleInternal(pageFunction, arg); + const context = await this._executionContextPromise; + return context._delegate.evaluate(context, false /* returnByValue */, pageFunction, arg); } } diff --git a/src/server/electron.ts b/src/server/electron.ts index a551fdeb0e..2e09b87427 100644 --- a/src/server/electron.ts +++ b/src/server/electron.ts @@ -133,7 +133,7 @@ export class ElectronApplication extends ExtendedEventEmitter { this._nodeExecutionContext = new js.ExecutionContext(new CRExecutionContext(this._nodeSession, event.context), this._logger); }); await this._nodeSession.send('Runtime.enable', {}).catch(e => {}); - this._nodeElectronHandle = await this._nodeExecutionContext!.evaluateHandleInternal(() => { + this._nodeElectronHandle = await this._nodeExecutionContext!._delegate.evaluate(this._nodeExecutionContext!, false /* returnByValue */, () => { // Resolving the race between the debugger and the boot-time script. if ((global as any)._playwrightRun) return (global as any)._playwrightRun(); diff --git a/src/types.ts b/src/types.ts index dd2a0f17a8..820ce4c66d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -22,6 +22,9 @@ type Unboxed = Arg extends dom.ElementHandle ? T : Arg extends js.JSHandle ? T : Arg extends NoHandles ? Arg : + Arg extends [infer A0] ? [Unboxed] : + Arg extends [infer A0, infer A1] ? [Unboxed, Unboxed] : + Arg extends [infer A0, infer A1, infer A2] ? [Unboxed, Unboxed, Unboxed] : Arg extends Array ? Array> : Arg extends object ? { [Key in keyof Arg]: Unboxed } : Arg; diff --git a/utils/generate_types/overrides.d.ts b/utils/generate_types/overrides.d.ts index 47c5b3ed71..0bb98da2a2 100644 --- a/utils/generate_types/overrides.d.ts +++ b/utils/generate_types/overrides.d.ts @@ -28,6 +28,10 @@ type Unboxed = Arg extends ElementHandle ? T : Arg extends JSHandle ? T : Arg extends NoHandles ? Arg : + Arg extends [infer A0] ? [Unboxed] : + Arg extends [infer A0, infer A1] ? [Unboxed, Unboxed] : + Arg extends [infer A0, infer A1, infer A2] ? [Unboxed, Unboxed, Unboxed] : + Arg extends [infer A0, infer A1, infer A2, infer A3] ? [Unboxed, Unboxed, Unboxed, Unboxed] : Arg extends Array ? Array> : Arg extends object ? { [Key in keyof Arg]: Unboxed } : Arg; diff --git a/utils/generate_types/test/test.ts b/utils/generate_types/test/test.ts index b871b9973d..7e47be0c64 100644 --- a/utils/generate_types/test/test.ts +++ b/utils/generate_types/test/test.ts @@ -409,6 +409,11 @@ playwright.chromium.launch().then(async browser => { const browser = await playwright.webkit.launch(); const page = await browser.newPage(); const windowHandle = await page.evaluateHandle(() => window); + + function wrap(t: T): [T, string, boolean, number] { + return [t, '1', true, 1]; + } + { const value = await page.evaluate(() => 1); const assertion: AssertType = true; @@ -437,6 +442,17 @@ playwright.chromium.launch().then(async browser => { const value = await page.evaluate(([a, b, c]) => ({a, b, c}), [3, '123', true] as const); const assertion: AssertType<{a: 3, b: '123', c: true}, typeof value> = true; } + { + const handle = await page.evaluateHandle(() => 3); + const value = await page.evaluate(([a, b, c, d]) => ({a, b, c, d}), wrap(handle)); + const assertion: AssertType<{a: number, b: string, c: boolean, d: number}, typeof value> = true; + } + { + const handle = await page.evaluateHandle(() => 3); + const h = await page.evaluateHandle(([a, b, c, d]) => ({a, b, c, d}), wrap(handle)); + const value = await h.evaluate(h => h); + const assertion: AssertType<{a: number, b: string, c: boolean, d: number}, typeof value> = true; + } { const handle = await page.evaluateHandle(() => ([{a: '123'}]));