chore: Migrate TaskQueue to TypeScript (#5658)

This is a simple module but took a bit of work because:

* It wraps a Promise that can return basically anything. In a pure TS
codebase we'd solve these with generics, so you could do `new
TaskQueue<T>` where `T` will be what's returned from the queue, but
because we're calling that from JS we can't yet. I've left a TODO and
once we migrate the call sites to TS we can do a much better job than
the `void | any` type I've gone with for now.

* It was used in typedefs via `Puppeteer.TaskQueue`. I've removed that
entry from `externs.d.ts` in favour of importing it and using the type
directly. This does mean that we have imports that ESLint doesn't
realiase are actually used but I think this is better than maintaining
`externs.d.ts`.
This commit is contained in:
Jack Franklin 2020-04-17 10:32:56 +01:00 committed by GitHub
parent ef3befa2e6
commit 1a57ba22a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 50 additions and 23 deletions

View File

@ -111,7 +111,8 @@ module.exports = {
"no-unused-vars": 0, "no-unused-vars": 0,
"@typescript-eslint/no-unused-vars": 2, "@typescript-eslint/no-unused-vars": 2,
"semi": 0, "semi": 0,
"@typescript-eslint/semi": 2 "@typescript-eslint/semi": 2,
"@typescript-eslint/no-empty-function": 0
} }
} }
] ]

View File

@ -30,6 +30,11 @@ const {Worker: PuppeteerWorker} = require('./Worker');
const {createJSHandle} = require('./JSHandle'); const {createJSHandle} = require('./JSHandle');
const {Accessibility} = require('./Accessibility'); const {Accessibility} = require('./Accessibility');
const {TimeoutSettings} = require('./TimeoutSettings'); const {TimeoutSettings} = require('./TimeoutSettings');
// This import is used as a TypeDef, but ESLint's rule doesn't
// understand that unfortunately.
// eslint-disable-next-line no-unused-vars
const {TaskQueue} = require('./TaskQueue');
const writeFileAsync = helper.promisify(fs.writeFile); const writeFileAsync = helper.promisify(fs.writeFile);
class Page extends EventEmitter { class Page extends EventEmitter {
@ -38,7 +43,7 @@ class Page extends EventEmitter {
* @param {!Puppeteer.Target} target * @param {!Puppeteer.Target} target
* @param {boolean} ignoreHTTPSErrors * @param {boolean} ignoreHTTPSErrors
* @param {?Puppeteer.Viewport} defaultViewport * @param {?Puppeteer.Viewport} defaultViewport
* @param {!Puppeteer.TaskQueue} screenshotTaskQueue * @param {!TaskQueue} screenshotTaskQueue
* @return {!Promise<!Page>} * @return {!Promise<!Page>}
*/ */
static async create(client, target, ignoreHTTPSErrors, defaultViewport, screenshotTaskQueue) { static async create(client, target, ignoreHTTPSErrors, defaultViewport, screenshotTaskQueue) {
@ -53,7 +58,7 @@ class Page extends EventEmitter {
* @param {!Puppeteer.CDPSession} client * @param {!Puppeteer.CDPSession} client
* @param {!Puppeteer.Target} target * @param {!Puppeteer.Target} target
* @param {boolean} ignoreHTTPSErrors * @param {boolean} ignoreHTTPSErrors
* @param {!Puppeteer.TaskQueue} screenshotTaskQueue * @param {!TaskQueue} screenshotTaskQueue
*/ */
constructor(client, target, ignoreHTTPSErrors, screenshotTaskQueue) { constructor(client, target, ignoreHTTPSErrors, screenshotTaskQueue) {
super(); super();

View File

@ -17,6 +17,10 @@
const {Events} = require('./Events'); const {Events} = require('./Events');
const {Page} = require('./Page'); const {Page} = require('./Page');
const {Worker: PuppeteerWorker} = require('./Worker'); const {Worker: PuppeteerWorker} = require('./Worker');
// This import is used as a TypeDef, but ESLint's rule doesn't
// understand that unfortunately.
// eslint-disable-next-line no-unused-vars
const {TaskQueue} = require('./TaskQueue');
class Target { class Target {
/** /**
@ -25,7 +29,7 @@ class Target {
* @param {!function():!Promise<!Puppeteer.CDPSession>} sessionFactory * @param {!function():!Promise<!Puppeteer.CDPSession>} sessionFactory
* @param {boolean} ignoreHTTPSErrors * @param {boolean} ignoreHTTPSErrors
* @param {?Puppeteer.Viewport} defaultViewport * @param {?Puppeteer.Viewport} defaultViewport
* @param {!Puppeteer.TaskQueue} screenshotTaskQueue * @param {!TaskQueue} screenshotTaskQueue
*/ */
constructor(targetInfo, browserContext, sessionFactory, ignoreHTTPSErrors, defaultViewport, screenshotTaskQueue) { constructor(targetInfo, browserContext, sessionFactory, ignoreHTTPSErrors, defaultViewport, screenshotTaskQueue) {
this._targetInfo = targetInfo; this._targetInfo = targetInfo;

View File

@ -1,17 +0,0 @@
class TaskQueue {
constructor() {
this._chain = Promise.resolve();
}
/**
* @param {Function} task
* @return {!Promise}
*/
postTask(task) {
const result = this._chain.then(task);
this._chain = result.catch(() => {});
return result;
}
}
module.exports = {TaskQueue};

36
src/TaskQueue.ts Normal file
View File

@ -0,0 +1,36 @@
/**
* Copyright 2020 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/* TODO(jacktfranklin@): once we are calling this from TS files we can
* avoid the horrible void | any type and instead make use of generics
* to make this into TaskQueue<T> and let the caller tell us what types
* the promise in the queue should return.
*/
class TaskQueue {
_chain: Promise<void | any>;
constructor() {
this._chain = Promise.resolve();
}
public postTask(task: () => any): Promise<void | any> {
const result = this._chain.then(task);
this._chain = result.catch(() => {});
return result;
}
}
export = { TaskQueue };

2
src/externs.d.ts vendored
View File

@ -2,7 +2,6 @@ import { Connection as RealConnection, CDPSession as RealCDPSession } from './Co
import { Browser as RealBrowser, BrowserContext as RealBrowserContext} from './Browser.js'; import { Browser as RealBrowser, BrowserContext as RealBrowserContext} from './Browser.js';
import {Target as RealTarget} from './Target.js'; import {Target as RealTarget} from './Target.js';
import {Page as RealPage} from './Page.js'; import {Page as RealPage} from './Page.js';
import {TaskQueue as RealTaskQueue} from './TaskQueue.js';
import {Mouse as RealMouse, Keyboard as RealKeyboard, Touchscreen as RealTouchscreen} from './Input.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 {Frame as RealFrame, FrameManager as RealFrameManager} from './FrameManager.js';
import {JSHandle as RealJSHandle, ElementHandle as RealElementHandle} from './JSHandle.js'; import {JSHandle as RealJSHandle, ElementHandle as RealElementHandle} from './JSHandle.js';
@ -21,7 +20,6 @@ declare global {
export class Mouse extends RealMouse {} export class Mouse extends RealMouse {}
export class Keyboard extends RealKeyboard {} export class Keyboard extends RealKeyboard {}
export class Touchscreen extends RealTouchscreen {} export class Touchscreen extends RealTouchscreen {}
export class TaskQueue extends RealTaskQueue {}
export class Browser extends RealBrowser {} export class Browser extends RealBrowser {}
export class BrowserContext extends RealBrowserContext {} export class BrowserContext extends RealBrowserContext {}
export class Target extends RealTarget {} export class Target extends RealTarget {}