chore: migrate src/ExecutionContext (#5705)

* chore: migrate src/ExecutionContext to TypeScript

I spent a while trying to decide on the best course of action for
typing the `evaluate` function.

Ideally I wanted to use generics so that as a user you could type
something like:

```
handle.evaluate<HTMLElement, number, boolean>((node, x) => true, 5)
```

And have TypeScript know the arguments of `node` and `x` based on those
generics. But I hit two problems with that:

* you have to have n overloads of `evaluate` to cope for as many number
of arguments as you can be bothered too (e.g. we'd need an overload for
1 arg, 2 args, 3 args, etc)
* I decided it's actually confusing because you don't know as a user
what those generics actually map to.

So in the end I went with one generic which is the return type of the
function:

```
handle.evaluate<boolean>((node, x) => true, 5)
```

And `node` and `x` get typed as `any` which means you can tell TS
yourself:

```
handle.evaluate<boolean>((node: HTMLElement, x:  number) => true, 5)
```

I'd like to find a way to force that the arguments after the function do
match the arguments you've given (in the above example, TS would moan if
I swapped that `5` for `"foo"`), but I tried a few things and to be
honest the complexity of the types wasn't worth it, I don't think.

I'm very open to tweaking these but I'd rather ship this and tweak going
forwards rather than spend hours now tweaking. Once we ship these
typedefs and get feedback from the community I'm sure we can improve
them.

.eslintrc.js

@@ -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
             }
         }
     ]

src/DOMWorld.js

@@ -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
@@ -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);
@@ -62,7 +65,7 @@ class DOMWorld {
   }
 
   /**
-   * @param {?Puppeteer.ExecutionContext} context
+   * @param {?ExecutionContext} context
    */
   _setContext(context) {
     if (context) {
@@ -92,7 +95,7 @@ class DOMWorld {
   }
 
   /**
-   * @return {!Promise<!Puppeteer.ExecutionContext>}
+   * @return {!Promise<!ExecutionContext>}
    */
   executionContext() {
     if (this._detached)

src/ExecutionContext.ts

@@ -14,67 +14,44 @@
  * limitations under the License.
  */
 
-const {helper, assert} = require('./helper');
+import {helper, assert} from './helper';
-// Used as a TypeDef
+import {createJSHandle, JSHandle, ElementHandle} from './JSHandle';
-// eslint-disable-next-line no-unused-vars
+import {CDPSession} from './Connection';
-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__';
+export const EVALUATION_SCRIPT_URL = '__puppeteer_evaluation_script__';
 const SOURCE_URL_REGEX = /^[\040\t]*\/\/[@#] sourceURL=\s*(\S*?)\s*$/m;
 
-class ExecutionContext {
+export class ExecutionContext {
-  /**
+  _client: CDPSession;
-   * @param {!CDPSession} client
+  _world: Puppeteer.DOMWorld;
-   * @param {!Protocol.Runtime.ExecutionContextDescription} contextPayload
+  _contextId: number;
-   * @param {?Puppeteer.DOMWorld} world
+
-   */
+  constructor(client: CDPSession, contextPayload: Protocol.Runtime.ExecutionContextDescription, world: Puppeteer.DOMWorld) {
-  constructor(client, contextPayload, world) {
     this._client = client;
     this._world = world;
     this._contextId = contextPayload.id;
   }
 
-  /**
+  frame(): Puppeteer.Frame | null {
-   * @return {?Puppeteer.Frame}
-   */
-  frame() {
     return this._world ? this._world.frame() : null;
   }
 
-  /**
+  async evaluate<ReturnType extends any>(pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
-   * @param {Function|string} pageFunction
+    return await this._evaluateInternal<ReturnType>(true, pageFunction, ...args);
-   * @param {...*} args
-   * @return {!Promise<*>}
-   */
-  async evaluate(pageFunction, ...args) {
-    return await this._evaluateInternal(true /* returnByValue */, pageFunction, ...args);
   }
 
-  /**
+  async evaluateHandle(pageFunction: Function | string, ...args: unknown[]): Promise<JSHandle> {
-   * @param {Function|string} pageFunction
+    return this._evaluateInternal<JSHandle>(false, pageFunction, ...args);
-   * @param {...*} args
-   * @return {!Promise<!JSHandle>}
-   */
-  async evaluateHandle(pageFunction, ...args) {
-    return this._evaluateInternal(false /* returnByValue */, pageFunction, ...args);
   }
 
-  /**
+  private async _evaluateInternal<ReturnType>(returnByValue, pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
-   * @param {boolean} returnByValue
-   * @param {Function|string} pageFunction
-   * @param {...*} args
-   * @return {!Promise<*>}
-   */
-  async _evaluateInternal(returnByValue, pageFunction, ...args) {
     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,
@@ -82,8 +59,10 @@ class ExecutionContext {
         awaitPromise: true,
         userGesture: true
       }).catch(rewriteError);
+
       if (exceptionDetails)
         throw new Error('Evaluation failed: ' + helper.getExceptionMessage(exceptionDetails));
+
       return returnByValue ? helper.valueFromRemoteObject(remoteObject) : createJSHandle(this, remoteObject);
     }
 
@@ -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))
@@ -158,11 +137,7 @@ class ExecutionContext {
       return {value: arg};
     }
 
-    /**
+    function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue {
-     * @param {!Error} error
-     * @return {!Protocol.Runtime.evaluateReturnValue}
-     */
-    function rewriteError(error) {
       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'))
@@ -174,11 +149,7 @@ class ExecutionContext {
     }
   }
 
-  /**
+  async queryObjects(prototypeHandle: JSHandle): Promise<JSHandle> {
-   * @param {!JSHandle} prototypeHandle
-   * @return {!Promise<!JSHandle>}
-   */
-  async queryObjects(prototypeHandle) {
     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', {
@@ -187,23 +158,15 @@ class ExecutionContext {
     return createJSHandle(this, response.objects);
   }
 
-  /**
+  async _adoptBackendNodeId(backendNodeId: Protocol.DOM.BackendNodeId): Promise<ElementHandle> {
-   * @param {Protocol.DOM.BackendNodeId} backendNodeId
-   * @return {Promise<ElementHandle>}
-   */
-  async _adoptBackendNodeId(backendNodeId) {
     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;
   }
 
-  /**
+  async _adoptElementHandle(elementHandle: ElementHandle): Promise<ElementHandle> {
-   * @param {ElementHandle} elementHandle
-   * @return {Promise<ElementHandle>}
-   */
-  async _adoptElementHandle(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', {
@@ -212,5 +175,3 @@ class ExecutionContext {
     return this._adoptBackendNodeId(nodeInfo.node.backendNodeId);
   }
 }
-
-module.exports = {ExecutionContext, EVALUATION_SCRIPT_URL};

src/JSHandle.ts

@@ -15,6 +15,7 @@
  */
 
 import {helper, assert, debugError} from './helper';
+import {ExecutionContext} from './ExecutionContext';
 import {CDPSession} from './Connection';
 
 interface BoxModel {
@@ -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;
@@ -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> {
+  async evaluate<ReturnType extends any>(pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
-    return await this.executionContext().evaluate(pageFunction, this, ...args);
+    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;
@@ -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;
@@ -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'});
         return false;
       }
@@ -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);
 
@@ -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
@@ -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.
@@ -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
+  async $$eval<ReturnType extends any>(selector: string, pageFunction: Function | string, ...args: unknown[]): Promise<ReturnType> {
-  // 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> {
     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;
   }
@@ -478,8 +483,8 @@ export class ElementHandle extends JSHandle {
     return result;
   }
 
-  isIntersectingViewport(): Promise<boolean> {
+  async isIntersectingViewport(): Promise<boolean> {
-    return this.evaluate(async element => {
+    return await this.evaluate<Promise<boolean>>(async element => {
       const visibleRatio = await new Promise(resolve => {
         const observer = new IntersectionObserver(entries => {
           resolve(entries[0].intersectionRatio);

src/externs.d.ts

@@ -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 {
@@ -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 { }

src/helper.ts

@@ -52,7 +52,7 @@ function getExceptionMessage(exceptionDetails: Protocol.Runtime.ExceptionDetails
   return message;
 }
 
-function valueFromRemoteObject(remoteObject: Protocol.Runtime.RemoteObject): unknown {
+function valueFromRemoteObject(remoteObject: Protocol.Runtime.RemoteObject): any {
   assert(!remoteObject.objectId, 'Cannot extract value when objectId is given');
   if (remoteObject.unserializableValue) {
     if (remoteObject.type === 'bigint' && typeof BigInt !== 'undefined')