refactor: avoid dynamic requires in lib/ folder (puppeteer#3208)

This patch removes all dynamic requires in Puppeteer. This should make it much simpler to bundle puppeteer/puppeteer-core packages. We used dynamic requires in a few places in lib/: - BrowserFetcher was choosing between `http` and `https` based on some runtime value. This was easy to fix with explicit `require`. - BrowserFetcher and Launcher needed to know project root to store chromium revisions and to read package name and chromium revision from package.json. (projectRoot value would be different in node6). Instead of doing a backwards logic to infer these variables, we now pass them directly from `//index.js`. With this patch, I was able to bundle Puppeteer using browserify and the following config in `package.json`: ```json "browser": { "./lib/BrowserFetcher.js": false, "ws": "./lib/BrowserWebSocket", "fs": false, "child_process": false, "rimraf": false, "readline": false } ``` (where `lib/BrowserWebSocket.js` is a courtesy of @Janpot from puppeteer#2374) And command: ```sh $ browserify -r puppeteer:./index.js > ppweb.js ``` References puppeteer#2119
tangyu · Sep 6, 2018 · f230722 · f230722
1 parent d54c7ed
commit f230722
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 74 deletions.
diff --git a/index.js b/index.js
@@ -22,7 +22,9 @@ try {
 }
 
 // If node does not support async await, use the compiled version.
-if (asyncawait)
-  module.exports = require('./lib/Puppeteer');
-else
-  module.exports = require('./node6/lib/Puppeteer');
+const Puppeteer = asyncawait ? require('./lib/Puppeteer') : require('./node6/lib/Puppeteer');
+const packageJson = require('./package.json');
+const preferredRevision = packageJson.puppeteer.chromium_revision;
+const isPuppeteerCore = packageJson.name === 'puppeteer-core';
+
+module.exports = new Puppeteer(__dirname, preferredRevision, isPuppeteerCore);
diff --git a/lib/BrowserFetcher.js b/lib/BrowserFetcher.js
@@ -49,10 +49,11 @@ function existsAsync(filePath) {
 
 class BrowserFetcher {
   /**
+   * @param {string} projectRoot
    * @param {!BrowserFetcher.Options=} options
    */
-  constructor(options = {}) {
-    this._downloadsFolder = options.path || path.join(helper.projectRoot(), '.local-chromium');
+  constructor(projectRoot, options = {}) {
+    this._downloadsFolder = options.path || path.join(projectRoot, '.local-chromium');
     this._downloadHost = options.host || DEFAULT_DOWNLOAD_HOST;
     this._platform = options.platform || '';
     if (!this._platform) {
@@ -256,13 +257,15 @@ function httpRequest(url, method, response) {
     options.rejectUnauthorized = false;
   }
 
-  const driver = options.protocol === 'https:' ? 'https' : 'http';
-  const request = require(driver).request(options, res => {
+  const requestCallback = res => {
     if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location)
       httpRequest(res.headers.location, method, response);
     else
       response(res);
-  });
+  };
+  const request = options.protocol === 'https:' ?
+    require('https').request(options, requestCallback) :
+    require('http').request(options, requestCallback);
   request.end();
   return request;
 }

diff --git a/lib/Launcher.js b/lib/Launcher.js
@@ -23,7 +23,6 @@ const {Browser} = require('./Browser');
 const readline = require('readline');
 const fs = require('fs');
 const {helper, debugError} = require('./helper');
-const ChromiumRevision = require(path.join(helper.projectRoot(), 'package.json')).puppeteer.chromium_revision;
 const {TimeoutError} = require('./Errors');
 
 const mkdtempAsync = helper.promisify(fs.mkdtemp);
@@ -55,11 +54,22 @@ const DEFAULT_ARGS = [
 ];
 
 class Launcher {
+  /**
+   * @param {string} projectRoot
+   * @param {string} preferredRevision
+   * @param {boolean} isPuppeteerCore
+   */
+  constructor(projectRoot, preferredRevision, isPuppeteerCore) {
+    this._projectRoot = projectRoot;
+    this._preferredRevision = preferredRevision;
+    this._isPuppeteerCore = isPuppeteerCore;
+  }
+
   /**
    * @param {!(LaunchOptions & ChromeArgOptions & BrowserOptions)=} options
    * @return {!Promise<!Browser>}
    */
-  static async launch(options = {}) {
+  async launch(options = {}) {
     const {
       ignoreDefaultArgs = false,
       args = [],
@@ -95,7 +105,7 @@ class Launcher {
 
     let chromeExecutable = executablePath;
     if (!executablePath) {
-      const {missingText, executablePath} = resolveExecutablePath();
+      const {missingText, executablePath} = this._resolveExecutablePath();
       if (missingText)
         throw new Error(missingText);
       chromeExecutable = executablePath;
@@ -147,7 +157,7 @@ class Launcher {
     let connection = null;
     try {
       if (!usePipe) {
-        const browserWSEndpoint = await waitForWSEndpoint(chromeProcess, timeout);
+        const browserWSEndpoint = await waitForWSEndpoint(chromeProcess, timeout, this._preferredRevision);
         connection = await Connection.createForWebSocket(browserWSEndpoint, slowMo);
       } else {
         connection = Connection.createForPipe(/** @type {!NodeJS.WritableStream} */(chromeProcess.stdio[3]), /** @type {!NodeJS.ReadableStream} */ (chromeProcess.stdio[4]), slowMo);
@@ -221,7 +231,7 @@ class Launcher {
    * @param {!ChromeArgOptions=} options
    * @return {!Array<string>}
    */
-  static defaultArgs(options = {}) {
+  defaultArgs(options = {}) {
     const {
       devtools = false,
       headless = !devtools,
@@ -251,15 +261,15 @@ class Launcher {
   /**
    * @return {string}
    */
-  static executablePath() {
-    return resolveExecutablePath().executablePath;
+  executablePath() {
+    return this._resolveExecutablePath().executablePath;
   }
 
   /**
    * @param {!(BrowserOptions & {browserWSEndpoint: string})} options
    * @return {!Promise<!Browser>}
    */
-  static async connect(options) {
+  async connect(options) {
     const {
       browserWSEndpoint,
       ignoreHTTPSErrors = false,
@@ -270,14 +280,40 @@ class Launcher {
     const {browserContextIds} = await connection.send('Target.getBrowserContexts');
     return Browser.create(connection, browserContextIds, ignoreHTTPSErrors, defaultViewport, null, () => connection.send('Browser.close').catch(debugError));
   }
+
+  /**
+   * @return {{executablePath: string, missingText: ?string}}
+   */
+  _resolveExecutablePath() {
+    const browserFetcher = new BrowserFetcher(this._projectRoot);
+    // puppeteer-core doesn't take into account PUPPETEER_* env variables.
+    if (!this._isPuppeteerCore) {
+      const executablePath = process.env['PUPPETEER_EXECUTABLE_PATH'];
+      if (executablePath) {
+        const missingText = !fs.existsSync(executablePath) ? 'Tried to use PUPPETEER_EXECUTABLE_PATH env variable to launch browser but did not find any executable at: ' + executablePath : null;
+        return { executablePath, missingText };
+      }
+      const revision = process.env['PUPPETEER_CHROMIUM_REVISION'];
+      if (revision) {
+        const revisionInfo = browserFetcher.revisionInfo(revision);
+        const missingText = !revisionInfo.local ? 'Tried to use PUPPETEER_CHROMIUM_REVISION env variable to launch browser but did not find executable at: ' + revisionInfo.executablePath : null;
+        return {executablePath: revisionInfo.executablePath, missingText};
+      }
+    }
+    const revisionInfo = browserFetcher.revisionInfo(this._preferredRevision);
+    const missingText = !revisionInfo.local ? `Chromium revision is not downloaded. Run "npm install" or "yarn install"` : null;
+    return {executablePath: revisionInfo.executablePath, missingText};
+  }
+
 }
 
 /**
  * @param {!Puppeteer.ChildProcess} chromeProcess
  * @param {number} timeout
+ * @param {string} preferredRevision
  * @return {!Promise<string>}
  */
-function waitForWSEndpoint(chromeProcess, timeout) {
+function waitForWSEndpoint(chromeProcess, timeout, preferredRevision) {
   return new Promise((resolve, reject) => {
     const rl = readline.createInterface({ input: chromeProcess.stderr });
     let stderr = '';
@@ -305,7 +341,7 @@ function waitForWSEndpoint(chromeProcess, timeout) {
 
     function onTimeout() {
       cleanup();
-      reject(new TimeoutError(`Timed out after ${timeout} ms while trying to connect to Chrome! The only Chrome revision guaranteed to work is r${ChromiumRevision}`));
+      reject(new TimeoutError(`Timed out after ${timeout} ms while trying to connect to Chrome! The only Chrome revision guaranteed to work is r${preferredRevision}`));
     }
 
     /**
@@ -328,27 +364,6 @@ function waitForWSEndpoint(chromeProcess, timeout) {
   });
 }
 
-/**
- * @return {{executablePath: string, missingText: ?string}}
- */
-function resolveExecutablePath() {
-  const executablePath = helper.getEnv('PUPPETEER_EXECUTABLE_PATH');
-  if (executablePath) {
-    const missingText = !fs.existsSync(executablePath) ? 'Tried to use PUPPETEER_EXECUTABLE_PATH env variable to launch browser but did not find any executable at: ' + executablePath : null;
-    return { executablePath, missingText };
-  }
-  const browserFetcher = new BrowserFetcher();
-  const revision = helper.getEnv('PUPPETEER_CHROMIUM_REVISION');
-  if (revision) {
-    const revisionInfo = browserFetcher.revisionInfo(revision);
-    const missingText = !revisionInfo.local ? 'Tried to use PUPPETEER_CHROMIUM_REVISION env variable to launch browser but did not find executable at: ' + revisionInfo.executablePath : null;
-    return {executablePath: revisionInfo.executablePath, missingText};
-  }
-  const revisionInfo = browserFetcher.revisionInfo(ChromiumRevision);
-  const missingText = !revisionInfo.local ? `Chromium revision is not downloaded. Run "npm install" or "yarn install"` : null;
-  return {executablePath: revisionInfo.executablePath, missingText};
-}
-
 /**
  * @typedef {Object} ChromeArgOptions
  * @property {boolean=} headless

diff --git a/lib/Puppeteer.js b/lib/Puppeteer.js
@@ -18,42 +18,52 @@ const Launcher = require('./Launcher');
 const BrowserFetcher = require('./BrowserFetcher');
 
 module.exports = class {
+  /**
+   * @param {string} projectRoot
+   * @param {string} preferredRevision
+   * @param {boolean} isPuppeteerCore
+   */
+  constructor(projectRoot, preferredRevision, isPuppeteerCore) {
+    this._projectRoot = projectRoot;
+    this._launcher = new Launcher(projectRoot, preferredRevision, isPuppeteerCore);
+  }
+
   /**
    * @param {!Object=} options
    * @return {!Promise<!Puppeteer.Browser>}
    */
-  static launch(options) {
-    return Launcher.launch(options);
+  launch(options) {
+    return this._launcher.launch(options);
   }
 
   /**
    * @param {{browserWSEndpoint: string, ignoreHTTPSErrors: boolean}} options
    * @return {!Promise<!Puppeteer.Browser>}
    */
-  static connect(options) {
-    return Launcher.connect(options);
+  connect(options) {
+    return this._launcher.connect(options);
   }
 
   /**
    * @return {string}
    */
-  static executablePath() {
-    return Launcher.executablePath();
+  executablePath() {
+    return this._launcher.executablePath();
   }
 
   /**
    * @return {!Array<string>}
    */
-  static defaultArgs(options) {
-    return Launcher.defaultArgs(options);
+  defaultArgs(options) {
+    return this._launcher.defaultArgs(options);
   }
 
   /**
    * @param {!Object=} options
    * @return {!BrowserFetcher}
    */
-  static createBrowserFetcher(options) {
-    return new BrowserFetcher(options);
+  createBrowserFetcher(options) {
+    return new BrowserFetcher(this._projectRoot, options);
   }
 };
 

diff --git a/lib/helper.js b/lib/helper.js
@@ -13,18 +13,12 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-const fs = require('fs');
-const path = require('path');
 const {TimeoutError} = require('./Errors');
 
 const debugError = require('debug')(`puppeteer:error`);
 /** @type {?Map<string, boolean>} */
 let apiCoverage = null;
 
-// Project root will be different for node6-transpiled code.
-const projectRoot = fs.existsSync(path.join(__dirname, '..', 'package.json')) ? path.join(__dirname, '..') : path.join(__dirname, '..', '..');
-const packageJson = require(path.join(projectRoot, 'package.json'));
-
 class Helper {
   /**
    * @param {Function|string} fun
@@ -49,24 +43,6 @@ class Helper {
     }
   }
 
-  /**
-   * @param {string} name
-   * @return {(string|undefined)}
-   */
-  static getEnv(name) {
-    // Ignore all PUPPETEER_* env variables in puppeteer-core package.
-    if (name.startsWith('PUPPETEER_') && packageJson.name === 'puppeteer-core')
-      return undefined;
-    return process.env[name];
-  }
-
-  /**
-   * @return {string}
-   */
-  static projectRoot() {
-    return projectRoot;
-  }
-
   /**
    * @param {!Protocol.Runtime.ExceptionDetails} exceptionDetails
    * @return {string}