chore: migrate src/Target to TypeScript (puppeteer#5771)

ajbouh · Apr 29, 2020 · af2e458 · af2e458
1 parent 8a5008e
commit af2e458
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 83 deletions.
diff --git a/src/Browser.ts b/src/Browser.ts
@@ -23,12 +23,6 @@ import {Connection} from './Connection';
 
 type BrowserCloseCallback = () => Promise<void> | void;
 
-/* TODO(jacktfranklin): once Target is migrated to TS
- * we can import + use its type here. But right now Target
- * is implemented in JS so we can't import the type and have to use
- * Puppeteer.Target
- */
-
 export class Browser extends EventEmitter {
   static async create(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: Puppeteer.ChildProcess, closeCallback?: BrowserCloseCallback): Promise<Browser> {
     const browser = new Browser(connection, contextIds, ignoreHTTPSErrors, defaultViewport, process, closeCallback);
@@ -44,7 +38,7 @@ export class Browser extends EventEmitter {
    _defaultContext: BrowserContext;
    _contexts: Map<string, BrowserContext>;
    // TODO: once Target is in TypeScript we can type this properly.
-   _targets: Map<string, Puppeteer.Target>;
+   _targets: Map<string, Target>;
 
    constructor(connection: Connection, contextIds: string[], ignoreHTTPSErrors: boolean, defaultViewport?: Puppeteer.Viewport, process?: Puppeteer.ChildProcess, closeCallback?: BrowserCloseCallback) {
      super();
@@ -154,11 +148,11 @@ export class Browser extends EventEmitter {
      return page;
    }
 
-   targets(): Puppeteer.Target[] {
+   targets(): Target[] {
      return Array.from(this._targets.values()).filter(target => target._isInitialized);
    }
 
-   target(): Puppeteer.Target {
+   target(): Target {
      return this.targets().find(target => target.type() === 'browser');
    }
 
@@ -167,27 +161,27 @@ export class Browser extends EventEmitter {
    * @param {{timeout?: number}=} options
    * @return {!Promise<!Target>}
    */
-   async waitForTarget(predicate: (x: Puppeteer.Target) => boolean, options: { timeout?: number} = {}): Promise<Puppeteer.Target> {
+   async waitForTarget(predicate: (x: Target) => boolean, options: { timeout?: number} = {}): Promise<Target> {
      const {
        timeout = 30000
      } = options;
      const existingTarget = this.targets().find(predicate);
      if (existingTarget)
        return existingTarget;
      let resolve;
-     const targetPromise = new Promise<Puppeteer.Target>(x => resolve = x);
+     const targetPromise = new Promise<Target>(x => resolve = x);
      this.on(Events.Browser.TargetCreated, check);
      this.on(Events.Browser.TargetChanged, check);
      try {
        if (!timeout)
          return await targetPromise;
-       return await helper.waitWithTimeout<Puppeteer.Target>(targetPromise, 'target', timeout);
+       return await helper.waitWithTimeout<Target>(targetPromise, 'target', timeout);
      } finally {
        this.removeListener(Events.Browser.TargetCreated, check);
        this.removeListener(Events.Browser.TargetChanged, check);
      }
 
-     function check(target: Puppeteer.Target): void  {
+     function check(target: Target): void  {
        if (predicate(target))
          resolve(target);
      }
@@ -242,11 +236,11 @@ export class BrowserContext extends EventEmitter {
     this._id = contextId;
   }
 
-  targets(): Puppeteer.Target[] {
+  targets(): Target[] {
     return this._browser.targets().filter(target => target.browserContext() === this);
   }
 
-  waitForTarget(predicate: (x: Puppeteer.Target) => boolean, options: { timeout?: number}): Promise<Puppeteer.Target> {
+  waitForTarget(predicate: (x: Target) => boolean, options: { timeout?: number}): Promise<Target> {
     return this._browser.waitForTarget(target => target.browserContext() === this && predicate(target), options);
   }
 

diff --git a/src/Page.js b/src/Page.js
@@ -36,6 +36,9 @@ const {Worker: PuppeteerWorker} = require('./Worker');
 const {Browser, BrowserContext} = require('./Browser');
 // Import used as typedef
 // eslint-disable-next-line no-unused-vars
+const {Target} = require('./Target');
+// Import used as typedef
+// eslint-disable-next-line no-unused-vars
 const {createJSHandle, JSHandle, ElementHandle} = require('./JSHandle');
 const {Accessibility} = require('./Accessibility');
 const {TimeoutSettings} = require('./TimeoutSettings');
@@ -49,7 +52,7 @@ const writeFileAsync = helper.promisify(fs.writeFile);
 class Page extends EventEmitter {
   /**
    * @param {!CDPSession} client
-   * @param {!Puppeteer.Target} target
+   * @param {!Target} target
    * @param {boolean} ignoreHTTPSErrors
    * @param {?Puppeteer.Viewport} defaultViewport
    * @param {!TaskQueue} screenshotTaskQueue
@@ -65,7 +68,7 @@ class Page extends EventEmitter {
 
   /**
    * @param {!CDPSession} client
-   * @param {!Puppeteer.Target} target
+   * @param {!Target} target
    * @param {boolean} ignoreHTTPSErrors
    * @param {!TaskQueue} screenshotTaskQueue
    */
@@ -202,7 +205,7 @@ class Page extends EventEmitter {
   }
 
   /**
-   * @return {!Puppeteer.Target}
+   * @return {!Target}
    */
   target() {
     return this._target;

diff --git a/src/Target.js → src/Target.ts b/src/Target.js → src/Target.ts
@@ -14,30 +14,30 @@
  * limitations under the License.
  */
 
-const {Events} = require('./Events');
-const {Page} = require('./Page');
-const {Worker: PuppeteerWorker} = require('./Worker');
-// CDPSession is used only as a typedef
-// eslint-disable-next-line no-unused-vars
-const {CDPSession} = require('./Connection');
-// 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');
-// Import used as typedef
-// eslint-disable-next-line no-unused-vars
-const {Browser, BrowserContext} = require('./Browser');
+import {Events} from './Events';
+import {Page} from './Page';
+import {Worker as PuppeteerWorker} from './Worker';
+import {CDPSession} from './Connection';
+import {TaskQueue} from './TaskQueue';
+import {Browser, BrowserContext} from './Browser';
 
-class Target {
-  /**
-   * @param {!Protocol.Target.TargetInfo} targetInfo
-   * @param {!BrowserContext} browserContext
-   * @param {!function():!Promise<!CDPSession>} sessionFactory
-   * @param {boolean} ignoreHTTPSErrors
-   * @param {?Puppeteer.Viewport} defaultViewport
-   * @param {!TaskQueue} screenshotTaskQueue
-   */
-  constructor(targetInfo, browserContext, sessionFactory, ignoreHTTPSErrors, defaultViewport, screenshotTaskQueue) {
+export class Target {
+  _targetInfo: Protocol.Target.TargetInfo;
+  _browserContext: BrowserContext;
+  _targetId: string;
+  _sessionFactory: () => Promise<CDPSession>;
+  _ignoreHTTPSErrors: boolean;
+  _defaultViewport?: Puppeteer.Viewport;
+  _screenshotTaskQueue: TaskQueue;
+  _pagePromise?: Promise<Puppeteer.Page>;
+  _workerPromise?: Promise<PuppeteerWorker>;
+  _initializedPromise: Promise<boolean>;
+  _initializedCallback: (x: boolean) => void;
+  _isClosedPromise: Promise<boolean>;
+  _closedCallback: () => void;
+  _isInitialized: boolean;
+
+  constructor(targetInfo: Protocol.Target.TargetInfo, browserContext: BrowserContext, sessionFactory: () => Promise<CDPSession>, ignoreHTTPSErrors: boolean, defaultViewport: Puppeteer.Viewport | null, screenshotTaskQueue: TaskQueue) {
     this._targetInfo = targetInfo;
     this._browserContext = browserContext;
     this._targetId = targetInfo.targetId;
@@ -49,7 +49,7 @@ class Target {
     this._pagePromise = null;
     /** @type {?Promise<!PuppeteerWorker>} */
     this._workerPromise = null;
-    this._initializedPromise = new Promise(fulfill => this._initializedCallback = fulfill).then(async success => {
+    this._initializedPromise = new Promise<boolean>(fulfill => this._initializedCallback = fulfill).then(async success => {
       if (!success)
         return false;
       const opener = this.opener();
@@ -62,34 +62,25 @@ class Target {
       openerPage.emit(Events.Page.Popup, popupPage);
       return true;
     });
-    this._isClosedPromise = new Promise(fulfill => this._closedCallback = fulfill);
+    this._isClosedPromise = new Promise<boolean>(fulfill => this._closedCallback = fulfill);
     this._isInitialized = this._targetInfo.type !== 'page' || this._targetInfo.url !== '';
     if (this._isInitialized)
       this._initializedCallback(true);
   }
 
-  /**
-   * @return {!Promise<!CDPSession>}
-   */
-  createCDPSession() {
+  createCDPSession(): Promise<CDPSession> {
     return this._sessionFactory();
   }
 
-  /**
-   * @return {!Promise<?Page>}
-   */
-  async page() {
+  async page(): Promise<Puppeteer.Page | null> {
     if ((this._targetInfo.type === 'page' || this._targetInfo.type === 'background_page') && !this._pagePromise) {
       this._pagePromise = this._sessionFactory()
           .then(client => Page.create(client, this, this._ignoreHTTPSErrors, this._defaultViewport, this._screenshotTaskQueue));
     }
     return this._pagePromise;
   }
 
-  /**
-   * @return {!Promise<?PuppeteerWorker>}
-   */
-  async worker() {
+  async worker(): Promise<PuppeteerWorker | null> {
     if (this._targetInfo.type !== 'service_worker' && this._targetInfo.type !== 'shared_worker')
       return null;
     if (!this._workerPromise) {
@@ -100,51 +91,33 @@ class Target {
     return this._workerPromise;
   }
 
-  /**
-   * @return {string}
-   */
-  url() {
+  url(): string {
     return this._targetInfo.url;
   }
 
-  /**
-   * @return {"page"|"background_page"|"service_worker"|"shared_worker"|"other"|"browser"}
-   */
-  type() {
+  type(): 'page'|'background_page'|'service_worker'|'shared_worker'|'other'|'browser'{
     const type = this._targetInfo.type;
     if (type === 'page' || type === 'background_page' || type === 'service_worker' || type === 'shared_worker' || type === 'browser')
       return type;
     return 'other';
   }
 
-  /**
-   * @return {!Browser}
-   */
-  browser() {
+  browser(): Browser {
     return this._browserContext.browser();
   }
 
-  /**
-   * @return {!BrowserContext}
-   */
-  browserContext() {
+  browserContext(): BrowserContext {
     return this._browserContext;
   }
 
-  /**
-   * @return {?Puppeteer.Target}
-   */
-  opener() {
+  opener(): Target | null {
     const {openerId} = this._targetInfo;
     if (!openerId)
       return null;
     return this.browser()._targets.get(openerId);
   }
 
-  /**
-   * @param {!Protocol.Target.TargetInfo} targetInfo
-   */
-  _targetInfoChanged(targetInfo) {
+  _targetInfoChanged(targetInfo: Protocol.Target.TargetInfo): void {
     this._targetInfo = targetInfo;
 
     if (!this._isInitialized && (this._targetInfo.type !== 'page' || this._targetInfo.url !== '')) {
@@ -154,5 +127,3 @@ class Target {
     }
   }
 }
-
-module.exports = {Target};
diff --git a/src/externs.d.ts b/src/externs.d.ts
@@ -1,10 +1,8 @@
-import {Target as RealTarget} from './Target.js';
 import {Page as RealPage} from './Page.js';
 import { NetworkManager as RealNetworkManager, Request as RealRequest, Response as RealResponse } from './NetworkManager.js';
 import * as child_process from 'child_process';
 declare global {
   module Puppeteer {
-    export class Target extends RealTarget {}
     export class NetworkManager extends RealNetworkManager {}
     export class Page extends RealPage { }
     export class Response extends RealResponse { }