From 9a10aec13a42fd62e10dda44aac78f5486eb89d9 Mon Sep 17 00:00:00 2001 From: Randolf J Date: Wed, 22 Jun 2022 22:45:27 +0200 Subject: [PATCH] feat!: better TS types --- src/common/Connection.ts | 4 +- src/common/DOMWorld.ts | 9 ++- src/common/FrameManager.ts | 8 +++ src/common/JSHandle.ts | 15 +++-- src/common/Page.ts | 54 ++++++++--------- test/src/waittask.spec.ts | 117 ------------------------------------- 6 files changed, 56 insertions(+), 151 deletions(-) diff --git a/src/common/Connection.ts b/src/common/Connection.ts index 07b124a1edc6a..0b5c46f5bec0f 100644 --- a/src/common/Connection.ts +++ b/src/common/Connection.ts @@ -33,8 +33,8 @@ export {ConnectionTransport, ProtocolMapping}; * @public */ export interface ConnectionCallback { - resolve: Function; - reject: Function; + resolve(args: unknown): void; + reject(args: unknown): void; error: ProtocolError; method: string; } diff --git a/src/common/DOMWorld.ts b/src/common/DOMWorld.ts index 11c726473e613..bb1813f105a6e 100644 --- a/src/common/DOMWorld.ts +++ b/src/common/DOMWorld.ts @@ -578,7 +578,6 @@ export class DOMWorld { async function addStyleContent(content: string): Promise { const style = document.createElement('style'); - style.type = 'text/css'; style.appendChild(document.createTextNode(content)); const promise = new Promise((res, rej) => { style.onload = res; @@ -640,6 +639,14 @@ export class DOMWorld { await handle.dispose(); } + async waitForSelector( + selector: Selector, + options: WaitForSelectorOptions + ): Promise | null>; + async waitForSelector( + selector: string, + options: WaitForSelectorOptions + ): Promise; async waitForSelector( selector: string, options: WaitForSelectorOptions diff --git a/src/common/FrameManager.ts b/src/common/FrameManager.ts index cd75655bdf781..c2454383ef655 100644 --- a/src/common/FrameManager.ts +++ b/src/common/FrameManager.ts @@ -1345,6 +1345,14 @@ export class Frame { * @returns a promise which resolves when an element matching the selector * string is added to the DOM. */ + async waitForSelector( + selector: Selector, + options?: WaitForSelectorOptions + ): Promise | null>; + async waitForSelector( + selector: string, + options?: WaitForSelectorOptions + ): Promise; async waitForSelector( selector: string, options: WaitForSelectorOptions = {} diff --git a/src/common/JSHandle.ts b/src/common/JSHandle.ts index d588e6d9c79e9..11364fd783889 100644 --- a/src/common/JSHandle.ts +++ b/src/common/JSHandle.ts @@ -30,6 +30,7 @@ import { releaseObject, valueFromRemoteObject, } from './util.js'; +import {WaitForSelectorOptions} from './DOMWorld.js'; /** * @public @@ -406,13 +407,17 @@ export class ElementHandle< * (30 seconds). Pass `0` to disable timeout. The default value can be changed * by using the {@link Page.setDefaultTimeout} method. */ + async waitForSelector( + selector: Selector, + options?: Exclude + ): Promise | null>; async waitForSelector( selector: string, - options: { - visible?: boolean; - hidden?: boolean; - timeout?: number; - } = {} + options?: Exclude + ): Promise; + async waitForSelector( + selector: string, + options: Exclude = {} ): Promise { const frame = this._context.frame(); assert(frame); diff --git a/src/common/Page.ts b/src/common/Page.ts index 5c151a5c68154..eea64080580b4 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -23,6 +23,7 @@ import {CDPSession, CDPSessionEmittedEvents, Connection} from './Connection.js'; import {ConsoleMessage, ConsoleMessageType} from './ConsoleMessage.js'; import {Coverage} from './Coverage.js'; import {Dialog} from './Dialog.js'; +import {WaitForSelectorOptions} from './DOMWorld.js'; import {EmulationManager} from './EmulationManager.js'; import {EventEmitter, Handler} from './EventEmitter.js'; import {FileChooser} from './FileChooser.js'; @@ -459,9 +460,7 @@ export class Page extends EventEmitter { #viewport: Viewport | null; #screenshotTaskQueue: TaskQueue; #workers = new Map(); - // TODO: improve this typedef - it's a function that takes a file chooser or - // something? - #fileChooserInterceptors = new Set(); + #fileChooserInterceptors = new Set<(chooser: FileChooser) => void>(); #disconnectPromise?: Promise; #userDragInterceptionEnabled = false; @@ -637,7 +636,7 @@ export class Page extends EventEmitter { event ); for (const interceptor of interceptors) { - interceptor.call(null, fileChooser); + interceptor.call(undefined, fileChooser); } } @@ -733,7 +732,7 @@ export class Page extends EventEmitter { } const {timeout = this.#timeoutSettings.timeout()} = options; - let callback!: (value: FileChooser | PromiseLike) => void; + let callback!: (value: FileChooser) => void; const promise = new Promise(x => { return (callback = x); }); @@ -1428,7 +1427,7 @@ export class Page extends EventEmitter { * * NOTE: Functions installed via `page.exposeFunction` survive navigations. * @param name - Name of the function on the window object - * @param puppeteerFunction - Callback function which will be called in + * @param pptrFunction - Callback function which will be called in * Puppeteer's context. * @example * An example of adding an `md5` function into the page: @@ -1480,7 +1479,7 @@ export class Page extends EventEmitter { */ async exposeFunction( name: string, - puppeteerFunction: Function | {default: Function} + pptrFunction: Function | {default: Function} ): Promise { if (this.#pageBindings.has(name)) { throw new Error( @@ -1489,14 +1488,13 @@ export class Page extends EventEmitter { } let exposedFunction: Function; - if (typeof puppeteerFunction === 'function') { - exposedFunction = puppeteerFunction; - } else if (typeof puppeteerFunction.default === 'function') { - exposedFunction = puppeteerFunction.default; - } else { - throw new Error( - `Failed to add page binding with name ${name}: ${puppeteerFunction} is not a function or a module with a default export.` - ); + switch (typeof pptrFunction) { + case 'function': + exposedFunction = pptrFunction; + break; + default: + exposedFunction = pptrFunction.default; + break; } this.#pageBindings.set(name, exposedFunction); @@ -2719,10 +2717,10 @@ export class Page extends EventEmitter { * await page.evaluateOnNewDocument(preloadFile); * ``` */ - async evaluateOnNewDocument( - pageFunction: Function | string, - ...args: unknown[] - ): Promise { + async evaluateOnNewDocument< + Params extends unknown[], + Func extends (...args: Params) => unknown = (...args: Params) => unknown + >(pageFunction: Func | string, ...args: Params): Promise { const source = evaluationString(pageFunction, ...args); await this.#client.send('Page.addScriptToEvaluateOnNewDocument', { source, @@ -3315,15 +3313,19 @@ export class Page extends EventEmitter { * (30 seconds). Pass `0` to disable timeout. The default value can be changed * by using the {@link Page.setDefaultTimeout} method. */ - waitForSelector( + async waitForSelector( + selector: Selector, + options?: Exclude + ): Promise | null>; + async waitForSelector( selector: string, - options: { - visible?: boolean; - hidden?: boolean; - timeout?: number; - } = {} + options?: Exclude + ): Promise; + async waitForSelector( + selector: string, + options: Exclude = {} ): Promise { - return this.mainFrame().waitForSelector(selector, options); + return await this.mainFrame().waitForSelector(selector, options); } /** diff --git a/test/src/waittask.spec.ts b/test/src/waittask.spec.ts index 2e01ca3c8bfa5..b75d19f744931 100644 --- a/test/src/waittask.spec.ts +++ b/test/src/waittask.spec.ts @@ -15,7 +15,6 @@ */ import expect from 'expect'; -import sinon from 'sinon'; import {isErrorLike} from '../../lib/cjs/puppeteer/common/util.js'; import { getTestState, @@ -29,122 +28,6 @@ describe('waittask specs', function () { setupTestBrowserHooks(); setupTestPageAndContextHooks(); - describe('Page.waitFor', function () { - /* This method is deprecated but we don't want the warnings showing up in - * tests. Until we remove this method we still want to ensure we don't break - * it. - */ - beforeEach(() => { - return sinon.stub(console, 'warn').callsFake(() => {}); - }); - - it('should wait for selector', async () => { - const {page, server} = getTestState(); - - let found = false; - const waitFor = page.waitForSelector('div').then(() => { - return (found = true); - }); - await page.goto(server.EMPTY_PAGE); - expect(found).toBe(false); - await page.goto(server.PREFIX + '/grid.html'); - await waitFor; - expect(found).toBe(true); - }); - - it('should wait for an xpath', async () => { - const {page, server} = getTestState(); - - let found = false; - const waitFor = page.waitForXPath('//div').then(() => { - return (found = true); - }); - await page.goto(server.EMPTY_PAGE); - expect(found).toBe(false); - await page.goto(server.PREFIX + '/grid.html'); - await waitFor; - expect(found).toBe(true); - }); - it('should allow you to select an element with parenthesis-starting xpath', async () => { - const {page, server} = getTestState(); - let found = false; - const waitFor = page.waitForXPath('(//img)[200]').then(() => { - found = true; - }); - await page.goto(server.EMPTY_PAGE); - expect(found).toBe(false); - await page.goto(server.PREFIX + '/grid.html'); - await waitFor; - expect(found).toBe(true); - }); - it('should not allow you to select an element with single slash xpath', async () => { - const {page} = getTestState(); - - await page.setContent(`
some text
`); - let error!: Error; - await page.waitForXPath('/html/body/div').catch(error_ => { - return (error = error_); - }); - expect(error).toBeTruthy(); - }); - it('should timeout', async () => { - const {page} = getTestState(); - - const startTime = Date.now(); - const timeout = 42; - await page.waitForTimeout(timeout); - expect(Date.now() - startTime).not.toBeLessThan(timeout / 2); - }); - it('should work with multiline body', async () => { - const {page} = getTestState(); - - const result = await page.waitForFunction(` - (() => true)() - `); - expect(await result.jsonValue()).toBe(true); - }); - it('should wait for predicate', async () => { - const {page} = getTestState(); - - await Promise.all([ - page.waitForFunction(() => { - return window.innerWidth < 100; - }), - page.setViewport({width: 10, height: 10}), - ]); - }); - it('should wait for predicate with arguments', async () => { - const {page} = getTestState(); - - await page.waitForFunction( - (arg1, arg2) => { - return arg1 !== arg2; - }, - {}, - 1, - 2 - ); - }); - - it('should log a deprecation warning', async () => { - const {page} = getTestState(); - - await page.waitForFunction(() => { - return true; - }); - - const consoleWarnStub = console.warn as sinon.SinonSpy; - - expect(consoleWarnStub.calledOnce).toBe(true); - expect( - consoleWarnStub.firstCall.calledWith( - 'waitFor is deprecated and will be removed in a future release. See https://github.com/puppeteer/puppeteer/issues/6214 for details and how to migrate your code.' - ) - ).toBe(true); - expect((console.warn as sinon.SinonSpy).calledOnce).toBe(true); - }); - }); - describe('Frame.waitForFunction', function () { it('should accept a string', async () => { const {page} = getTestState();