Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: migrate src/ExecutionContext #5705

Merged
merged 3 commits into from Apr 22, 2020
Merged

chore: migrate src/ExecutionContext #5705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b4f31c
chore: migrate src/ExecutionContext to TypeScript
jackfranklin Apr 21, 2020
f863119
whoops
jackfranklin Apr 22, 2020
fd69197
linting
jackfranklin Apr 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion .eslintrc.js
View file
Open in desktop
Expand Up @@ -114,7 +114,9 @@ module.exports = {
"semi": 0,
"@typescript-eslint/semi": 2,
"@typescript-eslint/no-empty-function": 0,
"@typescript-eslint/no-use-before-define": 0
"@typescript-eslint/no-use-before-define": 0,
// We know it's bad and use it very sparingly but it's needed :(
"@typescript-eslint/ban-ts-ignore": 0
}
}
]
Expand Down
9 changes: 6 additions & 3 deletions src/DOMWorld.js
View file
Open in desktop
Expand Up @@ -21,6 +21,9 @@ const {TimeoutError} = require('./Errors');
// Used as a TypeDef
// eslint-disable-next-line no-unused-vars
const {JSHandle, ElementHandle} = require('./JSHandle');
// Used as a TypeDef
// eslint-disable-next-line no-unused-vars
const {ExecutionContext} = require('./ExecutionContext');

// Used as a TypeDef
// eslint-disable-next-line no-unused-vars
Expand All @@ -44,7 +47,7 @@ class DOMWorld {

/** @type {?Promise<!ElementHandle>} */
this._documentPromise = null;
/** @type {!Promise<!Puppeteer.ExecutionContext>} */
/** @type {!Promise<!ExecutionContext>} */
this._contextPromise;
this._contextResolveCallback = null;
this._setContext(null);
Expand All @@ -62,7 +65,7 @@ class DOMWorld {
}

/**
* @param {?Puppeteer.ExecutionContext} context
* @param {?ExecutionContext} context
*/
_setContext(context) {
if (context) {
Expand Down Expand Up @@ -92,7 +95,7 @@ class DOMWorld {
}

/**
* @return {!Promise<!Puppeteer.ExecutionContext>}
* @return {!Promise<!ExecutionContext>}
*/
executionContext() {
if (this._detached)
Expand Down
93 changes: 27 additions & 66 deletions src/ExecutionContext.js → src/ExecutionContext.ts
View file
Open in desktop
Expand Up @@ -14,76 +14,55 @@
* limitations under the License.
*/

const {helper, assert} = require('./helper');
// Used as a TypeDef
// eslint-disable-next-line no-unused-vars
const {CDPSession} = require('./Connection');
// Used as a TypeDef
// eslint-disable-next-line no-unused-vars
const {createJSHandle, JSHandle, ElementHandle} = require('./JSHandle');

const EVALUATION_SCRIPT_URL = '__puppeteer_evaluation_script__';
import {helper, assert} from './helper';
import {createJSHandle, JSHandle, ElementHandle} from './JSHandle';
import {CDPSession} from './Connection';

export const EVALUATION_SCRIPT_URL = '__puppeteer_evaluation_script__';
const SOURCE_URL_REGEX = /^[\040\t]*\/\/[@#] sourceURL=\s*(\S*?)\s*$/m;

class ExecutionContext {
/**
* @param {!CDPSession} client
* @param {!Protocol.Runtime.ExecutionContextDescription} contextPayload
* @param {?Puppeteer.DOMWorld} world
*/
constructor(client, contextPayload, world) {
export class ExecutionContext {
_client: CDPSession;
_world: Puppeteer.DOMWorld;
_contextId: number;

constructor(client: CDPSession, contextPayload: Protocol.Runtime.ExecutionContextDescription, world: Puppeteer.DOMWorld) {
this._client = client;
this._world = world;
this._contextId = contextPayload.id;
}

/**
* @return {?Puppeteer.Frame}
*/
frame() {
frame(): Puppeteer.Frame | null {
return this._world ? this._world.frame() : null;
}

/**
* @param {Function|string} pageFunction
* @param {...*} args
* @return {!Promise<*>}
*/
async evaluate(pageFunction, ...args) {
return await this._evaluateInternal(true /* returnByValue */, pageFunction, ...args);
async evaluate<ReturnType extends any>(pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
return await this._evaluateInternal<ReturnType>(true, pageFunction, ...args);
}

/**
* @param {Function|string} pageFunction
* @param {...*} args
* @return {!Promise<!JSHandle>}
*/
async evaluateHandle(pageFunction, ...args) {
return this._evaluateInternal(false /* returnByValue */, pageFunction, ...args);
async evaluateHandle(pageFunction: Function | string, ...args: unknown[]): Promise<JSHandle> {
return this._evaluateInternal<JSHandle>(false, pageFunction, ...args);
}

/**
* @param {boolean} returnByValue
* @param {Function|string} pageFunction
* @param {...*} args
* @return {!Promise<*>}
*/
async _evaluateInternal(returnByValue, pageFunction, ...args) {
private async _evaluateInternal<ReturnType>(returnByValue, pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
const suffix = `//# sourceURL=${EVALUATION_SCRIPT_URL}`;

if (helper.isString(pageFunction)) {
const contextId = this._contextId;
const expression = /** @type {string} */ (pageFunction);
const expression = pageFunction;
const expressionWithSourceUrl = SOURCE_URL_REGEX.test(expression) ? expression : expression + '\n' + suffix;

const {exceptionDetails, result: remoteObject} = await this._client.send('Runtime.evaluate', {
expression: expressionWithSourceUrl,
contextId,
returnByValue,
awaitPromise: true,
userGesture: true
}).catch(rewriteError);

if (exceptionDetails)
throw new Error('Evaluation failed: ' + helper.getExceptionMessage(exceptionDetails));

return returnByValue ? helper.valueFromRemoteObject(remoteObject) : createJSHandle(this, remoteObject);
}

Expand Down Expand Up @@ -132,7 +111,7 @@ class ExecutionContext {
* @return {*}
* @this {ExecutionContext}
*/
function convertArgument(arg) {
function convertArgument(this: ExecutionContext, arg: unknown): unknown {
if (typeof arg === 'bigint') // eslint-disable-line valid-typeof
return {unserializableValue: `${arg.toString()}n`};
if (Object.is(arg, -0))
Expand All @@ -158,11 +137,7 @@ class ExecutionContext {
return {value: arg};
}

/**
* @param {!Error} error
* @return {!Protocol.Runtime.evaluateReturnValue}
*/
function rewriteError(error) {
function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue {
if (error.message.includes('Object reference chain is too long'))
return {result: {type: 'undefined'}};
if (error.message.includes('Object couldn\'t be returned by value'))
Expand All @@ -174,11 +149,7 @@ class ExecutionContext {
}
}

/**
* @param {!JSHandle} prototypeHandle
* @return {!Promise<!JSHandle>}
*/
async queryObjects(prototypeHandle) {
async queryObjects(prototypeHandle: JSHandle): Promise<JSHandle> {
assert(!prototypeHandle._disposed, 'Prototype JSHandle is disposed!');
assert(prototypeHandle._remoteObject.objectId, 'Prototype JSHandle must not be referencing primitive value');
const response = await this._client.send('Runtime.queryObjects', {
Expand All @@ -187,23 +158,15 @@ class ExecutionContext {
return createJSHandle(this, response.objects);
}

/**
* @param {Protocol.DOM.BackendNodeId} backendNodeId
* @return {Promise<ElementHandle>}
*/
async _adoptBackendNodeId(backendNodeId) {
async _adoptBackendNodeId(backendNodeId: Protocol.DOM.BackendNodeId): Promise<ElementHandle> {
const {object} = await this._client.send('DOM.resolveNode', {
backendNodeId: backendNodeId,
executionContextId: this._contextId,
});
return /** @type {ElementHandle}*/(createJSHandle(this, object));
return createJSHandle(this, object) as ElementHandle;
}

/**
* @param {ElementHandle} elementHandle
* @return {Promise<ElementHandle>}
*/
async _adoptElementHandle(elementHandle) {
async _adoptElementHandle(elementHandle: ElementHandle): Promise<ElementHandle> {
assert(elementHandle.executionContext() !== this, 'Cannot adopt handle that already belongs to this execution context');
assert(this._world, 'Cannot adopt handle without DOMWorld');
const nodeInfo = await this._client.send('DOM.describeNode', {
Expand All @@ -212,5 +175,3 @@ class ExecutionContext {
return this._adoptBackendNodeId(nodeInfo.node.backendNodeId);
}
}

module.exports = {ExecutionContext, EVALUATION_SCRIPT_URL};
51 changes: 28 additions & 23 deletions src/JSHandle.ts
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@
*/

import {helper, assert, debugError} from './helper';
import {ExecutionContext} from './ExecutionContext';
import {CDPSession} from './Connection';

interface BoxModel {
Expand All @@ -26,7 +27,7 @@ interface BoxModel {
height: number;
}

export function createJSHandle(context: Puppeteer.ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): JSHandle {
export function createJSHandle(context: ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): JSHandle {
const frame = context.frame();
if (remoteObject.subtype === 'node' && frame) {
const frameManager = frame._frameManager;
Expand All @@ -36,31 +37,31 @@ export function createJSHandle(context: Puppeteer.ExecutionContext, remoteObject
}

export class JSHandle {
_context: Puppeteer.ExecutionContext;
_context: ExecutionContext;
_client: CDPSession;
_remoteObject: Protocol.Runtime.RemoteObject;
_disposed = false;

constructor(context: Puppeteer.ExecutionContext, client: CDPSession, remoteObject: Protocol.Runtime.RemoteObject) {
constructor(context: ExecutionContext, client: CDPSession, remoteObject: Protocol.Runtime.RemoteObject) {
this._context = context;
this._client = client;
this._remoteObject = remoteObject;
}

executionContext(): Puppeteer.ExecutionContext {
executionContext(): ExecutionContext {
return this._context;
}

async evaluate<T extends any>(pageFunction: Function | string, ...args: any[]): Promise<T | undefined> {
return await this.executionContext().evaluate(pageFunction, this, ...args);
async evaluate<ReturnType extends any>(pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
return await this.executionContext().evaluate<ReturnType>(pageFunction, this, ...args);
}

async evaluateHandle(pageFunction: Function | string, ...args: any[]): Promise<JSHandle> {
async evaluateHandle(pageFunction: Function | string, ...args: unknown[]): Promise<JSHandle> {
return await this.executionContext().evaluateHandle(pageFunction, this, ...args);
}

async getProperty(propertyName: string): Promise<JSHandle | undefined> {
const objectHandle = await this.evaluateHandle((object, propertyName) => {
const objectHandle = await this.evaluateHandle((object: HTMLElement, propertyName: string) => {
const result = {__proto__: null};
result[propertyName] = object[propertyName];
return result;
Expand Down Expand Up @@ -123,13 +124,13 @@ export class ElementHandle extends JSHandle {
_page: Puppeteer.Page;
_frameManager: Puppeteer.FrameManager;
/**
* @param {!Puppeteer.ExecutionContext} context
* @param {!ExecutionContext} context
* @param {!CDPSession} client
* @param {!Protocol.Runtime.RemoteObject} remoteObject
* @param {!Puppeteer.Page} page
* @param {!Puppeteer.FrameManager} frameManager
*/
constructor(context: Puppeteer.ExecutionContext, client: CDPSession, remoteObject: Protocol.Runtime.RemoteObject, page: Puppeteer.Page, frameManager: Puppeteer.FrameManager) {
constructor(context: ExecutionContext, client: CDPSession, remoteObject: Protocol.Runtime.RemoteObject, page: Puppeteer.Page, frameManager: Puppeteer.FrameManager) {
super(context, client, remoteObject);
this._client = client;
this._remoteObject = remoteObject;
Expand All @@ -151,13 +152,16 @@ export class ElementHandle extends JSHandle {
}

async _scrollIntoViewIfNeeded(): Promise<void> {
const error = await this.evaluate<string | false>(async(element, pageJavascriptEnabled) => {
const error = await this.evaluate<Promise<string | false>>(async(element: HTMLElement, pageJavascriptEnabled: boolean) => {
if (!element.isConnected)
return 'Node is detached from document';
if (element.nodeType !== Node.ELEMENT_NODE)
return 'Node is not of type HTMLElement';
// force-scroll if page's javascript is disabled.
if (!pageJavascriptEnabled) {
// Chrome still supports behavior: instant but it's not in the spec so TS shouts
// We don't want to make this breaking change in Puppeteer yet so we'll ignore the line.
// @ts-ignore
element.scrollIntoView({block: 'center', inline: 'center', behavior: 'instant'});
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting: TS shouts here because I think instant was going to be in the spec but now isn't? So I've ditched this in favour of auto which is the default. @mathiasbynens WDYT? I'm not sure if Chrome does still support instant but if it's going/gone from the spec presumably we'll stop supporting it one day.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woah, this is a breaking change. We shouldn't work around TypeScript bugs by changing observable behavior in Puppeteer. Chrome still supports behavior: 'instant'. Can we make TypeScript ignore this particular line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathiasbynens this isn't a TypeScript bug, but fine, yes we can.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It’s arguably a bug: it doesn’t match real-world browser behavior. In general, when the spec and implementations disagree, then implementations take precedence, IMHO. This is a separate discussion though :])

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keyword there is "arguably" ;) 😂

return false;
}
Expand All @@ -168,8 +172,12 @@ export class ElementHandle extends JSHandle {
});
observer.observe(element);
});
if (visibleRatio !== 1.0)
if (visibleRatio !== 1.0) {
// Chrome still supports behavior: instant but it's not in the spec so TS shouts
// We don't want to make this breaking change in Puppeteer yet so we'll ignore the line.
// @ts-ignore
element.scrollIntoView({block: 'center', inline: 'center', behavior: 'instant'});
}
return false;
}, this._page._javascriptEnabled);

Expand Down Expand Up @@ -275,7 +283,7 @@ export class ElementHandle extends JSHandle {
}

async uploadFile(...filePaths: string[]): Promise<void> {
const isMultiple = await this.evaluate(element => element.multiple);
const isMultiple = await this.evaluate<boolean>((element: HTMLInputElement) => element.multiple);
assert(filePaths.length <= 1 || isMultiple, 'Multiple file uploads only work with <input type=file multiple>');

// This import is only needed for `uploadFile`, so keep it scoped here to avoid paying
Expand All @@ -291,7 +299,7 @@ export class ElementHandle extends JSHandle {
// not actually update the files in that case, so the solution is to eval the element
// value to a new FileList directly.
if (files.length === 0) {
await this.evaluate(element => {
await this.evaluate((element: HTMLInputElement) => {
element.files = new DataTransfer().files;

// Dispatch events for this case because it should behave akin to a user action.
Expand Down Expand Up @@ -431,25 +439,22 @@ export class ElementHandle extends JSHandle {
return result;
}

async $eval<T extends any>(selector: string, pageFunction: Function|string, ...args: any[]): Promise<T | undefined> {
async $eval<ReturnType extends any>(selector: string, pageFunction: Function|string, ...args: unknown[]): Promise<ReturnType> {
const elementHandle = await this.$(selector);
if (!elementHandle)
throw new Error(`Error: failed to find element matching selector "${selector}"`);
const result = await elementHandle.evaluate(pageFunction, ...args);
const result = await elementHandle.evaluate<ReturnType>(pageFunction, ...args);
await elementHandle.dispose();
return result;
}

// TODO(jacktfranklin@): consider the types here
// we might want $$eval<SelectorType> which returns Promise<SelectorType[]>?
// Once ExecutionContext.evaluate is properly typed we can improve this a bunch
async $$eval<T>(selector: string, pageFunction: Function | string, ...args: any[]): Promise<T | undefined> {
async $$eval<ReturnType extends any>(selector: string, pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
const arrayHandle = await this.evaluateHandle(
(element, selector) => Array.from(element.querySelectorAll(selector)),
selector
);

const result = await arrayHandle.evaluate(pageFunction, ...args);
const result = await arrayHandle.evaluate<ReturnType>(pageFunction, ...args);
await arrayHandle.dispose();
return result;
}
Expand Down Expand Up @@ -478,8 +483,8 @@ export class ElementHandle extends JSHandle {
return result;
}

isIntersectingViewport(): Promise<boolean> {
return this.evaluate(async element => {
async isIntersectingViewport(): Promise<boolean> {
return await this.evaluate<Promise<boolean>>(async element => {
const visibleRatio = await new Promise(resolve => {
const observer = new IntersectionObserver(entries => {
resolve(entries[0].intersectionRatio);
Expand Down
2 changes: 0 additions & 2 deletions src/externs.d.ts
View file
Open in desktop
Expand Up @@ -4,7 +4,6 @@ import {Page as RealPage} from './Page.js';
import {Mouse as RealMouse, Keyboard as RealKeyboard, Touchscreen as RealTouchscreen} from './Input.js';
import {Frame as RealFrame, FrameManager as RealFrameManager} from './FrameManager.js';
import {DOMWorld as RealDOMWorld} from './DOMWorld.js';
import {ExecutionContext as RealExecutionContext} from './ExecutionContext.js';
import { NetworkManager as RealNetworkManager, Request as RealRequest, Response as RealResponse } from './NetworkManager.js';
import * as child_process from 'child_process';
declare global {
Expand All @@ -19,7 +18,6 @@ declare global {
export class FrameManager extends RealFrameManager {}
export class NetworkManager extends RealNetworkManager {}
export class DOMWorld extends RealDOMWorld {}
export class ExecutionContext extends RealExecutionContext {}
export class Page extends RealPage { }
export class Response extends RealResponse { }
export class Request extends RealRequest { }
Expand Down