From c310f139a0dc2870c34eb5921994b44059c9ec5a Mon Sep 17 00:00:00 2001
From: Sam Verschueren <sam.verschueren@gmail.com>
Date: Thu, 12 Oct 2017 10:26:44 +0200
Subject: [PATCH] feat(Frame): Support options in `addScriptTag` and
 `addStyleTag` (#996)

This patch:
- deprecates injectFile as it was confused with the addScriptTag
- accepts an options object in addScriptTag which supports properties url, path and content.
- accepts an options object in addStyleTag which supports properties url, path and content.

Fixes #949.

BREAKING CHANGE:
- the addStyleTag/addScriptTag have changed;
- the injectFile was removed in favor of (addStyleTag({path:}).
---
 docs/api.md         | 68 ++++++++++++++++++-------------------
 lib/FrameManager.js | 79 ++++++++++++++++++++++++++++++------------
 lib/Page.js         | 19 ++++-------
 test/test.js        | 83 ++++++++++++++++++++++++++++++++++-----------
 4 files changed, 161 insertions(+), 88 deletions(-)

diff --git a/docs/api.md b/docs/api.md
index c0a321fa8d105..5ec881f5f755e 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -35,8 +35,8 @@
     + [page.$$(selector)](#pageselector)
     + [page.$$eval(selector, pageFunction[, ...args])](#pageevalselector-pagefunction-args)
     + [page.$eval(selector, pageFunction[, ...args])](#pageevalselector-pagefunction-args)
-    + [page.addScriptTag(url)](#pageaddscripttagurl)
-    + [page.addStyleTag(url)](#pageaddstyletagurl)
+    + [page.addScriptTag(options)](#pageaddscripttagoptions)
+    + [page.addStyleTag(options)](#pageaddstyletagoptions)
     + [page.authenticate(credentials)](#pageauthenticatecredentials)
     + [page.click(selector[, options])](#pageclickselector-options)
     + [page.close()](#pageclose)
@@ -56,7 +56,6 @@
     + [page.goForward(options)](#pagegoforwardoptions)
     + [page.goto(url, options)](#pagegotourl-options)
     + [page.hover(selector)](#pagehoverselector)
-    + [page.injectFile(filePath)](#pageinjectfilefilepath)
     + [page.keyboard](#pagekeyboard)
     + [page.mainFrame()](#pagemainframe)
     + [page.mouse](#pagemouse)
@@ -114,12 +113,11 @@
     + [frame.$$(selector)](#frameselector)
     + [frame.$$eval(selector, pageFunction[, ...args])](#frameevalselector-pagefunction-args)
     + [frame.$eval(selector, pageFunction[, ...args])](#frameevalselector-pagefunction-args)
-    + [frame.addScriptTag(url)](#frameaddscripttagurl)
-    + [frame.addStyleTag(url)](#frameaddstyletagurl)
+    + [frame.addScriptTag(options)](#frameaddscripttagoptions)
+    + [frame.addStyleTag(options)](#frameaddstyletagoptions)
     + [frame.childFrames()](#framechildframes)
     + [frame.evaluate(pageFunction, ...args)](#frameevaluatepagefunction-args)
     + [frame.executionContext()](#frameexecutioncontext)
-    + [frame.injectFile(filePath)](#frameinjectfilefilepath)
     + [frame.isDetached()](#frameisdetached)
     + [frame.name()](#framename)
     + [frame.parentFrame()](#frameparentframe)
@@ -418,21 +416,27 @@ const html = await page.$eval('.main-container', e => e.outerHTML);
 
 Shortcut for [page.mainFrame().$eval(selector, pageFunction)](#frameevalselector-pagefunction-args).
 
-#### page.addScriptTag(url)
-- `url` <[string]> Url of the `<script>` tag
-- returns: <[Promise]> which resolves when the script's onload fires.
+#### page.addScriptTag(options)
+- `options` <[Object]>
+  - `url` <[string]> Url of a script to be added.
+  - `path` <[string]> Path to the JavaScript file to be injected into frame. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
+  - `content` <[string]> Raw JavaScript content to be injected into frame.
+- returns: <[Promise]> which resolves when the script's onload fires or when the script content was injected into frame.
 
-Adds a `<script>` tag into the page with the desired url. Alternatively, a local JavaScript file can be injected via [`page.injectFile`](#pageinjectfilefilepath) method.
+Adds a `<script>` tag into the page with the desired url or content.
 
-Shortcut for [page.mainFrame().addScriptTag(url)](#frameaddscripttagurl).
+Shortcut for [page.mainFrame().addScriptTag(options)](#frameaddscripttagoptions).
 
-#### page.addStyleTag(url)
-- `url` <[string]> Url of the `<link>` tag
-- returns: <[Promise]> which resolves when the stylesheet's onload fires.
+#### page.addStyleTag(options)
+- `options` <[Object]>
+  - `url` <[string]> Url of the `<link>` tag.
+  - `path` <[string]> Path to the CSS file to be injected into frame. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
+  - `content` <[string]> Raw CSS content to be injected into frame.
+- returns: <[Promise]> which resolves when the stylesheet's onload fires or when the CSS content was injected into frame.
 
-Adds a `<link rel="stylesheet">` tag into the page with the desired url.
+Adds a `<link rel="stylesheet">` tag into the page with the desired url or a `<style type="text/css">` tag with the content.
 
-Shortcut for [page.mainFrame().addStyleTag(url)](#frameaddstyletagurl).
+Shortcut for [page.mainFrame().addStyleTag(options)](#frameaddstyletagoptions).
 
 #### page.authenticate(credentials)
 - `credentials` <[Object]>
@@ -740,12 +744,6 @@ The `page.goto` will throw an error if:
 This method fetches an element with `selector`, scrolls it into view if needed, and then uses [page.mouse](#pagemouse) to hover over the center of the element.
 If there's no element matching `selector`, the method throws an error.
 
-#### page.injectFile(filePath)
-- `filePath` <[string]> Path to the JavaScript file to be injected into frame. If `filePath` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
-- returns: <[Promise]> Promise which resolves when file gets successfully evaluated in frame.
-
-Shortcut for [page.mainFrame().injectFile(filePath)](#frameinjectfilefilepath).
-
 #### page.keyboard
 
 - returns: <[Keyboard]>
@@ -1334,17 +1332,23 @@ const preloadHref = await frame.$eval('link[rel=preload]', el => el.href);
 const html = await frame.$eval('.main-container', e => e.outerHTML);
 ```
 
-#### frame.addScriptTag(url)
-- `url` <[string]> Url of a script to be added
-- returns: <[Promise]> Promise which resolves as the script gets added and loads.
+#### frame.addScriptTag(options)
+- `options` <[Object]>
+  - `url` <[string]> Url of a script to be added.
+  - `path` <[string]> Path to the JavaScript file to be injected into frame. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
+  - `content` <[string]> Raw JavaScript content to be injected into frame.
+- returns: <[Promise]> which resolves when the script's onload fires or when the script content was injected into frame.
 
-Adds a `<script>` tag to the frame with the desired url. Alternatively, JavaScript can be injected to the frame via [`frame.injectFile`](#frameinjectfilefilepath) method.
+Adds a `<script>` tag into the page with the desired url or content.
 
-#### frame.addStyleTag(url)
-- `url` <[string]> Url of a stylesheet to be added
-- returns: <[Promise]> Promise which resolves when the script gets added and loads.
+#### frame.addStyleTag(options)
+- `options` <[Object]>
+  - `url` <[string]> Url of the `<link>` tag.
+  - `path` <[string]> Path to the CSS file to be injected into frame. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
+  - `content` <[string]> Raw CSS content to be injected into frame.
+- returns: <[Promise]> which resolves when the stylesheet's onload fires or when the CSS content was injected into frame.
 
-Adds a `<link rel="stylesheet">` tag to the frame with the desired url.
+Adds a `<link rel="stylesheet">` tag into the page with the desired url or a `<style type="text/css">` tag with the content.
 
 #### frame.childFrames()
 - returns: <[Array]<[Frame]>>
@@ -1381,10 +1385,6 @@ await bodyHandle.dispose();
 #### frame.executionContext()
 - returns: <[ExecutionContext]> Execution context associated with this frame.
 
-#### frame.injectFile(filePath)
-- `filePath` <[string]> Path to the JavaScript file to be injected into frame. If `filePath` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
-- returns: <[Promise]> Promise which resolves when file gets successfully evaluated in frame.
-
 #### frame.isDetached()
 - returns: <[boolean]>
 
diff --git a/lib/FrameManager.js b/lib/FrameManager.js
index b61e30d181a17..6e895e5f5fa2b 100644
--- a/lib/FrameManager.js
+++ b/lib/FrameManager.js
@@ -20,6 +20,8 @@ const {helper} = require('./helper');
 const {ExecutionContext, JSHandle} = require('./ExecutionContext');
 const ElementHandle = require('./ElementHandle');
 
+const readFileAsync = helper.promisify(fs.readFile);
+
 class FrameManager extends EventEmitter {
   /**
    * @param {!Puppeteer.Session} client
@@ -303,48 +305,71 @@ class Frame {
   }
 
   /**
-   * @param {string} filePath
-   * @return {!Promise<*>}
+   * @param {Object} options
+   * @return {!Promise}
    */
-  async injectFile(filePath) {
-    let contents = await new Promise((resolve, reject) => {
-      fs.readFile(filePath, 'utf8', (err, data) => {
-        if (err) return reject(err);
-        resolve(data);
-      });
-    });
-    contents += `//# sourceURL=` + filePath.replace(/\n/g,'');
-    return this.evaluate(contents);
-  }
+  async addScriptTag(options) {
+    if (typeof options.url === 'string')
+      return this.evaluate(addScriptUrl, options.url);
 
-  /**
-   * @param {string} url
-   */
-  async addScriptTag(url) {
-    return this.evaluate(addScriptTag, url);
+    if (typeof options.path === 'string') {
+      let contents = await readFileAsync(options.path, 'utf8');
+      contents += '//# sourceURL=' + options.path.replace(/\n/g, '');
+
+      return this.evaluate(addScriptContent, contents);
+    }
+
+    if (typeof options.content === 'string')
+      return this.evaluate(addScriptContent, options.content);
+
+    throw new Error('Provide an object with a `url`, `path` or `content` property');
 
     /**
      * @param {string} url
      */
-    function addScriptTag(url) {
+    function addScriptUrl(url) {
       const script = document.createElement('script');
       script.src = url;
       const promise = new Promise(x => script.onload = x);
       document.head.appendChild(script);
       return promise;
     }
+
+    /**
+     * @param {string} content
+     */
+    function addScriptContent(content) {
+      const script = document.createElement('script');
+      script.type = 'text/javascript';
+      script.text = content;
+      document.head.appendChild(script);
+    }
   }
 
   /**
-   * @param {string} url
+   * @param {Object} options
+   * @return {!Promise}
    */
-  async addStyleTag(url) {
-    return this.evaluate(addStyleTag, url);
+  async addStyleTag(options) {
+    if (typeof options.url === 'string')
+      return this.evaluate(addStyleUrl, options.url);
+
+    if (typeof options.path === 'string') {
+      let contents = await readFileAsync(options.path, 'utf8');
+      contents += '/*# sourceURL=' + options.path.replace(/\n/g, '') + '*/';
+
+      return this.evaluate(addStyleContent, contents);
+    }
+
+    if (typeof options.content === 'string')
+      return this.evaluate(addStyleContent, options.content);
+
+    throw new Error('Provide an object with a `url`, `path` or `content` property');
 
     /**
      * @param {string} url
      */
-    function addStyleTag(url) {
+    function addStyleUrl(url) {
       const link = document.createElement('link');
       link.rel = 'stylesheet';
       link.href = url;
@@ -352,6 +377,16 @@ class Frame {
       document.head.appendChild(link);
       return promise;
     }
+
+    /**
+     * @param {string} content
+     */
+    function addStyleContent(content) {
+      const style = document.createElement('style');
+      style.type = 'text/css';
+      style.appendChild(document.createTextNode(content));
+      document.head.appendChild(style);
+    }
   }
 
   /**
diff --git a/lib/Page.js b/lib/Page.js
index ee933e1498c50..e054368442245 100644
--- a/lib/Page.js
+++ b/lib/Page.js
@@ -254,24 +254,17 @@ class Page extends EventEmitter {
   }
 
   /**
-   * @param {string} url
-   */
-  async addScriptTag(url) {
-    return this.mainFrame().addScriptTag(url);
-  }
-
-  /**
-   * @param {string} url
+   * @param {Object} options
    */
-  async addStyleTag(url) {
-    return this.mainFrame().addStyleTag(url);
+  async addScriptTag(options) {
+    return this.mainFrame().addScriptTag(options);
   }
 
   /**
-   * @param {string} filePath
+   * @param {Object} options
    */
-  async injectFile(filePath) {
-    return this.mainFrame().injectFile(filePath);
+  async addStyleTag(options) {
+    return this.mainFrame().addStyleTag(options);
   }
 
   /**
diff --git a/test/test.js b/test/test.js
index e45598d4896f7..46aa84432a6f1 100644
--- a/test/test.js
+++ b/test/test.js
@@ -456,21 +456,6 @@ describe('Page', function() {
     }));
   });
 
-  describe('Page.injectFile', function() {
-    it('should work', SX(async function() {
-      const helloPath = path.join(__dirname, 'assets', 'injectedfile.js');
-      await page.injectFile(helloPath);
-      const result = await page.evaluate(() => __injected);
-      expect(result).toBe(42);
-    }));
-    it('should include sourcemap', SX(async function() {
-      const helloPath = path.join(__dirname, 'assets', 'injectedfile.js');
-      await page.injectFile(helloPath);
-      const result = await page.evaluate(() => __injectedError.stack);
-      expect(result).toContain(path.join('assets', 'injectedfile.js'));
-    }));
-  });
-
   describe('Frame.context', function() {
     const FrameUtils = require('./frame-utils');
     it('should work', SX(async function() {
@@ -2066,19 +2051,79 @@ describe('Page', function() {
   });
 
   describe('Page.addScriptTag', function() {
-    it('should work', SX(async function() {
+    it('should throw an error if no options are provided', SX(async function() {
+      let error = null;
+      try {
+        await page.addScriptTag('/injectedfile.js');
+      } catch (e) {
+        error = e;
+      }
+      expect(error.message).toBe('Provide an object with a `url`, `path` or `content` property');
+    }));
+
+    it('should work with a url', SX(async function() {
       await page.goto(EMPTY_PAGE);
-      await page.addScriptTag('/injectedfile.js');
+      await page.addScriptTag({ url: '/injectedfile.js' });
       expect(await page.evaluate(() => __injected)).toBe(42);
     }));
+
+    it('should work with a path', SX(async function() {
+      await page.goto(EMPTY_PAGE);
+      await page.addScriptTag({ path: path.join(__dirname, 'assets/injectedfile.js') });
+      expect(await page.evaluate(() => __injected)).toBe(42);
+    }));
+
+    it('should include sourcemap when path is provided', SX(async function() {
+      await page.goto(EMPTY_PAGE);
+      await page.addScriptTag({ path: path.join(__dirname, 'assets/injectedfile.js') });
+      const result = await page.evaluate(() => __injectedError.stack);
+      expect(result).toContain(path.join('assets', 'injectedfile.js'));
+    }));
+
+    it('should work with content', SX(async function() {
+      await page.goto(EMPTY_PAGE);
+      await page.addScriptTag({ content: 'window.__injected = 35;' });
+      expect(await page.evaluate(() => __injected)).toBe(35);
+    }));
   });
 
   describe('Page.addStyleTag', function() {
-    it('should work', SX(async function() {
+    it('should throw an error if no options are provided', SX(async function() {
+      let error = null;
+      try {
+        await page.addStyleTag('/injectedstyle.css');
+      } catch (e) {
+        error = e;
+      }
+      expect(error.message).toBe('Provide an object with a `url`, `path` or `content` property');
+    }));
+
+    it('should work with a url', SX(async function() {
+      await page.goto(EMPTY_PAGE);
+      await page.addStyleTag({ url: '/injectedstyle.css' });
+      expect(await page.evaluate(`window.getComputedStyle(document.querySelector('body')).getPropertyValue('background-color')`)).toBe('rgb(255, 0, 0)');
+    }));
+
+    it('should work with a path', SX(async function() {
       await page.goto(EMPTY_PAGE);
-      await page.addStyleTag('/injectedstyle.css');
+      await page.addStyleTag({ path: path.join(__dirname, 'assets/injectedstyle.css') });
       expect(await page.evaluate(`window.getComputedStyle(document.querySelector('body')).getPropertyValue('background-color')`)).toBe('rgb(255, 0, 0)');
     }));
+
+    it('should include sourcemap when path is provided', SX(async function() {
+      await page.goto(EMPTY_PAGE);
+      await page.addStyleTag({ path: path.join(__dirname, 'assets/injectedstyle.css') });
+      const styleHandle = await page.$('style');
+      const styleContent = await page.evaluate(style => style.innerHTML, styleHandle);
+      expect(styleContent).toContain(path.join('assets', 'injectedstyle.css'));
+      styleHandle.dispose();
+    }));
+
+    it('should work with content', SX(async function() {
+      await page.goto(EMPTY_PAGE);
+      await page.addStyleTag({ content: 'body { background-color: green; }' });
+      expect(await page.evaluate(`window.getComputedStyle(document.querySelector('body')).getPropertyValue('background-color')`)).toBe('rgb(0, 128, 0)');
+    }));
   });
 
   describe('Page.url', function() {