From 6474edb9ba5ab6637361198b574dc64529eef26b Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Thu, 2 Jul 2020 10:09:34 +0100 Subject: [PATCH] feat(types): add types for `$eval` (#6135) This pulls in the types (based on the DefinitelyTyped repo) for `page.$eval` (and the `$eval` method on other classes). The `$eval` method is quite hard to type due to the way we wrap and unwrap ElementHandles that are passed to / returned from the `pageFunction` that users provide. Longer term we can improve the types by providing type overloads as DefinitelyTyped does but I've deferred that for now (see the `TODO` in the code for more details). --- new-docs/puppeteer.elementhandle._eval.md | 6 +- new-docs/puppeteer.elementhandle.aselement.md | 4 +- new-docs/puppeteer.elementhandle.md | 4 +- new-docs/puppeteer.frame._eval.md | 6 +- new-docs/puppeteer.jshandle.evaluatehandle.md | 2 +- new-docs/puppeteer.md | 2 + new-docs/puppeteer.page._eval.md | 54 +++++++++++-- new-docs/puppeteer.page.md | 2 +- new-docs/puppeteer.unwrapelementhandle.md | 13 ++++ new-docs/puppeteer.wrapelementhandle.md | 13 ++++ src/common/DOMWorld.ts | 10 ++- src/common/EvalTypes.ts | 14 +++- src/common/FrameManager.ts | 10 ++- src/common/JSHandle.ts | 56 ++++++++----- src/common/Page.ts | 78 ++++++++++++++++++- test/click.spec.ts | 2 +- test/input.spec.ts | 32 +++++--- test/keyboard.spec.ts | 18 +++-- test/page.spec.ts | 6 +- test/queryselector.spec.ts | 14 +++- test/requestinterception.spec.ts | 4 +- utils/doclint/check_public_api/index.js | 4 + 22 files changed, 282 insertions(+), 72 deletions(-) create mode 100644 new-docs/puppeteer.unwrapelementhandle.md create mode 100644 new-docs/puppeteer.wrapelementhandle.md diff --git a/new-docs/puppeteer.elementhandle._eval.md b/new-docs/puppeteer.elementhandle._eval.md index f0c8b8717eb96..4de0423e19c74 100644 --- a/new-docs/puppeteer.elementhandle._eval.md +++ b/new-docs/puppeteer.elementhandle._eval.md @@ -11,7 +11,7 @@ If `pageFunction` returns a Promise, then `frame.$eval` would wait for the promi Signature: ```typescript -$eval(selector: string, pageFunction: EvaluateFn | string, ...args: SerializableOrJSHandle[]): Promise; +$eval(selector: string, pageFunction: (element: Element, ...args: unknown[]) => ReturnType | Promise, ...args: SerializableOrJSHandle[]): Promise>; ``` ## Parameters @@ -19,12 +19,12 @@ $eval(selector: string, pageFunction: EvaluateFn | strin | Parameter | Type | Description | | --- | --- | --- | | selector | string | | -| pageFunction | [EvaluateFn](./puppeteer.evaluatefn.md) \| string | | +| pageFunction | (element: Element, ...args: unknown\[\]) => ReturnType \| Promise<ReturnType> | | | args | [SerializableOrJSHandle](./puppeteer.serializableorjshandle.md)\[\] | | Returns: -Promise<ReturnType> +Promise<[WrapElementHandle](./puppeteer.wrapelementhandle.md)<ReturnType>> ## Example diff --git a/new-docs/puppeteer.elementhandle.aselement.md b/new-docs/puppeteer.elementhandle.aselement.md index 0abe674ad140e..670b0f824b8ab 100644 --- a/new-docs/puppeteer.elementhandle.aselement.md +++ b/new-docs/puppeteer.elementhandle.aselement.md @@ -7,9 +7,9 @@ Signature: ```typescript -asElement(): ElementHandle | null; +asElement(): ElementHandle | null; ``` Returns: -[ElementHandle](./puppeteer.elementhandle.md) \| null +[ElementHandle](./puppeteer.elementhandle.md)<ElementType> \| null diff --git a/new-docs/puppeteer.elementhandle.md b/new-docs/puppeteer.elementhandle.md index 9d479637f7e2c..09e5e97085b5f 100644 --- a/new-docs/puppeteer.elementhandle.md +++ b/new-docs/puppeteer.elementhandle.md @@ -9,7 +9,7 @@ ElementHandle represents an in-page DOM element. Signature: ```typescript -export declare class ElementHandle extends JSHandle +export declare class ElementHandle extends JSHandle ``` Extends: [JSHandle](./puppeteer.jshandle.md) @@ -34,6 +34,8 @@ ElementHandle prevents the DOM element from being garbage-collected unless the h ElementHandle instances can be used as arguments in [Page.$eval()](./puppeteer.page._eval.md) and [Page.evaluate()](./puppeteer.page.evaluate.md) methods. +If you're using TypeScript, ElementHandle takes a generic argument that denotes the type of element the handle is holding within. For example, if you have a handle to a `` element, you can type it as + * `ElementHandle` and you get some nicer type checks. + * * @public */ -export class ElementHandle extends JSHandle { +export class ElementHandle< + ElementType extends Element = Element +> extends JSHandle { private _page: Page; private _frameManager: FrameManager; @@ -339,7 +347,7 @@ export class ElementHandle extends JSHandle { this._frameManager = frameManager; } - asElement(): ElementHandle | null { + asElement(): ElementHandle | null { return this; } @@ -358,7 +366,7 @@ export class ElementHandle extends JSHandle { private async _scrollIntoViewIfNeeded(): Promise { const error = await this.evaluate< ( - element: HTMLElement, + element: Element, pageJavascriptEnabled: boolean ) => Promise >(async (element, pageJavascriptEnabled) => { @@ -512,11 +520,9 @@ export class ElementHandle extends JSHandle { '"' ); - /* TODO(jacktfranklin@): once ExecutionContext is TypeScript, and - * its evaluate function is properly typed with generics we can - * return here and remove the typecasting - */ - return this.evaluate((element: HTMLSelectElement, values: string[]) => { + return this.evaluate< + (element: HTMLSelectElement, values: string[]) => string[] + >((element, values) => { if (element.nodeName.toLowerCase() !== 'select') throw new Error('Element is not a `); const [fileChooser] = await Promise.all([ page.waitForFileChooser(), - page.$eval('input', (input) => input.click()), + page.$eval('input', (input: HTMLInputElement) => input.click()), ]); await fileChooser.accept([]); let error = null; @@ -268,13 +276,13 @@ describe('input tests', function () { await page.setContent(``); const [fileChooser1] = await Promise.all([ page.waitForFileChooser(), - page.$eval('input', (input) => input.click()), + page.$eval('input', (input: HTMLInputElement) => input.click()), ]); await fileChooser1.cancel(); // If this resolves, than we successfully canceled file chooser. await Promise.all([ page.waitForFileChooser(), - page.$eval('input', (input) => input.click()), + page.$eval('input', (input: HTMLInputElement) => input.click()), ]); }); it('should fail when canceling file chooser twice', async () => { @@ -283,7 +291,7 @@ describe('input tests', function () { await page.setContent(``); const [fileChooser] = await Promise.all([ page.waitForFileChooser(), - page.$eval('input', (input) => input.click()), + page.$eval('input', (input: HTMLInputElement) => input.click()), ]); await fileChooser.cancel(); let error = null; diff --git a/test/keyboard.spec.ts b/test/keyboard.spec.ts index abf31e64826ed..c90876adeb08b 100644 --- a/test/keyboard.spec.ts +++ b/test/keyboard.spec.ts @@ -351,9 +351,12 @@ describe('Keyboard', function () { await page.goto(server.PREFIX + '/input/textarea.html'); await page.type('textarea', '👹 Tokyo street Japan 🇯🇵'); - expect(await page.$eval('textarea', (textarea) => textarea.value)).toBe( - '👹 Tokyo street Japan 🇯🇵' - ); + expect( + await page.$eval( + 'textarea', + (textarea: HTMLInputElement) => textarea.value + ) + ).toBe('👹 Tokyo street Japan 🇯🇵'); }); itFailsFirefox('should type emoji into an iframe', async () => { const { page, server } = getTestState(); @@ -367,9 +370,12 @@ describe('Keyboard', function () { const frame = page.frames()[1]; const textarea = await frame.$('textarea'); await textarea.type('👹 Tokyo street Japan 🇯🇵'); - expect(await frame.$eval('textarea', (textarea) => textarea.value)).toBe( - '👹 Tokyo street Japan 🇯🇵' - ); + expect( + await frame.$eval( + 'textarea', + (textarea: HTMLInputElement) => textarea.value + ) + ).toBe('👹 Tokyo street Japan 🇯🇵'); }); itFailsFirefox('should press the meta key', async () => { const { page, isFirefox } = getTestState(); diff --git a/test/page.spec.ts b/test/page.spec.ts index c258ea5d7bbcf..893d935650751 100644 --- a/test/page.spec.ts +++ b/test/page.spec.ts @@ -211,7 +211,7 @@ describe('Page', function () { ); const [popup] = await Promise.all([ new Promise((x) => page.once('popup', x)), - page.$eval('a', (a) => a.click()), + page.$eval('a', (a: HTMLAnchorElement) => a.click()), ]); expect(await page.evaluate(() => !!window.opener)).toBe(false); expect(await popup.evaluate(() => !!window.opener)).toBe(false); @@ -1616,7 +1616,7 @@ describe('Page', function () { await page.select('select', 'blue', 'black', 'magenta'); await page.select('select'); expect( - await page.$eval('select', (select) => + await page.$eval('select', (select: HTMLSelectElement) => Array.from(select.options).every( (option: HTMLOptionElement) => !option.selected ) @@ -1630,7 +1630,7 @@ describe('Page', function () { await page.select('select', 'blue', 'black', 'magenta'); await page.select('select'); expect( - await page.$eval('select', (select) => + await page.$eval('select', (select: HTMLSelectElement) => Array.from(select.options).every( (option: HTMLOptionElement) => !option.selected ) diff --git a/test/queryselector.spec.ts b/test/queryselector.spec.ts index 5dbf06f8952fc..db2e36e8ada0e 100644 --- a/test/queryselector.spec.ts +++ b/test/queryselector.spec.ts @@ -49,7 +49,7 @@ describe('querySelector', function () { const divHandle = await page.$('div'); const text = await page.$eval( 'section', - (e, div) => e.textContent + div.textContent, + (e, div: HTMLElement) => e.textContent + div.textContent, divHandle ); expect(text).toBe('hello world'); @@ -174,7 +174,10 @@ describe('querySelector', function () { '
10
' ); const tweet = await page.$('.tweet'); - const content = await tweet.$eval('.like', (node) => node.innerText); + const content = await tweet.$eval( + '.like', + (node: HTMLElement) => node.innerText + ); expect(content).toBe('100'); }); @@ -185,7 +188,10 @@ describe('querySelector', function () { '
not-a-child-div
a-child-div
'; await page.setContent(htmlContent); const elementHandle = await page.$('#myId'); - const content = await elementHandle.$eval('.a', (node) => node.innerText); + const content = await elementHandle.$eval( + '.a', + (node: HTMLElement) => node.innerText + ); expect(content).toBe('a-child-div'); }); @@ -197,7 +203,7 @@ describe('querySelector', function () { await page.setContent(htmlContent); const elementHandle = await page.$('#myId'); const errorMessage = await elementHandle - .$eval('.a', (node) => node.innerText) + .$eval('.a', (node: HTMLElement) => node.innerText) .catch((error) => error.message); expect(errorMessage).toBe( `Error: failed to find element matching selector ".a"` diff --git a/test/requestinterception.spec.ts b/test/requestinterception.spec.ts index c448f97c10529..9d4d60fb32756 100644 --- a/test/requestinterception.spec.ts +++ b/test/requestinterception.spec.ts @@ -65,7 +65,7 @@ describe('request interception', function () { `); await Promise.all([ - page.$eval('form', (form) => form.submit()), + page.$eval('form', (form: HTMLFormElement) => form.submit()), page.waitForNavigation(), ]); }); @@ -454,7 +454,7 @@ describe('request interception', function () { page.on('request', async (r) => (request = r)); page.$eval( 'iframe', - (frame, url) => (frame.src = url), + (frame: HTMLIFrameElement, url: string) => (frame.src = url), server.EMPTY_PAGE ), // Wait for request interception. diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index 45021b1993bb1..1443f52a8bf53 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -25,6 +25,10 @@ const EXCLUDE_PROPERTIES = new Set([ 'Page.create', 'JSHandle.toString', 'TimeoutError.name', + /* This isn't an actual property, but a TypeScript generic. + * DocLint incorrectly parses it as a property. + */ + 'ElementHandle.ElementType', ]); /**