fix: convert all getters to methods (#1621)

The patch converts all the getters in the codebase into the methods. For example, the `request.url` getter becomes the `request.url()` method. This is done in order to unify the API and make it more predictable. The general rule for all further changes would be: - there are no getters/fields exposed in the api - the only exceptions are "namespaces", e.g. `page.keyboard` Fixes #280. BREAKING CHANGE: This patch ditches getters and replaces them with methods throughout the API. The following methods were added instead of the fields: - dialog.type() - consoleMessage.args() - consoleMessage.text() - consoleMessage.type() - request.headers() - request.method() - request.postData() - request.resourceType() - request.url() - response.headers() - response.ok() - response.status() - response.url()
puppeteer · Dec 19, 2017 · b737373 · b737373
1 parent 10f3b92
commit b737373
Show file tree

Hide file tree

Showing 6 changed files with 230 additions and 148 deletions.
diff --git a/docs/api.md b/docs/api.md
@@ -113,11 +113,11 @@
   * [dialog.defaultValue()](#dialogdefaultvalue)
   * [dialog.dismiss()](#dialogdismiss)
   * [dialog.message()](#dialogmessage)
-  * [dialog.type](#dialogtype)
+  * [dialog.type()](#dialogtype)
 - [class: ConsoleMessage](#class-consolemessage)
-  * [consoleMessage.args](#consolemessageargs)
-  * [consoleMessage.text](#consolemessagetext)
-  * [consoleMessage.type](#consolemessagetype)
+  * [consoleMessage.args()](#consolemessageargs)
+  * [consoleMessage.text()](#consolemessagetext)
+  * [consoleMessage.type()](#consolemessagetype)
 - [class: Frame](#class-frame)
   * [frame.$(selector)](#frameselector)
   * [frame.$$(selector)](#frameselector)
@@ -173,22 +173,22 @@
   * [request.abort([errorCode])](#requestaborterrorcode)
   * [request.continue([overrides])](#requestcontinueoverrides)
   * [request.failure()](#requestfailure)
-  * [request.headers](#requestheaders)
-  * [request.method](#requestmethod)
-  * [request.postData](#requestpostdata)
-  * [request.resourceType](#requestresourcetype)
+  * [request.headers()](#requestheaders)
+  * [request.method()](#requestmethod)
+  * [request.postData()](#requestpostdata)
+  * [request.resourceType()](#requestresourcetype)
   * [request.respond(response)](#requestrespondresponse)
   * [request.response()](#requestresponse)
-  * [request.url](#requesturl)
+  * [request.url()](#requesturl)
 - [class: Response](#class-response)
   * [response.buffer()](#responsebuffer)
-  * [response.headers](#responseheaders)
+  * [response.headers()](#responseheaders)
   * [response.json()](#responsejson)
-  * [response.ok](#responseok)
+  * [response.ok()](#responseok)
   * [response.request()](#responserequest)
-  * [response.status](#responsestatus)
+  * [response.status()](#responsestatus)
   * [response.text()](#responsetext)
-  * [response.url](#responseurl)
+  * [response.url()](#responseurl)
 - [class: Target](#class-target)
   * [target.page()](#targetpage)
   * [target.type()](#targettype)
@@ -1396,23 +1396,21 @@ puppeteer.launch().then(async browser => {
 #### dialog.message()
 - returns: <[string]> A message displayed in the dialog.
 
-#### dialog.type
-- <[string]>
-
-Dialog's type, can be one of `alert`, `beforeunload`, `confirm` or `prompt`.
+#### dialog.type()
+- returns: <[string]> Dialog's type, can be one of `alert`, `beforeunload`, `confirm` or `prompt`.
 
 ### class: ConsoleMessage
 
 [ConsoleMessage] objects are dispatched by page via the ['console'](#event-console) event.
 
-#### consoleMessage.args
-- <[Array]<[JSHandle]>>
+#### consoleMessage.args()
+- returns: <[Array]<[JSHandle]>>
 
-#### consoleMessage.text
-- <[string]>
+#### consoleMessage.text()
+- returns: <[string]>
 
-#### consoleMessage.type
-- <[string]>
+#### consoleMessage.type()
+- returns: <[string]>
 
 One of the following values: `'log'`, `'debug'`, `'info'`, `'error'`, `'warning'`, `'dir'`, `'dirxml'`, `'table'`, `'trace'`, `'clear'`, `'startGroup'`, `'startGroupCollapsed'`, `'endGroup'`, `'assert'`, `'profile'`, `'profileEnd'`, `'count'`, `'timeEnd'`.
 
@@ -2010,21 +2008,17 @@ page.on('requestfailed', request => {
 });
 ```
 
-#### request.headers
-- <[Object]> An object with HTTP headers associated with the request. All header names are lower-case.
-
-#### request.method
-- <[string]>
-
-Contains the request's method (GET, POST, etc.)
+#### request.headers()
+- returns: <[Object]> An object with HTTP headers associated with the request. All header names are lower-case.
 
-#### request.postData
-- <[string]>
+#### request.method()
+- returns: <[string]> Request's method (GET, POST, etc.)
 
-Contains the request's post body, if any.
+#### request.postData()
+- returns: <[string]> Request's post body, if any.
 
-#### request.resourceType
-- <[string]>
+#### request.resourceType()
+- returns: <[string]>
 
 Contains the request's resource type as it was perceived by the rendering engine.
 ResourceType will be one of the following: `document`, `stylesheet`, `image`, `media`, `font`, `script`, `texttrack`, `xhr`, `fetch`, `eventsource`, `websocket`, `manifest`, `other`.
@@ -2060,10 +2054,8 @@ page.on('request', request => {
 #### request.response()
 - returns: <?[Response]> A matching [Response] object, or `null` if the response has not been received yet.
 
-#### request.url
-- <[string]>
-
-Contains the URL of the request.
+#### request.url()
+- returns: <[string]> URL of the request.
 
 ### class: Response
 
@@ -2072,32 +2064,32 @@ Contains the URL of the request.
 #### response.buffer()
 - returns: <Promise<[Buffer]>> Promise which resolves to a buffer with response body.
 
-#### response.headers
-- <[Object]> An object with HTTP headers associated with the response. All header names are lower-case.
+#### response.headers()
+- returns: <[Object]> An object with HTTP headers associated with the response. All header names are lower-case.
 
 #### response.json()
 - returns: <Promise<[Object]>> Promise which resolves to a JSON representation of response body.
 
 This method will throw if the response body is not parsable via `JSON.parse`.
 
-#### response.ok
-- <[boolean]>
+#### response.ok()
+- returns: <[boolean]>
 
 Contains a boolean stating whether the response was successful (status in the range 200-299) or not.
 
 #### response.request()
 - returns: <[Request]> A matching [Request] object.
 
-#### response.status
-- <[number]>
+#### response.status()
+- returns: <[number]>
 
 Contains the status code of the response (e.g., 200 for a success).
 
 #### response.text()
 - returns: <[Promise]<[string]>> Promise which resolves to a text representation of response body.
 
-#### response.url
-- <[string]>
+#### response.url()
+- returns: <[string]>
 
 Contains the URL of the response.
 

diff --git a/examples/block-images.js b/examples/block-images.js
@@ -24,7 +24,7 @@ const browser = await puppeteer.launch();
 const page = await browser.newPage();
 await page.setRequestInterception(true);
 page.on('request', request => {
-  if (request.resourceType === 'image')
+  if (request.resourceType() === 'image')
     request.abort();
   else
     request.continue();

diff --git a/lib/Dialog.js b/lib/Dialog.js
@@ -25,12 +25,19 @@ class Dialog {
    */
   constructor(client, type, message, defaultValue = '') {
     this._client = client;
-    this.type = type;
+    this._type = type;
     this._message = message;
     this._handled = false;
     this._defaultValue = defaultValue;
   }
 
+  /**
+   * @return {string}
+   */
+  type() {
+    return this._type;
+  }
+
   /**
    * @return {string}
    */

diff --git a/lib/NetworkManager.js b/lib/NetworkManager.js
@@ -294,13 +294,48 @@ class Request {
       this._completePromiseFulfill = fulfill;
     });
 
-    this.url = url;
-    this.resourceType = resourceType.toLowerCase();
-    this.method = payload.method;
-    this.postData = payload.postData;
-    this.headers = {};
+    this._url = url;
+    this._resourceType = resourceType.toLowerCase();
+    this._method = payload.method;
+    this._postData = payload.postData;
+    this._headers = {};
     for (const key of Object.keys(payload.headers))
-      this.headers[key.toLowerCase()] = payload.headers[key];
+      this._headers[key.toLowerCase()] = payload.headers[key];
+  }
+
+  /**
+   * @return {string}
+   */
+  url() {
+    return this._url;
+  }
+
+  /**
+   * @return {string}
+   */
+  resourceType() {
+    return this._resourceType;
+  }
+
+  /**
+   * @return {string}
+   */
+  method() {
+    return this._method;
+  }
+
+  /**
+   * @return {string}
+   */
+  postData() {
+    return this._postData;
+  }
+
+  /**
+   * @return {!Object}
+   */
+  headers() {
+    return this._headers;
   }
 
   /**
@@ -346,7 +381,7 @@ class Request {
    */
   async respond(response) {
     // Mocking responses for dataURL requests is not currently supported.
-    if (this.url.startsWith('data:'))
+    if (this._url.startsWith('data:'))
       return;
     console.assert(this._allowInterception, 'Request Interception is not enabled!');
     console.assert(!this._interceptionHandled, 'Request is already handled!');
@@ -438,12 +473,39 @@ class Response {
     this._request = request;
     this._contentPromise = null;
 
-    this.status = status;
-    this.ok = status >= 200 && status <= 299;
-    this.url = request.url;
-    this.headers = {};
+    this._status = status;
+    this._url = request.url();
+    this._headers = {};
     for (const key of Object.keys(headers))
-      this.headers[key.toLowerCase()] = headers[key];
+      this._headers[key.toLowerCase()] = headers[key];
+  }
+
+  /**
+   * @return {string}
+   */
+  url() {
+    return this._url;
+  }
+
+  /**
+   * @return {boolean}
+   */
+  ok() {
+    return this._status >= 200 && this._status <= 299;
+  }
+
+  /**
+   * @return {number}
+   */
+  status() {
+    return this._status;
+  }
+
+  /**
+   * @return {!Object}
+   */
+  headers() {
+    return this._headers;
   }
 
   /**

diff --git a/lib/Page.js b/lib/Page.js
@@ -457,7 +457,7 @@ class Page extends EventEmitter {
 
     const requests = new Map();
     const eventListeners = [
-      helper.addEventListener(this._networkManager, NetworkManager.Events.Request, request => requests.set(request.url, request))
+      helper.addEventListener(this._networkManager, NetworkManager.Events.Request, request => requests.set(request.url(), request))
     ];
 
     const mainFrame = this._frameManager.mainFrame();
@@ -513,7 +513,7 @@ class Page extends EventEmitter {
     const watcher = new NavigatorWatcher(this._frameManager, mainFrame, options);
 
     const responses = new Map();
-    const listener = helper.addEventListener(this._networkManager, NetworkManager.Events.Response, response => responses.set(response.url, response));
+    const listener = helper.addEventListener(this._networkManager, NetworkManager.Events.Response, response => responses.set(response.url(), response));
     const error = await watcher.navigationPromise();
     helper.removeEventListeners([listener]);
     if (error)
@@ -978,9 +978,30 @@ class ConsoleMessage {
    * @param {!Array<*>} args
    */
   constructor(type, text, args) {
-    this.type = type;
-    this.text = text;
-    this.args = args;
+    this._type = type;
+    this._text = text;
+    this._args = args;
+  }
+
+  /**
+   * @return {string}
+   */
+  type() {
+    return this._type;
+  }
+
+  /**
+   * @return {string}
+   */
+  text() {
+    return this._text;
+  }
+
+  /**
+   * @return {!Array<string>}
+   */
+  args() {
+    return this._args;
   }
 }