Depracate adloader loadscript fn (#4228)

* test commit

* add callback support to loadExternalScript()

* pass moduleCode for renderers

* test cases for loadExternalScript()

* store callbacks for cached url

* fully remove adloader()

* call callback() from stub

* remove and update function name

CONTRIBUTING.md

@@ -48,7 +48,7 @@ When you are adding code to Prebid.js, or modifying code that isn't covered by a
   - _Assert_: check that the expected results have occurred
     - e.g., use Chai assertions to check that the expected output is equal to the actual output
 - Test the public interface, not the internal implementation
-- If you need to check `adloader.loadScript` in a test, use a `stub` rather than a `spy`. `spy`s trigger a network call which can result in a `script error` and cause unrelated unit tests to fail. `stub`s will let you gather information about the `adloader.loadScript` call without affecting external resources
+- If you need to check `adloader.loadExternalScript` in a test, use a `stub` rather than a `spy`. `spy`s trigger a network call which can result in a `script error` and cause unrelated unit tests to fail. `stub`s will let you gather information about the `adloader.loadExternalScript` call without affecting external resources
 - When writing tests you may use ES2015 syntax if desired
 
 ### Test Examples

src/Renderer.js

@@ -1,6 +1,7 @@
-import { loadScript } from './adloader';
+import { loadExternalScript } from './adloader';
 import * as utils from './utils';
 import find from 'core-js/library/fn/array/find';
+const moduleCode = 'outstream';
 
 /**
  * @typedef {object} Renderer
@@ -18,7 +19,7 @@ export function Renderer(options) {
   this.id = id;
 
   // a renderer may push to the command queue to delay rendering until the
-  // render function is loaded by loadScript, at which point the the command
+  // render function is loaded by loadExternalScript, at which point the the command
   // queue will be processed
   this.loaded = loaded;
   this.cmd = [];
@@ -38,7 +39,7 @@ export function Renderer(options) {
 
   if (!isRendererDefinedOnAdUnit(adUnitCode)) {
     // we expect to load a renderer url once only so cache the request to load script
-    loadScript(url, this.callback, true);
+    loadExternalScript(url, moduleCode, this.callback);
   } else {
     utils.logWarn(`External Js not loaded by Renderer since renderer url and callback is already defined on adUnit ${adUnitCode}`);
   }
@@ -70,7 +71,7 @@ Renderer.prototype.handleVideoEvent = function({ id, eventName }) {
 
 /*
  * Calls functions that were pushed to the command queue before the
- * renderer was loaded by `loadScript`
+ * renderer was loaded by `loadExternalScript`
  */
 Renderer.prototype.process = function() {
   while (this.cmd.length > 0) {

src/adloader.js

@@ -2,89 +2,60 @@ import includes from 'core-js/library/fn/array/includes';
 import * as utils from './utils';
 
 const _requestCache = {};
-const _vendorWhitelist = [
+// The below list contains modules or vendors whom Prebid allows to load external JS.
+const _approvedLoadExternalJSList = [
   'criteo',
+  'outstream'
 ]
 
 /**
  * Loads external javascript. Can only be used if external JS is approved by Prebid. See https://github.com/prebid/prebid-js-external-js-template#policy
  * Each unique URL will be loaded at most 1 time.
  * @param {string} url the url to load
  * @param {string} moduleCode bidderCode or module code of the module requesting this resource
+ * @param {function} callback callback function to be called after the script is loaded.
  */
-export function loadExternalScript(url, moduleCode) {
+export function loadExternalScript(url, moduleCode, callback) {
   if (!moduleCode || !url) {
     utils.logError('cannot load external script without url and moduleCode');
     return;
   }
-  if (!includes(_vendorWhitelist, moduleCode)) {
+  if (!includes(_approvedLoadExternalJSList, moduleCode)) {
     utils.logError(`${moduleCode} not whitelisted for loading external JavaScript`);
     return;
   }
   // only load each asset once
   if (_requestCache[url]) {
+    if (callback && typeof callback === 'function') {
+      if (_requestCache[url].loaded) {
+        // invokeCallbacks immediately
+        callback();
+      } else {
+        // queue the callback
+        _requestCache[url].callbacks.push(callback);
+      }
+    }
     return;
   }
-
-  utils.logWarn(`module ${moduleCode} is loading external JavaScript`);
-  const script = document.createElement('script');
-  script.type = 'text/javascript';
-  script.async = true;
-  script.src = url;
-
-  utils.insertElement(script);
-  _requestCache[url] = true;
-};
-
-/**
- *
- * @deprecated
- * Do not use this function. Will be removed in the next release. If external resources are required, use #loadExternalScript instead.
- */
-export function loadScript(tagSrc, callback, cacheRequest) {
-  // var noop = () => {};
-  //
-  // callback = callback || noop;
-  if (!tagSrc) {
-    utils.logError('Error attempting to request empty URL', 'adloader.js:loadScript');
-    return;
+  _requestCache[url] = {
+    loaded: false,
+    callbacks: []
+  };
+  if (callback && typeof callback === 'function') {
+    _requestCache[url].callbacks.push(callback);
   }
 
-  if (cacheRequest) {
-    if (_requestCache[tagSrc]) {
-      if (callback && typeof callback === 'function') {
-        if (_requestCache[tagSrc].loaded) {
-          // invokeCallbacks immediately
-          callback();
-        } else {
-          // queue the callback
-          _requestCache[tagSrc].callbacks.push(callback);
-        }
-      }
-    } else {
-      _requestCache[tagSrc] = {
-        loaded: false,
-        callbacks: []
-      };
-      if (callback && typeof callback === 'function') {
-        _requestCache[tagSrc].callbacks.push(callback);
+  utils.logWarn(`module ${moduleCode} is loading external JavaScript`);
+  requestResource(url, function () {
+    _requestCache[url].loaded = true;
+    try {
+      for (let i = 0; i < _requestCache[url].callbacks.length; i++) {
+        _requestCache[url].callbacks[i]();
       }
-
-      requestResource(tagSrc, function () {
-        _requestCache[tagSrc].loaded = true;
-        try {
-          for (let i = 0; i < _requestCache[tagSrc].callbacks.length; i++) {
-            _requestCache[tagSrc].callbacks[i]();
-          }
-        } catch (e) {
-          utils.logError('Error executing callback', 'adloader.js:loadScript', e);
-        }
-      });
+    } catch (e) {
+      utils.logError('Error executing callback', 'adloader.js:loadExternalScript', e);
     }
-  } else {
-    // trigger one time request
-    requestResource(tagSrc, callback);
-  }
+  });
 };
 
 function requestResource(tagSrc, callback) {
@@ -111,10 +82,5 @@ function requestResource(tagSrc, callback) {
   jptScript.src = tagSrc;
 
   // add the new script tag to the page
-  var elToAppend = document.getElementsByTagName('head');
-  elToAppend = elToAppend.length ? elToAppend : document.getElementsByTagName('body');
-  if (elToAppend.length) {
-    elToAppend = elToAppend[0];
-    elToAppend.insertBefore(jptScript, elToAppend.firstChild);
-  }
+  utils.insertElement(jptScript);
 }

src/prebid.js

@@ -4,7 +4,6 @@ import { getGlobal } from './prebidGlobal';
 import { flatten, uniques, isGptPubadsDefined, adUnitsFilter, getLatestHighestCpmBid, isArrayOfNums } from './utils';
 import { listenMessagesFromCreative } from './secureCreatives';
 import { userSync } from './userSync.js';
-import { loadScript } from './adloader';
 import { config } from './config';
 import { auctionManager } from './auctionManager';
 import { targeting, getHighestCpmBidsFromBidPool } from './targeting';
@@ -588,17 +587,6 @@ $$PREBID_GLOBAL$$.createBid = function (statusCode) {
   return createBid(statusCode);
 };
 
-/**
- * @deprecated this function will be removed in the next release. Prebid has deprected external JS loading.
- * @param  {string} tagSrc [description]
- * @param  {Function} callback [description]
- * @alias module:pbjs.loadScript
- */
-$$PREBID_GLOBAL$$.loadScript = function (tagSrc, callback, useCache) {
-  utils.logInfo('Invoking $$PREBID_GLOBAL$$.loadScript', arguments);
-  loadScript(tagSrc, callback, useCache);
-};
-
 /**
  * Enable sending analytics data to the analytics provider of your
  * choice.

test/mocks/adloaderStub.js

@@ -3,19 +3,17 @@ import * as adloader from 'src/adloader';
 
 let sandbox;
 
-export let loadScript;
 export let loadExternalScript;
-export let loadScriptStub;
 export let loadExternalScriptStub;
 
 beforeEach(function() {
   sandbox = sinon.sandbox.create();
-  loadScript = adloader.loadScript;
   loadExternalScript = adloader.loadExternalScript;
-  loadScriptStub = sandbox.stub(adloader, 'loadScript').callsFake((...args) => {
-    args[1]();
+  loadExternalScriptStub = sandbox.stub(adloader, 'loadExternalScript').callsFake((...args) => {
+    if (typeof args[2] === 'function') {
+      args[2]();
+    }
   });
-  loadExternalScriptStub = sandbox.stub(adloader, 'loadExternalScript');
 });
 
 afterEach(function() {

test/spec/adloader_spec.js

@@ -27,5 +27,16 @@ describe('adLoader', function () {
       expect(utilsLogErrorStub.called).to.be.false;
       expect(utilsinsertElementStub.called).to.be.true;
     });
+
+    it('should not load cached script again', function() {
+      adLoader.loadExternalScript('someURL', 'criteo');
+      expect(utilsinsertElementStub.called).to.be.false;
+    });
+
+    it('callback function can be passed to the function', function() {
+      let callback = function() {};
+      adLoader.loadExternalScript('someURL1', 'criteo', callback);
+      expect(utilsinsertElementStub.called).to.be.true;
+    });
   });
 });

test/spec/renderer_spec.js

@@ -1,6 +1,7 @@
 import { expect } from 'chai';
 import { Renderer } from 'src/Renderer';
 import * as utils from 'src/utils';
+import { loadExternalScript } from 'src/adloader';
 
 describe('Renderer', function () {
   describe('Renderer: A renderer installed on a bid response', function () {
@@ -127,5 +128,22 @@ describe('Renderer', function () {
       });
       expect(utilsSpy.callCount).to.equal(1);
     });
+
+    it('should call loadExternalScript() for script not defined on adUnit', function() {
+      $$PREBID_GLOBAL$$.adUnits = [{
+        code: 'video1',
+        renderer: {
+          url: 'http://cdn.adnxs.com/renderer/video/ANOutstreamVideo.js',
+          render: sinon.spy()
+        }
+      }];
+      let testRenderer = Renderer.install({
+        url: 'https://httpbin.org/post',
+        config: { test: 'config1' },
+        id: 1,
+        adUnitCode: undefined
+      });
+      expect(loadExternalScript.called).to.be.true;
+    });
   });
 });

test/spec/unit/pbjs_api_spec.js

@@ -2033,17 +2033,6 @@ describe('Unit: Prebid Module', function () {
     });
   });
 
-  describe('loadScript', function () {
-    it('should call adloader.loadScript', function () {
-      const tagSrc = '';
-      const callback = Function;
-      const useCache = false;
-
-      $$PREBID_GLOBAL$$.loadScript(tagSrc, callback, useCache);
-      assert.ok(adloader.loadScriptStub.calledWith(tagSrc, callback, useCache), 'called adloader.loadScript');
-    });
-  });
-
   describe('aliasBidder', function () {
     it('should call adapterManager.aliasBidder', function () {
       const aliasBidAdapterSpy = sinon.spy(adapterManager, 'aliasBidAdapter');