Skip to content

Commit

Permalink
fix: type page event listeners correctly (#6891)
Browse files Browse the repository at this point in the history
This PR fixes the fact that currently if you have:

```ts
page.on('request', request => {

})
```

Then `request` will be typed as `any`. We can fix this by defining an
interface of event name => callback argument type, and looking that up
when you call `page.on`.

Also includes a drive-by fix to ensure we convert response headers to
strings, and updates the types accordingly.
  • Loading branch information
jackfranklin committed Mar 25, 2021
1 parent e31e68d commit 866d34e
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 3 deletions.
11 changes: 8 additions & 3 deletions src/common/HTTPRequest.ts
Expand Up @@ -40,7 +40,10 @@ export interface ContinueRequestOverrides {
*/
export interface ResponseForRequest {
status: number;
headers: Record<string, string>;
/**
* Optional response headers. All values are converted to strings.
*/
headers: Record<string, unknown>;
contentType: string;
body: string | Buffer;
}
Expand Down Expand Up @@ -346,7 +349,7 @@ export class HTTPRequest {
*
* @param response - the response to fulfill the request with.
*/
async respond(response: ResponseForRequest): Promise<void> {
async respond(response: Partial<ResponseForRequest>): Promise<void> {
// Mocking responses for dataURL requests is not currently supported.
if (this._url.startsWith('data:')) return;
assert(this._allowInterception, 'Request Interception is not enabled!');
Expand All @@ -361,7 +364,9 @@ export class HTTPRequest {
const responseHeaders: Record<string, string> = {};
if (response.headers) {
for (const header of Object.keys(response.headers))
responseHeaders[header.toLowerCase()] = response.headers[header];
responseHeaders[header.toLowerCase()] = String(
response.headers[header]
);
}
if (response.contentType)
responseHeaders['content-type'] = response.contentType;
Expand Down
50 changes: 50 additions & 0 deletions src/common/Page.ts
Expand Up @@ -329,6 +329,35 @@ export const enum PageEmittedEvents {
WorkerDestroyed = 'workerdestroyed',
}

/**
* Denotes the objects received by callback functions for page events.
*
* See {@link PageEmittedEvents} for more detail on the events and when they are
* emitted.
* @public
*/
export interface PageEventObject {
close: never;
console: ConsoleMessage;
dialog: Dialog;
domcontentloaded: never;
error: Error;
frameattached: Frame;
framedetached: Frame;
framenavigated: Frame;
load: never;
metrics: { title: string; metrics: Metrics };
pageerror: Error;
popup: Page;
request: HTTPRequest;
response: HTTPResponse;
requestfailed: HTTPRequest;
requestfinished: HTTPRequest;
requestservedfromcache: HTTPRequest;
workercreated: WebWorker;
workerdestroyed: WebWorker;
}

class ScreenshotTaskQueue {
_chain: Promise<Buffer | string | void>;

Expand Down Expand Up @@ -559,6 +588,27 @@ export class Page extends EventEmitter {
return this._javascriptEnabled;
}

/**
* Listen to page events.
*/
public on<K extends keyof PageEventObject>(
eventName: K,
handler: (event: PageEventObject[K]) => void
): EventEmitter {
// Note: this method only exists to define the types; we delegate the impl
// to EventEmitter.
return super.on(eventName, handler);
}

public once<K extends keyof PageEventObject>(
eventName: K,
handler: (event: PageEventObject[K]) => void
): EventEmitter {
// Note: this method only exists to define the types; we delegate the impl
// to EventEmitter.
return super.once(eventName, handler);
}

/**
* @param options - Optional waiting parameters
* @returns Resolves after a page requests a file picker.
Expand Down
3 changes: 3 additions & 0 deletions test-ts-types/ts-cjs-import-cjs-output/good.ts
Expand Up @@ -5,6 +5,9 @@ async function run() {
const devices = puppeteer.devices;
console.log(devices);
const page = await browser.newPage();
page.on('request', (request) => {
const resourceType = request.resourceType();
});
const div = (await page.$('div')) as puppeteer.ElementHandle<
HTMLAnchorElement
>;
Expand Down
21 changes: 21 additions & 0 deletions utils/doclint/check_public_api/index.js
Expand Up @@ -864,6 +864,27 @@ function compareDocumentations(actual, expected) {
expectedName: 'Array<Permission>',
},
],
[
'Method HTTPRequest.respond() response.body',
{
actualName: 'string|Buffer',
expectedName: 'Object',
},
],
[
'Method HTTPRequest.respond() response.contentType',
{
actualName: 'string',
expectedName: 'Object',
},
],
[
'Method HTTPRequest.respond() response.status',
{
actualName: 'number',
expectedName: 'Object',
},
],
]);

const expectedForSource = expectedNamingMismatches.get(source);
Expand Down

0 comments on commit 866d34e

Please sign in to comment.